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

[Jeff King <peff@peff.net>]

On Fri, Sep 30, 2011 at 03:16:58PM -0400, Jay Soffian wrote:

> This makes no sense to me at all. Ignore OS X for the moment. You use
> git on the command-line. Why would there be any expectation of it
> interacting with the user via anything other than the terminal.

Because security prompts sometimes are out of band. In particular,
you're already interacting with the user outside of the terminal;
opening the keychain will get you an "allow git to open the keychain"
dialog.

Another example: if you're running gpg-agent, and you run "git tag -s",
you'll be prompted for your key passphrase in an out-of-band dialog.

Maybe it doesn't make sense for the actual username/password, though.

> I don't understand where you're running ssh from/to in this scenario,
> but OS X has a notion of security contexts (this is a bit of a
> tangent):
> 
> http://developer.apple.com/library/mac/#technotes/tn2083/_index.html

I meant running sshd on OS X, and then ssh-ing in from some other box.
Your credential helper doesn't seem to work at all (I guess because it
has no access to the keychains). I don't think it's a big deal, though.
It's the minority case, and if somebody wants to figure out how to make
it work later, they can.

> I found it ugly that git's native getpass doesn't echo the username
> back, and it seems hackish to me for the credential helper to turn
> back around and invoke it in any case. :-(

Yes, but I think that's a bug that should be fixed in git. :)

As far as being hack-ish, the original design was that you could chain
these things if you wanted. But in practice, I don't know how useful
that is. In fact, after all of this discussion, I'm wondering how useful
it is that the helpers are allowed to prompt themselves at all.

The KDE one does it. But would people be happier if git simply did:

  1. ask the helper for a credential. if we get it, done

  2. prompt the user

  3. if the credential is valid, ask the helper to store

That's way less flexible. But it also makes the helpers really simple.

> > My test harness checks that this case just asks for the password without
> > bothering to do any lookup or storage. It probably doesn't really matter
> > in practice; I think git should always be providing _some_ context.
> 
> Okay, that wasn't clear from whatever documentation I read on how
> credential helpers should behave. But why invoke the credential helper
> just to ask for a password?

Because the helper might have an alternate way of asking for the
password (e.g., the KDE helper has its own dialog). Maybe it will never
be useful. I just wanted to future-proof helpers by giving them sane
behavior for this case, on the off chance that future versions of git do
actually do this.

> > 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.
> 
> Keychain entries have distinct fields. I broke apart the token and
> stored it the way other applications mostly do on OS X.

Don't get me wrong; I think you did the only sane thing. It was more
"Hmm, I was hoping that the right way to use the various security APIs
wouldn't need to...". And clearly my hope was wrong. :)

But this is exactly the sort of feedback I was hoping for; the interface
to the helpers could be better, and we are catching it now.

> > 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.
> 
> Yep, I was punting on certificate for v1.

Not unreasonable. Again, my real concern is not perfect helpers, but
having helpers that exercise the helper interface enough so that we know
whether that interface is sufficient.

> Each keychain entry has an ACL of applications that are allowed to
> access it. When an application asks for an entry and the application
> isn't on that entry's ACL, OS X (not the application) presents the
> user the dialog you refer to. The application has no control over that
> dialog.
> 
> Now, I could have the credential helper ask the user "store this
> password?" after prompting for it, but why even use a credential
> helper if you don't want it to store your credentials?

That's not an unreasonable attitude. I mostly let the browser store
passwords, but sometimes override it for specific sites. But in this
case, I think it would be more per-repo. And you can turn off the helper
for a particular repo (actually, I'm not sure you can, but you probably
should be able to).

> It is for security reasons. 99% of users will probably just click
> "Okay" no matter what. For the 1% that bother to pay attention to the
> dialog, it provides the full path to the binary. I'd be suspicious if
> /tmp/iTunes wanted a password.

Not for me (on 10.7). Doing:

  cp git-credential-osxkeychain /tmp/iTunes
  /tmp/iTunes --unique=https:foo.tld

gives me the dialog "iTunes wants to access the 'login' keychain" with a
password prompt.

Anyway, that's neither here nor there. It would be nice if we could set
the text, but if we can't, then we'll have to live with it.

-Peff