Re: [PATCH v6 5/9] ssh signing: parse ssh-keygen output and verify signatures

From: Fabian Stelzer <fs@gigacodes.de>
Date: Jul 30, 2021, 08:17 UTC
Message-ID: <30489b9f-8bbb-22c3-bd36-95f430a45ba9@gigacodes.de>
In-Reply-To: <03a101d784c9$0cb413a0$261c3ae0$@nexbridge.com>

On 30.07.21 00:28, Randall S. Becker wrote:

Show 43 quoted lines

> On July 29, 2021 5:29 PM, Fabian Stelzer wrote:
>> On 29.07.21 23:25, Randall S. Becker wrote:
>>> On July 29, 2021 5:13 PM, Fabian Stelzer wrote:
>>>> Subject: Re: [PATCH v6 5/9] ssh signing: parse ssh-keygen output and
>>>> verify signatures
>>>>
>>>> On 29.07.21 23:01, Randall S. Becker wrote:
>>>>> On July 29, 2021 4:46 PM, Junio wrote:
>>>>>> Fabian Stelzer <fs@gigacodes.de> writes:
>>>>>>
>>>>>>> On 29.07.21 01:04, Jonathan Tan wrote:
>>>>>>>
>>>>>>>> Also, is this output documented to be stable even across locales?
>>>>>>> Not really :/ (it currently is not locale specific)
>>>>>>
>>>>>> We probably want to defeat l10n of the message by spawning it in the C locale regardless.
>>>>>>
>>>>>>> The documentation states to only check the commands exit code. Do
>>>>>>> we trust the exit code enough to rely on it for verification?
>>>>>>
>>>>>> Is the exit code sufficient to learn who signed it?  Without
>>>>>> knowing that, we cannot see if the principal is in or not in our
>>>>> keychain, no?
>>>>>
>>>>> Have we not had issues in the past depending on exit code? I'm not sure this can be made entirely portable.
>>>>>
>>>>
>>>> To find the principal (who signed it) we don't have to parse the output.
>>>> Since verification is first a call to look up the principals matching
>>>> the signatures public key from the allowedSignersFile and then trying
>>>> verification with each one we already know which one matched (usually there is only one. I think multiples is only possible with an SSH
>> CA).
>>>> Of course this even more relies on the exit code of ssh-keygen.
>>>>
>>>> Not sure which is more portable and reliable. Parsing the textual output or the exit code. At the moment my patch does both.
>>>
>>> What about a configurable exit code for this? See the comment below about that.
>>>
>>
>> I'm not sure what you mean. Something like "treat exit(123) as success"?
> 
> How about gpg.ssh.successExit=123 or something like that.
>

I don't quite understand what the benefit would be. Do you have any specific portability problems/concerns where the ssh-keygen format is different or exit codes differ? I think using a script that provides exit(0) on success and the correct output to wrap ssh-keygen and setting it in gpg.ssh.command can already cover edge cases when needed.

> 
> Is there documentation on the possible arguments the patch series will use for this so one can create a wrapper script? I had to look into the code to find out what GIT_SSH_COMMAND actually required when the ssh variant was "ssh". I'd rather not have to do that in this case.
>

The documentation in ssh-keygen(1) is quite good and straight forward for verification and signing. Again if you have any specific portability concerns i'd be glad to help.