On Thu, 7 Mar 2024, Wiktor Kwapisiewicz wrote:> Hello, > > I've noticed that `ssh-keygen -Y find-principals` warns about empty > lines in the allowed signers file, even though the documentation says > they should be treated as comments: > > $ ssh-keygen -Y find-principals -f allowed_signers.md -I > wiktor at metacode.biz -n file -s rsa-key.txt.sig < rsa-key.txt > allowed_signers.md:3: missing key <---- here > wiktor at metacode.bizI think this is what is happening:> allowed_signers.md:3: missing key^MYou have line feed characters in your allowed_signers file, possibly from editing it on a Windows system. We don't currently ignore this character at the ends of lines. You could try removing them or try this patch: diff --git a/sshsig.c b/sshsig.c index d50d65fe2..145bca862 100644 --- a/sshsig.c +++ b/sshsig.c @@ -747,7 +747,7 @@ parse_principals_key_and_options(const char *path, u_long linenum, char *line, cp = line; cp = cp + strspn(cp, " \t"); /* skip leading whitespace */ - if (*cp == '#' || *cp == '\0') + if (*cp == '#' || *cp == '\0' || strcmp(cp, "\r") == 0) return SSH_ERR_KEY_NOT_FOUND; /* blank or all-comment line */ /* format: identity[,identity...] [option[,option...]] key */
Hi Damien, Thanks for contacting me back! Fortunately, I don't use Windows, and as far as I can see, the file doesn't have any windows line endings: curl -sSL https://raw.githubusercontent.com/wiktor-k/ssh-repro/main/allowed_signers.md | xxd Displays only \n's. I think the line ending is an issue with git not ssh keygen. Maybe the patch should include \n instead of \r? I will test it tomorrow when I'm near my computer, but I just wanted to point out this linefeed observation. Thanks for taking the time to write the patch! Kind regards, Wiktor
> On Mar 7, 2024, at 10:44, Damien Miller <djm at mindrot.org> wrote: > >> allowed_signers.md:3: missing key^M > > You have line feed characters in your allowed_signers file, possibly from > editing it on a Windows system. [...]It will be obvious to most readers that Damien meant "carriage return characters" (that is, 0x1D = ^M = '\r'). Just heading off any pedantic corrrections at the pass before they hit the mailing list archive. -- jmk
Hi Damien, I've verified that slightly modifying your patch makes the problem disappear: diff --git a/sshsig.c b/sshsig.c index d50d65fe2..145bca862 100644 --- a/sshsig.c +++ b/sshsig.c @@ -747,7 +747,7 @@ parse_principals_key_and_options(const char *path, u_long linenum, char *line, cp = line; cp = cp + strspn(cp, " \t"); /* skip leading whitespace */ - if (*cp == '#' || *cp == '\0') + if (*cp == '#' || *cp == '\0' || strcmp(cp, "\n") == 0) return SSH_ERR_KEY_NOT_FOUND; /* blank or all-comment line */ /* format: identity[,identity...] [option[,option...]] key */ (Note the \n instead of \r) I've also experimented with the code a bit and found out that if the line that skips the whitespace: cp = cp + strspn(cp, " \t"); /* skip leading whitespace */ is adjusted slightly to include newline characters: cp = cp + strspn(cp, " \t\n\r"); /* skip leading whitespace */ if (*cp == '#' || *cp == '\0') /* <- no change in this line */ then the problem also disappears. I'd be happy to send a patch if you think one of these look reasonable. Thanks for help! Kind regards, Wiktor