Hi!  I've pushed a patch that adds `-U` conditional on is_literal_ssh_key().

According to the OpenSSH issue ([1]), that option is backward compatible:

It should be safe to use -U even for older versions. It won't require the agent (as openssh-9.1 will) but it won't cause an error.

[1]: https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Cheers
— Adam


On Mon, Jan 23, 2023 at 11:02 AM Fabian Stelzer [off-list ref] wrote:

On 23.01.2023 09:33, Phillip Wood wrote:

quoted

On 20/01/2023 09:03, Fabian Stelzer wrote:

quoted

On 18.01.2023 16:29, Phillip Wood wrote:

quoted

Hi Adam

I've cc'd Fabian who knows more about the ssh signing code that I do.

On 18/01/2023 15:28, Adam Szkoda wrote:

quoted

Hi Phillip,

Good point!  My first thought is to try doing a stat() syscall on the
path from 'user.signingKey' to see if it exists and if not, treat it
as a public key (and pass the -U option).  If that sounds reasonable,
I can update the patch.

My reading of the documentation is that user.signingKey may point
to a public or private key so I'm not sure how stat()ing would
help. Looking at the code in sign_buffer_ssh() we have a function
is_literal_ssh_key() that checks if the config value is a public
key. When the user passes the path to a key we could read the file
check use is_literal_ssh_key() to check if it is a public key (or
possibly just check if the file begins with "ssh-"). Fabian - does
that sound reasonable?

Hi,
I have encountered the mentioned problem before as well and tried to
fix it but did not find a good / reasonable way to do so. Git just
passes the user.signingKey to ssh-keygen which states:
`The key used for signing is specified using the -f option and may
refer to either a private key, or a public key with the private half
available via ssh-agent(1)`

I don't think it's a good idea for git to parse the key and try to
determine if it's public or private. The fix should probably be in
openssh (different error message) but when looking into it last time
i remember that the logic for using the key is quite deeply embedded
into the ssh code and not easily adjusted for the signing use case.
At the moment I don't have the time to look into it but the openssh
code for signing is quite readable so feel free to give it a try.
Maybe you find a good way.

Thanks Fabian, perhaps the easiest way forward is for us to only pass
"-U" when we have a literal key in user.signingKey as we know it must
a be public key in that case.

Yes, i think that's a good idea as long as the `-U` flag is ignored in older
ssh versions and shouldn't be too hard to implement. And it should work just
as well when using `defaultKeyCommand`.

Best,
Fabian

quoted

Best Wishes

Phillip

quoted

Best regards,
Fabian

quoted

Best Wishes

Phillip

quoted

Best
— Adam


On Wed, Jan 18, 2023 at 3:34 PM Phillip Wood
[off-list ref] wrote:

quoted

On 18/01/2023 11:10, Phillip Wood wrote:

quoted

quoted

the agent [1].  A fix is scheduled to be released in OpenSSH 9.1. All
that
needs to be done is to pass an additional backward-compatible option
-U to
'ssh-keygen -Y sign' call.  With '-U', ssh-keygen always interprets
the file
as public key and expects to find the private key in the agent.

The documentation for user.signingKey says

 If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users
from setting
this to a private key.

If there is an easy way to tell if the user has given us a public key
then we could pass "-U" in that case.

Best Wishes

Phillip