From: djm@openbsd.org Date: Mon, 3 Aug 2020 02:53:51 +0000 (+0000) Subject: upstream: ensure that certificate extensions are lexically sorted. X-Git-Tag: V_8_4_P1~56 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2d8a3b7e8b0408dfeb933ac5cfd3a58f5bac49af;p=thirdparty%2Fopenssh-portable.git 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 --- diff --git a/ssh-keygen.c b/ssh-keygen.c index 4a27d3bfd..cc092368e 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -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 * Copyright (c) 1994 Tatu Ylonen , Espoo, Finland @@ -133,13 +133,13 @@ static char *certflags_command = NULL; static char *certflags_src_addr = NULL; /* Arbitrary extensions specified by user */ -struct cert_userext { +struct cert_ext { char *key; char *val; int crit; }; -static struct cert_userext *cert_userext; -static size_t ncert_userext; +static struct cert_ext *cert_ext; +static size_t ncert_ext; /* Conversion to/from various formats */ enum { @@ -1601,31 +1601,32 @@ do_change_comment(struct passwd *pw, const char *identity_comment) } static void -add_flag_option(struct sshbuf *c, const char *name) +cert_ext_add(const char *key, const char *value, int iscrit) { - int r; - - debug3("%s: %s", __func__, name); - if ((r = sshbuf_put_cstring(c, name)) != 0 || - (r = sshbuf_put_string(c, NULL, 0)) != 0) - fatal("%s: buffer error: %s", __func__, ssh_err(r)); + cert_ext = xreallocarray(cert_ext, ncert_ext + 1, sizeof(*cert_ext)); + cert_ext[ncert_ext].key = xstrdup(key); + cert_ext[ncert_ext].val = value == NULL ? NULL : xstrdup(value); + cert_ext[ncert_ext].crit = iscrit; + ncert_ext++; } -static void -add_string_option(struct sshbuf *c, const char *name, const char *value) +/* qsort(3) comparison function for certificate extensions */ +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; - debug3("%s: %s=%s", __func__, name, value); - if ((b = sshbuf_new()) == NULL) - fatal("%s: sshbuf_new failed", __func__); - if ((r = sshbuf_put_cstring(b, value)) != 0 || - (r = sshbuf_put_cstring(c, name)) != 0 || - (r = sshbuf_put_stringb(c, b)) != 0) - fatal("%s: buffer error: %s", __func__, ssh_err(r)); - - sshbuf_free(b); + if (a->crit != b->crit) + return (a->crit < b->crit) ? -1 : 1; + if ((r = strcmp(a->key, b->key)) != 0) + return r; + if ((a->val == NULL) != (b->val == NULL)) + return (a->val == NULL) ? -1 : 1; + if (a->val != NULL && (r = strcmp(a->val, b->val)) != 0) + return r; + return 0; } #define OPTIONS_CRITICAL 1 @@ -1633,44 +1634,62 @@ add_string_option(struct sshbuf *c, const char *name, const char *value) static void prepare_options_buf(struct sshbuf *c, int which) { + struct sshbuf *b; size_t i; + int r; + const struct cert_ext *ext; + if ((b = sshbuf_new()) == NULL) + fatal("%s: sshbuf_new failed", __func__); sshbuf_reset(c); - if ((which & OPTIONS_CRITICAL) != 0 && - certflags_command != NULL) - add_string_option(c, "force-command", certflags_command); - if ((which & OPTIONS_EXTENSIONS) != 0 && - (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))) + for (i = 0; i < ncert_ext; i++) { + ext = &cert_ext[i]; + if ((ext->crit && (which & OPTIONS_EXTENSIONS)) || + (!ext->crit && (which & OPTIONS_CRITICAL))) continue; - if (cert_userext[i].val == NULL) - add_flag_option(c, cert_userext[i].key); - else { - add_string_option(c, cert_userext[i].key, - cert_userext[i].val); + if (ext->val == NULL) { + /* flag option */ + debug3("%s: %s", __func__, ext->key); + if ((r = sshbuf_put_cstring(c, ext->key)) != 0 || + (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 * @@ -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); + finalise_cert_exts(); for (i = 0; i < argc; i++) { /* Split list of principals */ n = 0; @@ -1994,13 +2014,8 @@ add_cert_option(char *opt) val = xstrdup(strchr(opt, ':') + 1); if ((cp = strchr(val, '=')) != NULL) *cp++ = '\0'; - cert_userext = xreallocarray(cert_userext, ncert_userext + 1, - sizeof(*cert_userext)); - cert_userext[ncert_userext].key = val; - cert_userext[ncert_userext].val = cp == NULL ? - NULL : xstrdup(cp); - cert_userext[ncert_userext].crit = iscrit; - ncert_userext++; + cert_ext_add(val, cp, iscrit); + free(val); } else fatal("Unsupported certificate option \"%s\"", opt); } @@ -2008,7 +2023,7 @@ add_cert_option(char *opt) static void show_options(struct sshbuf *optbuf, int in_critical) { - char *name, *arg; + char *name, *arg, *hex; struct sshbuf *options, *option = NULL; int r; @@ -2037,11 +2052,14 @@ show_options(struct sshbuf *optbuf, int in_critical) __func__, ssh_err(r)); printf(" %s\n", arg); free(arg); - } else { - printf(" UNKNOWN OPTION (len %zu)\n", - sshbuf_len(option)); + } else if (sshbuf_len(option) > 0) { + hex = sshbuf_dtob16(option); + printf(" UNKNOWN OPTION: %s (len %zu)\n", + hex, sshbuf_len(option)); sshbuf_reset(option); - } + free(hex); + } else + printf(" UNKNOWN FLAG OPTION\n"); free(name); if (sshbuf_len(option) != 0) fatal("Option corrupt: extra data at end");