Mailing List Archive

Question about ssh-keygen -Y find-principals
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@metacode.biz -n file -s rsa-key.txt.sig < rsa-key.txt
allowed_signers.md:3: missing key <---- here
wiktor@metacode.biz

`-Y verify` doesn't have this issue:

$ ssh-keygen -Y verify -f allowed_signers.md -I wiktor@metacode.biz -n
file -s rsa-key.txt.sig < rsa-key.txt
Good "file" signature for wiktor@metacode.biz with RSA key
SHA256:xb+QgBmoSdveobEdwKqUb3BCk9SLJVxq3Ltu2o/FK7U

The man page documentation for ALLOWED_SIGNERS
(https://man.archlinux.org/man/ssh-keygen.1#ALLOWED_SIGNERS):

> Empty lines and lines starting with a ‘#’ are ignored as comments.

I'm using openssh version 9.6p1-3 as packaged in Arch Linux.

I've made a repo with all keys and files I'm using:
https://github.com/wiktor-k/ssh-repro

Context: I'm using SSH signatures in git and wanted to add a bit of
spacing in the file but then `git log --show-signature` shows all these
warnings which I traced to be coming from `find-principals`:

commit 78bf960bccfd7677a72362ace717027dc4a7151a
Good "git" signature for wiktor@metacode.biz with ECDSA key
SHA256:gp2CMX5++SXkPHiyva6kyhp2ftFo6r1HvYeDPVAxvXc
allowed_signers.md:3: missing key^M
allowed_signers.md:5: missing key^M
allowed_signers.md:7: missing key^M

Is this a minor issue or am I holding it wrong?

Thanks for your time!

Kind regards,
Wiktor
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
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@metacode.biz -n file -s rsa-key.txt.sig < rsa-key.txt
> allowed_signers.md:3: missing key <---- here
> wiktor@metacode.biz

I think this is what is happening:

> 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. 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 */
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
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
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
> On Mar 7, 2024, at 10:44, Damien Miller <djm@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
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
>It will be obvious to most readers that Damien meant "carriage return characters" (that is, 0x1D = ^M = '\r').

I think you meant 0x0D :)

>Just heading off any pedantic corrrections at the pass before they hit the mailing list archive.

No worries!

Kind regards,
Wiktor
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
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
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
On Fri, 8 Mar 2024, Wiktor Kwapisiewicz wrote:

> Hi Damien,
>
> I've verified that slightly modifying your patch makes the problem disappear:

Thanks.

> 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've committed this and it will be in next week's release.

Thanks,
Damien
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: Question about ssh-keygen -Y find-principals [ In reply to ]
On 8.03.2024 23:17, Damien Miller wrote:
> I've committed this and it will be in next week's release.

Thanks a lot for your time and the quick turnaround!

Kind regards,
Wiktor
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev