[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] OSX Keychain support

From: mark benedetto king <mbk_at_lowlatency.com>
Date: 2005-12-03 16:11:01 CET

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_LIBS="-framework Security -framework CoreFoundation -framework CoreServices"
+ AC_DEFINE([SVN_HAVE_KEYCHAIN_SERVICES], [1], [Is Mac OS KeyChain support enabled?])

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>


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?


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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.