git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] gpg-interface: trim CR from ssh-keygen

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 3, 2022, 23:34 UTC
Message-ID
<xmqqee5oieb2.fsf@gitster.g>
In-Reply-To
<CAPig+cS6h6o2_dJAZC1M1Ace29bN2mhPgaEtTWtj3oXfcHq9cA@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 22 quoted lines
> On Mon, Jan 3, 2022 at 9:24 AM Fabian Stelzer <fs@gigacodes.de> wrote:
>> We need to trim \r from the output of 'ssh-keygen -Y find-principals' on
>> Windows, or we end up calling 'ssh-keygen -Y verify' with a bogus signer
>> identity. ssh-keygen.c:2841 contains a call to puts(3), which confirms
>> this hypothesis. Signature verification passes with the fix.
>>
>> Helped-by: Pedro Martelletto <pedro@yubico.com>
>> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
>> ---
>> diff --git a/gpg-interface.c b/gpg-interface.c
>> @@ -509,7 +509,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
>> -                       trust_size = strcspn(line, "\n");
>> +                       trust_size = strcspn(line, "\n"); /* truncate at LF */
>> +                       if (trust_size && trust_size != strlen(line) &&
>> +                           line[trust_size - 1] == '\r')
>> +                               trust_size--; /* the LF was part of CRLF at the end */
>
> I may be misunderstanding, but isn't the strlen() unnecessary?
>
>     if (trust_size && line[trust_size] &&
>         line[trust_size - 1] == '\r')
>             trust_size--;

That changes behaviour when "line" has more than one lines in it. strcspn() finds the first LF, and the posted patch ignores CRLF not at the end of line[]. Your variant feels more correct if the objective is to find the end of the first line (regardless of the choice of the end-of-line convention, either LF or CRLF) and omit the line terminator.

Previous: Eric SunshineNext: Eric Sunshine
Message 10 of 30 in “gpg-interface: trim CR from ssh-keygen -Y find-principals”
  1. gpg-interface: trim CR from ssh-keygen -Y find-principalsJohannes Schindelin via GitGitGadget, Dec 3, 2021
  2. Fabian StelzerDec 3, 2021
  3. Jeff KingDec 3, 2021
  4. Fabian StelzerDec 4, 2021
  5. Junio C HamanoDec 5, 2021
  6. Damien MillerDec 5, 2021
  7. Fabian StelzerDec 6, 2021
  8. gpg-interface: trim CR from ssh-keygenFabian Stelzer, Jan 3, 2022
  9. Eric SunshineJan 3, 2022
  10. Junio C HamanoJan 3, 2022
  11. Eric SunshineJan 4, 2022
  12. Junio C HamanoJan 4, 2022
  13. Eric SunshineJan 4, 2022
  14. Fabian StelzerJan 4, 2022
  15. Junio C HamanoJan 4, 2022
  16. Eric SunshineJan 5, 2022
  17. Fabian StelzerJan 5, 2022
  18. Junio C HamanoJan 5, 2022
  19. Fabian StelzerJan 6, 2022
  20. Junio C HamanoJan 6, 2022
  21. Eric SunshineJan 9, 2022
  22. Fabian StelzerJan 10, 2022
  23. gpg-interface: trim CR from ssh-keygenFabian Stelzer, Jan 7, 2022
  24. Eric SunshineJan 9, 2022
  25. Fabian StelzerJan 10, 2022
  26. Junio C HamanoJan 10, 2022
  27. Junio C HamanoJan 10, 2022
  28. Fabian StelzerDec 9, 2021
  29. Fabian StelzerDec 9, 2021
  30. Fabian StelzerDec 30, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.