Mailing List Archive

[Bug 3190] Inconsistent handling of private keys without accompanying public keys
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

Jakub Jelen <jjelen@redhat.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #3424| |ok?
Flags| |

--- Comment #1 from Jakub Jelen <jjelen@redhat.com> ---
Created attachment 3424
--> https://bugzilla.mindrot.org/attachment.cgi?id=3424&action=edit
Proposed patch to fall back to alternative methods of getting public
key

It turned out that initial solution is as easy as fixing the logic in
the conditions (see attached patch).

In this function we need to return (goto out) in case we found the key,
not the other way round.

As this code was recently written by Damien, I added him for review.

With the attached patch, keys in openssh format seems to work
correctly. If there would not be anything against, I would like to have
a look also to normal non-encrypted PEM files to take similar approach
and probably add some regression test case to keep this working.

--
You are receiving this mail because:
You are watching the assignee of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

Jakub Jelen <jjelen@redhat.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |djm@mindrot.org
Attachment #3424|ok? |ok?(djm@mindrot.org)
Flags| |

--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

Jakub Jelen <jjelen@redhat.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #3424|ok?(djm@mindrot.org) |
Flags| |
Attachment #3424|0 |1
is obsolete| |

--- Comment #2 from Jakub Jelen <jjelen@redhat.com> ---
Comment on attachment 3424
--> https://bugzilla.mindrot.org/attachment.cgi?id=3424
Proposed patch to fall back to alternative methods of getting public
key

ok, not so fast. It will require some more tweaking.

--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

--- Comment #3 from Jakub Jelen <jjelen@redhat.com> ---
It looks like the initial attempt to reproduce the issue was not done
with master, where the new openssh keys work as expected.

But the same does not work with legacy PEM files and this conversion is
not tested at all so this is what the bug/patch will be about.

--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

--- Comment #4 from Jakub Jelen <jjelen@redhat.com> ---
Created attachment 3425
--> https://bugzilla.mindrot.org/attachment.cgi?id=3425&action=edit
regression test + getting public key from PEM

This is another take with regression test for the
sshkey_parse_pubkey_from_private_fileblob_type() function, adding
support for parsing PEM files and retrieving pubkeys from them.

--
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

--- Comment #5 from Damien Miller <djm@mindrot.org> ---
Created attachment 3428
--> https://bugzilla.mindrot.org/attachment.cgi?id=3428&action=edit
attempt to load public key from passphraseless private keys

PEM doesn't include the public key in encrypted private keys' cleartext
though, right?

IMO we could load passphrase-free keys, but we should remove their
private elements immediately after loading.

--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs
[Bug 3190] Inconsistent handling of private keys without accompanying public keys [ In reply to ]
https://bugzilla.mindrot.org/show_bug.cgi?id=3190

--- Comment #6 from Jakub Jelen <jjelen@redhat.com> ---
(In reply to Damien Miller from comment #5)
> Created attachment 3428 [details]
> attempt to load public key from passphraseless private keys
>
> PEM doesn't include the public key in encrypted private keys'
> cleartext though, right?

right.

> IMO we could load passphrase-free keys, but we should remove their
> private elements immediately after loading.

Right. That was the idea and I think the only missing bit.

But I got a bit confused since had old openssh installed and the
handling of new format was already in master.

Your patch works fine after fixing two minor nits:


{
char *pubfile = NULL, *privcmt = NULL;
int r, oerrno;
- struct sshkey *privkey;
+ struct sshkey *privkey = NULL;

if (keyp != NULL)
*keyp = NULL;


*/
if ((r = sshkey_load_private(filename, "", &privkey, &privcmt))
== 0) {
if ((r = sshkey_from_private(privkey, keyp)) == 0) {
- if (commentp != NULL)
+ if (commentp != NULL) {
*commentp = privcmt;
privcmt = NULL; /* transferred */
}


The only ugly corner case is the removal of key from ssh-agent, which
still fails with cryptic error if the key is encrypted PEM missing
sidecar public key:

$ ssh-add -d /tmp/rsa
Bad key file /tmp/rsa: No such file or directory

Otherwise it looks good.

--
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs