On Fri, Dec 02, 2005 at 03:03:54PM -0800, Wilfredo S?nchez Vega wrote:
> I've reworked Ken's patch to work on trunk, added the necessary
> glue to autoconf, and then reworked a few things.
>
I think you left out keychain_impl.h :-)
> In particular, swapped the use of service name and account around
> to better match what happens with internet passwords (those used by
> Safari), and got rid of static buffer allocations.
>
> Patch file (against today's trunk) is attached.
>
> If there is code somewhere to split up the realmstring passed to
> the auth functions (we'd need to pull out protocol, host(+port), and
> realm name), then we can use internet passwords instead of generic
> passwords, which would allow us to use the same credentials as Safari
> (and other clients) is using and vice versa.
We don't (AFAIK) have a realm-string parsing API.
I looked at the Internet API before settling on Generic passwords;
it wasn't clear to me that we could use "svn" as a protocol (it's
not one of the enumerated SecProtocolType values). Maybe you can
shed some light on that.
>
> The only thing I don't understand is whether the patch to
> subversion/libsvn_ra_dav/session.c is actually necessary. I don't
> have a client side cert for any svn repo, so I couldn't test the SSL
> cert password bits.
>
I had the same question about the original patch. I haven't tried to
store cert passwords in the keychain yet. I will commit the simple
work first and then work through the SSL case.
I think a few things remain, though:
1.) The Windows password encryption/decryption was implemented
inside the simple provider. I made some changes a little
while ago:
r16602 | mbk | 2005-10-08 21:40:29 -0400 (Sat, 08 Oct 2005) | 19 lines
Rework the simple auth provider internals to allow for more control over
the storage of the password component of the credentials (in preparation
for adding Mac OS X Keychain support).
So my implementation added a third simple_provider ("darwin") that
used the same helper system to drive the config-file storage of the
username. This enabled me to call SecKeychainFindGenericPassword()
only once, using the username from the config-file as the key,
rather than two calls, one with a "username" prompt and one
with a "password" prompt.
Perhaps this wasn't the best strategy; perhaps I should have made
a first-class provider as you have done, and stored the username
and password as separate entries. That might be more consistent
with other OS X application behaviour.
I'm not sentimentially attached to those changes, and I'll revert
them if they serve no purpose, but I'd like to make sure
that we're moving in the right direction when I do. Do you have
an opinion on this?
2.) Configury.
+if test "$enable_keychain" = "yes"; then
+ AC_MSG_RESULT([yes])
+ SVN_KEYCHAIN_INCLUDES=""
+ SVN_KEYCHAIN_LIBS="-framework Security -framework CoreFoundation -framework CoreServices"
+ AC_DEFINE([SVN_HAVE_KEYCHAIN_SERVICES], [1], [Is Mac OS KeyChain support enabled?])
+ AC_SUBST(SVN_KEYCHAIN_INCLUDES)
+ AC_SUBST(SVN_KEYCHAIN_LIBS)
+else
+ AC_MSG_RESULT([no])
+fi
Yay, configury! But won't this break the build on non-OS-X systems, or
at least systems whose ld can't ignore -framework?
And is it sufficient?
I had code like
#ifdef DARWIN
#include <AvailabilityMacros.h>
#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_2)
#define SVN_HAVE_KEYCHAIN_SERVICES 1
#endif
to test for the presence of the API. We could do this test at
configure-time. But is this the right test?
3.) Support for --non-interactive.
Is surrounding the SecKeychain calls with calls to
SecKeychainSetUserInteractionAllowed(FALSE) and then
SecKeychainSetUserInteractionAllowed(TRUE) the right way to implement
--non-interactive? Could it be that easy?
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 3 16:09:44 2005