Re: [PATCH] osxkeychain: lock for exclusive execution

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: May 10, 2024, 20:33 UTC
Message-ID: <Zj6EhJi9MgALC5Ti@tapette.crustytoothpaste.net>
In-Reply-To: <20240510200114.GC1954863@coredump.intra.peff.net>

On 2024-05-10 at 20:01:14, Jeff King wrote:

Show 15 quoted lines

> And I think there are several problems with that, besides inefficiency
> and locking. See this old patch, which fixes it by remembering when
> a credential came from a helper:
> 
>   https://lore.kernel.org/git/20120407033417.GA13914@sigill.intra.peff.net/
> 
> But we didn't merge it because some people rely on the behavior of
> helpers feeding back to themselves. I outlined some solutions there, but
> it would definitely be a change in behavior that people would have to
> adapt to.
> 
> 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.

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

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.

-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA