Re: [PATCH] osxkeychain: lock for exclusive execution

From: Jeff King <peff@peff.net>
Date: May 10, 2024, 22:07 UTC
Message-ID: <20240510220715.GB1962678@coredump.intra.peff.net>
In-Reply-To: <Zj6EhJi9MgALC5Ti@tapette.crustytoothpaste.net>

On Fri, May 10, 2024 at 08:33:08PM +0000, brian m. carlson wrote:

Show 17 quoted lines

> > Some possible alternatives:
> > 
> >   - we could remember _which_ helper we got the credential from, and
> >     avoid invoking it again.
> 
> This will break the new `state[]` feature, which relies on being able to
> see the state after the fact to know whether the operation was
> successful.  As an example of the functionality the current approach
> allows, authentication could use an HOTP (like TOTP, but using a counter
> instead of time) value, and storing the correct used counter on success
> would be important.
> 
> I agree it's not super important if we're just using a username and
> password, but considering I just added support for arbitrary
> authentication schemes, which can include things such as limited-use
> OAuth tokens, one-time use passcodes, and certain types of HMAC-based
> signing, we probably don't want to choose this approach.

Yeah, I think it makes sense to keep the Git side as general as possible. So invoking the helper but giving it extra information (so it can decide whether to be a noop or not) seems like the better approach.

Show 7 quoted lines

> >   - we could record a bit saying that the credential came from a helper,
> >     and then feed that back to helpers when storing. So osxkeychain
> >     could then decide not to store it.
> 
> This is actually possible with the new `state[]` feature.  `osxkeychain`
> can simply set that field to something like `osxkeychain:seen=1` and
> simply do nothing if it sees that field.

Makes sense. Back in that old thread I showed a patch which would let helpers pass arbitrary fields to each other (or back to themselves), and this works in roughly the same way.

> All the credential helper needs to do is declare support for that
> functionality with the appropriate capability and emit the field if it
> gets that capability on standard input.

If I understand the protocol, it is just:

  printf("capability[]=state\n");
  printf("state[]=osxkeychain:seen=1\n");

in the helper when it returns a username/password? And I guess the matching parse/check on "store".

Sounds like that would be easy for folks on macOS to play with.

-Peff