> Sent: Monday, February 03, 2020 at 2:28 AM
> From: "Damien Miller" <d...@mindrot.org>
> To: "Aham Brahmasmi" <aham.brahma...@gmx.com>
> Cc: Misc <misc@openbsd.org>
> Subject: Re: ssh: probable bug in ssh -current
>
> On Fri, 31 Jan 2020, Aham Brahmasmi wrote:
>
> > Bug:
> > When the client connects to the server, they use the ed25519-cert to
> > establish the connection. After the ssh session is established, the
> > server sends the "hostkeys...@openssh.com" message with the server's
> > ed25519 host public key.
> >
> > This results in the client looping over the keys in known hosts file,
> > and deciding that the @cert-authority host certificate authority public
> > key is "deprecated", because it was not sent by the server [1]. The
> > client then informs the user:
> > "
> > The server has updated its host keys.
> > These changes were verified by the server's existing trusted key.
> > Deprecating obsolete hostkey: ED25519
> > SHA256:<host_ca_public_key_fingerprint>
> > Accept updated hostkeys? (yes/no):
> > "
>
> Could you plesse try this patch?
>
>
> Index: clientloop.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
> retrieving revision 1.338
> diff -u -p -r1.338 clientloop.c
> --- clientloop.c 30 Jan 2020 07:20:57 -0000 1.338
> +++ clientloop.c 3 Feb 2020 02:27:18 -0000
> @@ -1856,13 +1859,25 @@ hostkeys_find(struct hostkey_foreach_lin
>
> /* Mark off keys we've already seen for this host */
> for (i = 0; i < ctx->nkeys; i++) {
> - if (sshkey_equal(l->key, ctx->keys[i])) {
> + if ((l->marker & MRK_CA) != 0) {
> + if (!sshkey_is_cert(ctx->keys[i]))
> + continue;
> + if (!sshkey_equal(ctx->keys[i]->cert->signature_key,
> + l->key))
> + continue;
> + debug3("%s: found %s CA key at %s:%ld", __func__,
> + sshkey_ssh_name(ctx->keys[i]), l->path, l->linenum);
> + ctx->keys_seen[i] = 1;
> + return 0;
> + } else if (sshkey_equal(l->key, ctx->keys[i])) {
> debug3("%s: found %s key at %s:%ld", __func__,
> sshkey_ssh_name(ctx->keys[i]), l->path, l->linenum);
> ctx->keys_seen[i] = 1;
> return 0;
> }
> }
> + if ((l->marker & MRK_REVOKE) != 0)
> + return 0;
> /* This line contained a key that not offered by the server */
> debug3("%s: deprecated %s key at %s:%ld", __func__,
> sshkey_ssh_name(l->key), l->path, l->linenum);
> @@ -1961,10 +1976,11 @@ update_known_hosts(struct hostkeys_updat
> if (stat(options.user_hostfiles[i], &sb) != 0) {
> if (errno == ENOENT) {
> debug("%s: known hosts file %s does not exist",
> - __func__, strerror(errno));
> + __func__, options.user_hostfiles[i]);
> } else {
> - error("%s: known hosts file %s inaccessible",
> - __func__, strerror(errno));
> + error("%s: known hosts file %s inaccessible: "
> + "%s", __func__, options.user_hostfiles[i],
> + strerror(errno));
> }
> continue;
> }
>
Namaste Damien,
Thank you very much for the patch. I apologize for the delay in my
response.
Applying the patch as is on -current (clientloop.c, v 1.340) did not
resolve the problem. In fact, the patched ssh client tried to
simultaneously add as well as delete the ssh-ed25519 key present in line
2.
As a result, I tried to read and execute the code. Here is what I
understood:
1) The changes to the strings in the error and debug functions work
correctly.
2) I think we may benefit from changing the lines in the patch:
if ((l->marker & MRK_CA) != 0) {
to
if (l->marker == MRK_CA) {
and
if ((l->marker & MRK_REVOKE) != 0)
to
if (l->marker == MRK_REVOKE)
I base this on what I saw in record_hostkey function in hostfile.c [1].
In case we do intend to perform bit-wise and operations, could you
please share the rationale behind it?
3) Now, with the above change, given that the known_hosts contains:
@cert-authority <server> ssh-ed25519 <host_ca_public_key>
<server> ssh-ed25519 <server_ed25519_host_public_key>
the following is what happens during the first iteration of
hostkeys_find function in the patched clientloop.c (1.340 + patch +
above change):
the execution enters the following if block and the continue is
executed:
if (!sshkey_is_cert(ctx->keys[i]))
continue;
The sshkey_is_cert function is in the sshkey.c file [2].
...
int
sshkey_type_is_cert(int type)
{
const struct keytype *kt;
for (kt = keytypes; kt->type != -1; kt++) {
if (kt->type == type)
return kt->cert;
}
return 0;
}
...
int
sshkey_is_cert(const struct sshkey *k)
{
if (k == NULL)
return 0;
return sshkey_type_is_cert(k->type);
}
...
During the execution, this sshkey_is_cert function returns 0. This is
because the argument key is of type=3 which implies "KEY_ED25519"
according to the enum sshkey_types in file sshkey.h [3].
...
/* Key types */
enum sshkey_types {
KEY_RSA,
KEY_DSA,
KEY_ECDSA,
KEY_ED25519,
KEY_RSA_CERT,
KEY_DSA_CERT,
KEY_ECDSA_CERT,
KEY_ED25519_CERT,
KEY_XMSS,
KEY_XMSS_CERT,
KEY_ECDSA_SK,
KEY_ECDSA_SK_CERT,
KEY_ED25519_SK,
KEY_ED25519_SK_CERT,
KEY_UNSPEC
};
...
The enum keytypes is defined in file sshkey.c [2] as
...
/* Supported key types */
struct keytype {
const char *name;
const char *shortname;
const char *sigalg;
int type;
int nid;
int cert;
int sigonly;
};
static const struct keytype keytypes[] = {
{ "ssh-ed25519", "ED25519", NULL, KEY_ED25519, 0, 0, 0 },
{ "ssh-ed25519-cert-...@openssh.com", "ED25519-CERT", NULL,
KEY_ED25519_CERT, 0, 1, 0 },
...
Since "ssh-ed25519" (KEY_ED25519) has cert=0, the sshkey_is_cert
function returns 0.
Now, the notify_hostkeys function in the file sshd.c [4] contains
...
for (i = nkeys = 0; i < options.num_host_key_files; i++) {
key = get_hostkey_public_by_index(i, ssh);
if (key == NULL || key->type == KEY_UNSPEC ||
sshkey_is_cert(key))
continue;
...
which implies that the ssh daemon does not seem to send the ssh
host certificate keys as part of the hostkeys-00 message.
Further, the client_input_hostkeys function in the file clientloop.c [5]
contains
...
/* Skip certs */
if (sshkey_is_cert(key)) {
debug3("%s: %s key is a certificate; skipping",
__func__, sshkey_ssh_name(key));
continue;
}
...
which implies that the ssh client seems to be skipping the ssh host
certificate keys.
So, there does not seem to be any way to satisfy the sshkey_is_cert
function check in the patch, because from what I understand, the server
does not seem to send the certificate keys and the client skips the
certificate keys before iterating through the known_hosts file.
I am out of ideas beyond this. I am sorry. If there is a stupid mistake
that I am making here, please let me know.
Dhanyavaad,
ab
[1] -
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/hostfile.c?rev=1.77&content-type=text/x-cvsweb-markup
[2] -
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/sshkey.c?rev=1.99&content-type=text/x-cvsweb-markup
[3] -
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/sshkey.h?rev=1.44&content-type=text/x-cvsweb-markup
[4] -
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/sshd.c?rev=1.549&content-type=text/x-cvsweb-markup
[5] -
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/clientloop.c?rev=1.340&content-type=text/x-cvsweb-markup
---------|---------|---------|---------|---------|---------|---------|--
