Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain

From: Jay Soffian <jaysoffian@gmail.com>
Date: Sep 30, 2011, 19:33 UTC
Message-ID: <CAG+J_DyhcA7RmHwgGJBw4r9JRij0_ONp3ZMD6oMTJ_f4dvYW8w@mail.gmail.com>
In-Reply-To: <CAEBDL5WhpVg17aPuRqrE5=2Q293kVD4fYtxGqRzx_K=87t-jgw@mail.gmail.com>

On Thu, Sep 29, 2011 at 6:03 AM, John Szakmeister <john@szakmeister.net> wrote:

>
> I've been working on a version of the keychain credential cache as
> well.  I did create a gui, although it's a bit painful.

I still don't understand why a CLI app should have a GUI credential prompt.

Show 9 quoted lines

>>  2. The "get the username from the config" feature is triggered at the
>>     time of prompting the user (so instead of asking for the username,
>>     we check the config and pretend the user told us).
>>
>>     I did it this way originally so that helpers would have the first
>>     crack at setting a username, and we would fall back to the config.
>>     Thinking on it more, that may be backwards. If the user has told
>>     git "for github.com, I am user 'foo'", then that should probably
>>     take effect first, and --username=foo get passed to the helper.

Sorry, missed this part in my previous reply. I don't understand - how do you ever send a username to the credential helper if you don't get it from the config? But in any case, if you have a username (via config or some other way), yes, I think it should be given to the credential helper.

Show 5 quoted lines

> I think that makes sense.  I think one thing we have to be careful
> about partial matches.  I wouldn't want the credential cache to send
> off the wrong password to a service.  This may be me being cautious,
> but if I don't have all the necessary bits, I'd rather we fail that to
> guess which entry is right.

The credential helper I wrote doesn't work that way. To do so would mean using a rather more complicated form of the OS X Security API. It asks for an entry using whatever fields it has, and OS X returns the first match that satisfies. It's up to the user to yea/nay that match if the credential helper isn't on the entry's ACL.

Show 7 quoted lines

>>> +     /* "GitHub for Mac" compatibility */
>>> +     if (!strcmp(hostname, "github.com"))
>>> +             hostname = "github.com/mac";
>>
>> Nice touch. :)
>
> I honestly don't understand why this needs to be done.

Because GitHub for Mac stores its entries using "github.com/mac" as the hostname.

> I don't use GitHub for Mac... does that mean this is busted for me?

No. It just means that the credential helper and GitHub for Mac store their entry in a compatible fashion. (So that each can locate the entry stored by the other.)

Show 11 quoted lines

> [snip]
>> My series will also produce "cert:/path/to/certificate" when unlocking a
>> certificate. The other candidates for conversion are smtp-auth (for
>> send-email) and imap (for imap-send).  I guess for certs, you'd want to
>> use the "generic" keychain type.
>
> There is a method for adding a certificate to the keychain:
>   <http://developer.apple.com/library/mac/#documentation/Security/Reference/certifkeytrustservices/Reference/reference.html#//apple_ref/doc/uid/TP30000157>
>
> I'm not sure what that does exactly, but I do have a cert, and it
> shows up as "certificate" in the keychain.

That's for storing a certificate itself. In this case, I think we're just talking about storing the passphrase which protects the certificate's private key.

Show 10 quoted lines

>> I wonder if some people would not want to cache cert passwords. Speaking
>> of which, I remember keychain asking me "do you want to let git see this
>> password?", but I don't ever remember it asking "do you want to save
>> this password?". Is that usually automatic? Again, I was kind of
>> expecting a dialog with a "remember this" checkbox.
>
> By the time you get Keychain involved, the decision has been made.
> Most applications offer that ability... and you're right, this should
> probably offer the same capability.  That also means stashing that
> data somewhere. :-(  OTOH, it does make for a better user experience.

What, no? If you don't want git to store usernames/passwords stored in the OS X Keychain, don't use the git-osx-keychain credential helper.

j.