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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 29, 2011 at 3:56 AM, Jeff King <peff@xxxxxxxx> wrote:
[snip]
> This was my first one. I kind of expected there to be some kind of
> graphical password dialog. Especially because keychain will pop up a
> dialog and ask you "is it OK for git to access this password?". So I
> sort of assumed that people would assume that credentials happened
> outside of the regular terminal session (I see the same thing on Linux,
> for example, with gpg-agent, which will open a new window and grab
> focus).
>
> But I have no idea what's "normal" on OS X.

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 wondered if you were trying to be friendly to people who were
> connecting via ssh. But that doesn't seem to work at all. I couldn't get
> either version of your helper to actually do anything in an ssh session
> (even with the same user logged in on console).  I guess there is some
> magic to hook it into the keychain manager.

I'm not sure there is.  For instance, ssh will look up passphrases for
ssh keys in the keychain, but if you ssh into the box, and then try to
ssh to somewhere else, it'll prompt you for the passphrase for the
key.  In my own experiments, the lookup will come back with a failure,
despite the fact that the keychain is already unlocked. :-(

Looking at the Subversion source, they recognize the issue too:

> * XXX (2005-12-07): If no GUI is available (e.g. over a SSH session),
> * you won't be prompted for credentials with which to unlock your
> * keychain.  Apple recognizes lack of TTY prompting as a known
> * problem.

...obviously, Apple hasn't fixed this issue. :-(

> Also, regarding opening /dev/tty yourself versus using getpass. There
> are a few magic things getpass will do that your helper won't:
>
>  1. It respects core.askpass, GIT_ASKPASS, and SSH_ASKPASS if they are
>     set.
>
>  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.

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.

[snip]
> Hrm. I was really hoping people wouldn't need to pick apart the "unique"
> token, and it could remain an opaque blob. If helpers are going to do
> this sort of parsing, then I'd just as soon have git break it down for
> them, and do something like:
>
>  git credential-osxkeychain \
>    --protocol=https \
>    --host=github.com \
>    --path=peff/git.git
>    --username=peff
>
> to just hand over as much information as possible, and let the helper
> throw it all together if it wants to.

I think this is a good idea too.  Keychain definitely wants this
information stored in separate fields, and I suspect the secrets api
does too (I haven't found any formal docs about the preferred way of
storing attributes related to urls though).

>> +     /* "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.  I don't use
GitHub for Mac... does that mean this is busted for me?

[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.

> 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.

[snip]
> I noticed that when using the python helper, the dialog asking something
> like: "security wants to know this password. Allow it?"
>
> Which was kind of lame. I would hope we could convince it to say "git".
> But I didn't see any option in the "security" tool for specifying the
> context[1]. The C helper says "git-credential-osxkeychain". Which isn't
> the end of the world, but it would be prettier if it just said "git".

I'm not sure how easy it is to do that.  I think it's meant to be a
security measure that it points out the executable name... but it does
make it less friendly for users. :-(

> -Peff
>
> [1] I can kind of see why they might not want you to set this for
> security reasons (because it makes impersonating other programs easy).
> On the other hand, saying "security" conveys absolutely nothing. And as
> far as I can tell, I could just call my program /tmp/iTunes, and it
> would say "iTunes wants to know this password...".

Yep, I agree.  And it's worse when using the security command line
tool... when you grant security access to the key, then any app could
technically gain access to the item via the security tool.  That's one
of the reasons I didn't pursue that route early on.

-John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]