upstream: ensure that certificate extensions are lexically sorted.

Previously if the user specified a custom extension then the everything would
be in order except the custom ones. bz3198 ok dtucker markus

OpenBSD-Commit-ID: d97deb90587b06cb227c66ffebb2d9667bf886f0
This commit is contained in:
djm@openbsd.org 2020-08-03 02:53:51 +00:00 committed by Damien Miller
parent a8732d74cb
commit 2d8a3b7e8b

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssh-keygen.c,v 1.414 2020/07/15 07:50:46 solene Exp $ */ /* $OpenBSD: ssh-keygen.c,v 1.415 2020/08/03 02:53:51 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -133,13 +133,13 @@ static char *certflags_command = NULL;
static char *certflags_src_addr = NULL; static char *certflags_src_addr = NULL;
/* Arbitrary extensions specified by user */ /* Arbitrary extensions specified by user */
struct cert_userext { struct cert_ext {
char *key; char *key;
char *val; char *val;
int crit; int crit;
}; };
static struct cert_userext *cert_userext; static struct cert_ext *cert_ext;
static size_t ncert_userext; static size_t ncert_ext;
/* Conversion to/from various formats */ /* Conversion to/from various formats */
enum { enum {
@ -1601,31 +1601,32 @@ do_change_comment(struct passwd *pw, const char *identity_comment)
} }
static void static void
add_flag_option(struct sshbuf *c, const char *name) cert_ext_add(const char *key, const char *value, int iscrit)
{ {
int r; cert_ext = xreallocarray(cert_ext, ncert_ext + 1, sizeof(*cert_ext));
cert_ext[ncert_ext].key = xstrdup(key);
debug3("%s: %s", __func__, name); cert_ext[ncert_ext].val = value == NULL ? NULL : xstrdup(value);
if ((r = sshbuf_put_cstring(c, name)) != 0 || cert_ext[ncert_ext].crit = iscrit;
(r = sshbuf_put_string(c, NULL, 0)) != 0) ncert_ext++;
fatal("%s: buffer error: %s", __func__, ssh_err(r));
} }
static void /* qsort(3) comparison function for certificate extensions */
add_string_option(struct sshbuf *c, const char *name, const char *value) static int
cert_ext_cmp(const void *_a, const void *_b)
{ {
struct sshbuf *b; const struct cert_ext *a = (const struct cert_ext *)_a;
const struct cert_ext *b = (const struct cert_ext *)_b;
int r; int r;
debug3("%s: %s=%s", __func__, name, value); if (a->crit != b->crit)
if ((b = sshbuf_new()) == NULL) return (a->crit < b->crit) ? -1 : 1;
fatal("%s: sshbuf_new failed", __func__); if ((r = strcmp(a->key, b->key)) != 0)
if ((r = sshbuf_put_cstring(b, value)) != 0 || return r;
(r = sshbuf_put_cstring(c, name)) != 0 || if ((a->val == NULL) != (b->val == NULL))
(r = sshbuf_put_stringb(c, b)) != 0) return (a->val == NULL) ? -1 : 1;
fatal("%s: buffer error: %s", __func__, ssh_err(r)); if (a->val != NULL && (r = strcmp(a->val, b->val)) != 0)
return r;
sshbuf_free(b); return 0;
} }
#define OPTIONS_CRITICAL 1 #define OPTIONS_CRITICAL 1
@ -1633,44 +1634,62 @@ add_string_option(struct sshbuf *c, const char *name, const char *value)
static void static void
prepare_options_buf(struct sshbuf *c, int which) prepare_options_buf(struct sshbuf *c, int which)
{ {
struct sshbuf *b;
size_t i; size_t i;
int r;
const struct cert_ext *ext;
if ((b = sshbuf_new()) == NULL)
fatal("%s: sshbuf_new failed", __func__);
sshbuf_reset(c); sshbuf_reset(c);
if ((which & OPTIONS_CRITICAL) != 0 && for (i = 0; i < ncert_ext; i++) {
certflags_command != NULL) ext = &cert_ext[i];
add_string_option(c, "force-command", certflags_command); if ((ext->crit && (which & OPTIONS_EXTENSIONS)) ||
if ((which & OPTIONS_EXTENSIONS) != 0 && (!ext->crit && (which & OPTIONS_CRITICAL)))
(certflags_flags & CERTOPT_X_FWD) != 0)
add_flag_option(c, "permit-X11-forwarding");
if ((which & OPTIONS_EXTENSIONS) != 0 &&
(certflags_flags & CERTOPT_AGENT_FWD) != 0)
add_flag_option(c, "permit-agent-forwarding");
if ((which & OPTIONS_EXTENSIONS) != 0 &&
(certflags_flags & CERTOPT_PORT_FWD) != 0)
add_flag_option(c, "permit-port-forwarding");
if ((which & OPTIONS_EXTENSIONS) != 0 &&
(certflags_flags & CERTOPT_PTY) != 0)
add_flag_option(c, "permit-pty");
if ((which & OPTIONS_EXTENSIONS) != 0 &&
(certflags_flags & CERTOPT_USER_RC) != 0)
add_flag_option(c, "permit-user-rc");
if ((which & OPTIONS_EXTENSIONS) != 0 &&
(certflags_flags & CERTOPT_NO_REQUIRE_USER_PRESENCE) != 0)
add_flag_option(c, "no-touch-required");
if ((which & OPTIONS_CRITICAL) != 0 &&
certflags_src_addr != NULL)
add_string_option(c, "source-address", certflags_src_addr);
for (i = 0; i < ncert_userext; i++) {
if ((cert_userext[i].crit && (which & OPTIONS_EXTENSIONS)) ||
(!cert_userext[i].crit && (which & OPTIONS_CRITICAL)))
continue; continue;
if (cert_userext[i].val == NULL) if (ext->val == NULL) {
add_flag_option(c, cert_userext[i].key); /* flag option */
else { debug3("%s: %s", __func__, ext->key);
add_string_option(c, cert_userext[i].key, if ((r = sshbuf_put_cstring(c, ext->key)) != 0 ||
cert_userext[i].val); (r = sshbuf_put_string(c, NULL, 0)) != 0)
fatal("%s: buffer: %s", __func__, ssh_err(r));
} else {
/* key/value option */
debug3("%s: %s=%s", __func__, ext->key, ext->val);
sshbuf_reset(b);
if ((r = sshbuf_put_cstring(c, ext->key)) != 0 ||
(r = sshbuf_put_cstring(b, ext->val)) != 0 ||
(r = sshbuf_put_stringb(c, b)) != 0)
fatal("%s: buffer: %s", __func__, ssh_err(r));
} }
} }
sshbuf_free(b);
}
static void
finalise_cert_exts(void)
{
/* critical options */
if (certflags_command != NULL)
cert_ext_add("force-command", certflags_command, 1);
if (certflags_src_addr != NULL)
cert_ext_add("source-address", certflags_src_addr, 1);
/* extensions */
if ((certflags_flags & CERTOPT_X_FWD) != 0)
cert_ext_add("permit-X11-forwarding", NULL, 0);
if ((certflags_flags & CERTOPT_AGENT_FWD) != 0)
cert_ext_add("permit-agent-forwarding", NULL, 0);
if ((certflags_flags & CERTOPT_PORT_FWD) != 0)
cert_ext_add("permit-port-forwarding", NULL, 0);
if ((certflags_flags & CERTOPT_PTY) != 0)
cert_ext_add("permit-pty", NULL, 0);
if ((certflags_flags & CERTOPT_USER_RC) != 0)
cert_ext_add("permit-user-rc", NULL, 0);
if ((certflags_flags & CERTOPT_NO_REQUIRE_USER_PRESENCE) != 0)
cert_ext_add("no-touch-required", NULL, 0);
/* order lexically by key */
if (ncert_ext > 0)
qsort(cert_ext, ncert_ext, sizeof(*cert_ext), cert_ext_cmp);
} }
static struct sshkey * static struct sshkey *
@ -1780,6 +1799,7 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
} }
ca_fp = sshkey_fingerprint(ca, fingerprint_hash, SSH_FP_DEFAULT); ca_fp = sshkey_fingerprint(ca, fingerprint_hash, SSH_FP_DEFAULT);
finalise_cert_exts();
for (i = 0; i < argc; i++) { for (i = 0; i < argc; i++) {
/* Split list of principals */ /* Split list of principals */
n = 0; n = 0;
@ -1994,13 +2014,8 @@ add_cert_option(char *opt)
val = xstrdup(strchr(opt, ':') + 1); val = xstrdup(strchr(opt, ':') + 1);
if ((cp = strchr(val, '=')) != NULL) if ((cp = strchr(val, '=')) != NULL)
*cp++ = '\0'; *cp++ = '\0';
cert_userext = xreallocarray(cert_userext, ncert_userext + 1, cert_ext_add(val, cp, iscrit);
sizeof(*cert_userext)); free(val);
cert_userext[ncert_userext].key = val;
cert_userext[ncert_userext].val = cp == NULL ?
NULL : xstrdup(cp);
cert_userext[ncert_userext].crit = iscrit;
ncert_userext++;
} else } else
fatal("Unsupported certificate option \"%s\"", opt); fatal("Unsupported certificate option \"%s\"", opt);
} }
@ -2008,7 +2023,7 @@ add_cert_option(char *opt)
static void static void
show_options(struct sshbuf *optbuf, int in_critical) show_options(struct sshbuf *optbuf, int in_critical)
{ {
char *name, *arg; char *name, *arg, *hex;
struct sshbuf *options, *option = NULL; struct sshbuf *options, *option = NULL;
int r; int r;
@ -2037,11 +2052,14 @@ show_options(struct sshbuf *optbuf, int in_critical)
__func__, ssh_err(r)); __func__, ssh_err(r));
printf(" %s\n", arg); printf(" %s\n", arg);
free(arg); free(arg);
} else { } else if (sshbuf_len(option) > 0) {
printf(" UNKNOWN OPTION (len %zu)\n", hex = sshbuf_dtob16(option);
sshbuf_len(option)); printf(" UNKNOWN OPTION: %s (len %zu)\n",
hex, sshbuf_len(option));
sshbuf_reset(option); sshbuf_reset(option);
} free(hex);
} else
printf(" UNKNOWN FLAG OPTION\n");
free(name); free(name);
if (sshbuf_len(option) != 0) if (sshbuf_len(option) != 0)
fatal("Option corrupt: extra data at end"); fatal("Option corrupt: extra data at end");