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@xxxxxxxxxxxx> 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. >>>>>> If so then i can move the main result and only parse the text for >>>>>> the signer/fingerprint info thats used in log formats. This way >>>>>> only the logs would break in case the output changes. >>>>>> >>>>>> I added the output check since the gpg code did so as well: >>>>>> ret |= !strstr(gpg_stdout.buf, "\n[GNUPG:] GOODSIG "); >>>>> >>>>> Does ssh-keygen have a mode similar to gpg's --status-fd feature >>>>> where its output is geared more towards being stable and marchine parseable than being human friendly, by the way? >>>> >>>> I do not think this can be done in a platform independent way. Not >>>> every platform that has ssh-keygen conforms to the OpenSSH UI or output - a particular annoyance I get daily. >>>> >> >> What about a configurable command, like GIT_SSH_COMMAND to allow someone to plug in a mechanism or write something that >supplies a result you can handle? That's something I could probably work out on my own platforms. >> > >This is already possible by setting gpg.ssh.program (although you'd have to pass the sign operation as well) 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. Thanks, Randall
