From 4e270b05dd9d850fb9e2e0ac43f33cb4090d3ebc Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 16 Apr 2010 15:56:21 +1000 Subject: - djm@cvs.openbsd.org 2010/04/16 01:47:26 [PROTOCOL.certkeys auth-options.c auth-options.h auth-rsa.c] [auth2-pubkey.c authfd.c key.c key.h myproposal.h ssh-add.c] [ssh-agent.c ssh-dss.c ssh-keygen.1 ssh-keygen.c ssh-rsa.c] [sshconnect.c sshconnect2.c sshd.c] revised certificate format ssh-{dss,rsa}-cert-v01@openssh.com with the following changes: move the nonce field to the beginning of the certificate where it can better protect against chosen-prefix attacks on the signature hash Rename "constraints" field to "critical options" Add a new non-critical "extensions" field Add a serial number The older format is still support for authentication and cert generation (use "ssh-keygen -t v00 -s ca_key ..." to generate a v00 certificate) ok markus@ --- key.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 146 insertions(+), 31 deletions(-) (limited to 'key.c') diff --git a/key.c b/key.c index 66592c7ed..34f678b38 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.86 2010/03/15 19:40:02 stevesk Exp $ */ +/* $OpenBSD: key.c,v 1.87 2010/04/16 01:47:26 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -61,7 +61,8 @@ cert_new(void) cert = xcalloc(1, sizeof(*cert)); buffer_init(&cert->certblob); - buffer_init(&cert->constraints); + buffer_init(&cert->critical); + buffer_init(&cert->extensions); cert->key_id = NULL; cert->principals = NULL; cert->signature_key = NULL; @@ -82,6 +83,7 @@ key_new(int type) switch (k->type) { case KEY_RSA1: case KEY_RSA: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: if ((rsa = RSA_new()) == NULL) fatal("key_new: RSA_new failed"); @@ -92,6 +94,7 @@ key_new(int type) k->rsa = rsa; break; case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: if ((dsa = DSA_new()) == NULL) fatal("key_new: DSA_new failed"); @@ -124,6 +127,7 @@ key_add_private(Key *k) switch (k->type) { case KEY_RSA1: case KEY_RSA: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: if ((k->rsa->d = BN_new()) == NULL) fatal("key_new_private: BN_new failed"); @@ -139,6 +143,7 @@ key_add_private(Key *k) fatal("key_new_private: BN_new failed"); break; case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: if ((k->dsa->priv_key = BN_new()) == NULL) fatal("key_new_private: BN_new failed"); @@ -165,7 +170,8 @@ cert_free(struct KeyCert *cert) u_int i; buffer_free(&cert->certblob); - buffer_free(&cert->constraints); + buffer_free(&cert->critical); + buffer_free(&cert->extensions); if (cert->key_id != NULL) xfree(cert->key_id); for (i = 0; i < cert->nprincipals; i++) @@ -184,12 +190,14 @@ key_free(Key *k) switch (k->type) { case KEY_RSA1: case KEY_RSA: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: if (k->rsa != NULL) RSA_free(k->rsa); k->rsa = NULL; break; case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: if (k->dsa != NULL) DSA_free(k->dsa); @@ -238,11 +246,13 @@ key_equal_public(const Key *a, const Key *b) switch (a->type) { case KEY_RSA1: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: case KEY_RSA: return a->rsa != NULL && b->rsa != NULL && BN_cmp(a->rsa->e, b->rsa->e) == 0 && BN_cmp(a->rsa->n, b->rsa->n) == 0; + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: case KEY_DSA: return a->dsa != NULL && b->dsa != NULL && @@ -304,6 +314,8 @@ key_fingerprint_raw(Key *k, enum fp_type dgst_type, u_int *dgst_raw_length) case KEY_RSA: key_to_blob(k, &blob, &len); break; + case KEY_DSA_CERT_V00: + case KEY_RSA_CERT_V00: case KEY_DSA_CERT: case KEY_RSA_CERT: /* We want a fingerprint of the _key_ not of the cert */ @@ -631,6 +643,8 @@ key_read(Key *ret, char **cpp) case KEY_UNSPEC: case KEY_RSA: case KEY_DSA: + case KEY_DSA_CERT_V00: + case KEY_RSA_CERT_V00: case KEY_DSA_CERT: case KEY_RSA_CERT: space = strchr(cp, ' '); @@ -757,11 +771,13 @@ key_write(const Key *key, FILE *f) error("key_write: failed for RSA key"); return 0; case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: if (key->dsa == NULL) return 0; break; case KEY_RSA: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: if (key->rsa == NULL) return 0; @@ -793,6 +809,10 @@ key_type(const Key *k) return "RSA"; case KEY_DSA: return "DSA"; + case KEY_RSA_CERT_V00: + return "RSA-CERT-V00"; + case KEY_DSA_CERT_V00: + return "DSA-CERT-V00"; case KEY_RSA_CERT: return "RSA-CERT"; case KEY_DSA_CERT: @@ -822,10 +842,14 @@ key_ssh_name(const Key *k) return "ssh-rsa"; case KEY_DSA: return "ssh-dss"; - case KEY_RSA_CERT: + case KEY_RSA_CERT_V00: return "ssh-rsa-cert-v00@openssh.com"; - case KEY_DSA_CERT: + case KEY_DSA_CERT_V00: return "ssh-dss-cert-v00@openssh.com"; + case KEY_RSA_CERT: + return "ssh-rsa-cert-v01@openssh.com"; + case KEY_DSA_CERT: + return "ssh-dss-cert-v01@openssh.com"; } return "ssh-unknown"; } @@ -836,9 +860,11 @@ key_size(const Key *k) switch (k->type) { case KEY_RSA1: case KEY_RSA: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: return BN_num_bits(k->rsa->n); case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: return BN_num_bits(k->dsa->p); } @@ -882,6 +908,8 @@ key_generate(int type, u_int bits) case KEY_RSA1: k->rsa = rsa_generate_private_key(bits); break; + case KEY_RSA_CERT_V00: + case KEY_DSA_CERT_V00: case KEY_RSA_CERT: case KEY_DSA_CERT: fatal("key_generate: cert keys cannot be generated directly"); @@ -912,9 +940,12 @@ key_cert_copy(const Key *from_key, struct Key *to_key) buffer_append(&to->certblob, buffer_ptr(&from->certblob), buffer_len(&from->certblob)); - buffer_append(&to->constraints, buffer_ptr(&from->constraints), - buffer_len(&from->constraints)); + buffer_append(&to->critical, + buffer_ptr(&from->critical), buffer_len(&from->critical)); + buffer_append(&to->extensions, + buffer_ptr(&from->extensions), buffer_len(&from->extensions)); + to->serial = from->serial; to->type = from->type; to->key_id = from->key_id == NULL ? NULL : xstrdup(from->key_id); to->valid_after = from->valid_after; @@ -940,6 +971,7 @@ key_from_private(const Key *k) Key *n = NULL; switch (k->type) { case KEY_DSA: + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: n = key_new(k->type); if ((BN_copy(n->dsa->p, k->dsa->p) == NULL) || @@ -950,6 +982,7 @@ key_from_private(const Key *k) break; case KEY_RSA: case KEY_RSA1: + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: n = key_new(k->type); if ((BN_copy(n->rsa->n, k->rsa->n) == NULL) || @@ -979,8 +1012,12 @@ key_type_from_name(char *name) } else if (strcmp(name, "ssh-dss") == 0) { return KEY_DSA; } else if (strcmp(name, "ssh-rsa-cert-v00@openssh.com") == 0) { - return KEY_RSA_CERT; + return KEY_RSA_CERT_V00; } else if (strcmp(name, "ssh-dss-cert-v00@openssh.com") == 0) { + return KEY_DSA_CERT_V00; + } else if (strcmp(name, "ssh-rsa-cert-v01@openssh.com") == 0) { + return KEY_RSA_CERT; + } else if (strcmp(name, "ssh-dss-cert-v01@openssh.com") == 0) { return KEY_DSA_CERT; } debug2("key_type_from_name: unknown key type '%s'", name); @@ -1012,26 +1049,31 @@ key_names_valid2(const char *names) static int cert_parse(Buffer *b, Key *key, const u_char *blob, u_int blen) { - u_char *principals, *constraints, *sig_key, *sig; - u_int signed_len, plen, clen, sklen, slen, kidlen; + u_char *principals, *critical, *exts, *sig_key, *sig; + u_int signed_len, plen, clen, sklen, slen, kidlen, elen; Buffer tmp; char *principal; int ret = -1; + int v00 = key->type == KEY_DSA_CERT_V00 || + key->type == KEY_RSA_CERT_V00; buffer_init(&tmp); /* Copy the entire key blob for verification and later serialisation */ buffer_append(&key->cert->certblob, blob, blen); - principals = constraints = sig_key = sig = NULL; - if (buffer_get_int_ret(&key->cert->type, b) != 0 || + elen = 0; /* Not touched for v00 certs */ + principals = exts = critical = sig_key = sig = NULL; + if ((!v00 && buffer_get_int64_ret(&key->cert->serial, b) != 0) || + buffer_get_int_ret(&key->cert->type, b) != 0 || (key->cert->key_id = buffer_get_string_ret(b, &kidlen)) == NULL || (principals = buffer_get_string_ret(b, &plen)) == NULL || buffer_get_int64_ret(&key->cert->valid_after, b) != 0 || buffer_get_int64_ret(&key->cert->valid_before, b) != 0 || - (constraints = buffer_get_string_ret(b, &clen)) == NULL || - /* skip nonce */ buffer_get_string_ptr_ret(b, NULL) == NULL || - /* skip reserved */ buffer_get_string_ptr_ret(b, NULL) == NULL || + (critical = buffer_get_string_ret(b, &clen)) == NULL || + (!v00 && (exts = buffer_get_string_ret(b, &elen)) == NULL) || + (v00 && buffer_get_string_ptr_ret(b, NULL) == NULL) || /* nonce */ + buffer_get_string_ptr_ret(b, NULL) == NULL || /* reserved */ (sig_key = buffer_get_string_ret(b, &sklen)) == NULL) { error("%s: parse error", __func__); goto out; @@ -1078,13 +1120,25 @@ cert_parse(Buffer *b, Key *key, const u_char *blob, u_int blen) buffer_clear(&tmp); - buffer_append(&key->cert->constraints, constraints, clen); - buffer_append(&tmp, constraints, clen); + buffer_append(&key->cert->critical, critical, clen); + buffer_append(&tmp, critical, clen); /* validate structure */ while (buffer_len(&tmp) != 0) { if (buffer_get_string_ptr_ret(&tmp, NULL) == NULL || buffer_get_string_ptr_ret(&tmp, NULL) == NULL) { - error("%s: Constraints data invalid", __func__); + error("%s: critical option data invalid", __func__); + goto out; + } + } + buffer_clear(&tmp); + + buffer_append(&key->cert->extensions, exts, elen); + buffer_append(&tmp, exts, elen); + /* validate structure */ + while (buffer_len(&tmp) != 0) { + if (buffer_get_string_ptr_ret(&tmp, NULL) == NULL || + buffer_get_string_ptr_ret(&tmp, NULL) == NULL) { + error("%s: extension data invalid", __func__); goto out; } } @@ -1121,8 +1175,10 @@ cert_parse(Buffer *b, Key *key, const u_char *blob, u_int blen) buffer_free(&tmp); if (principals != NULL) xfree(principals); - if (constraints != NULL) - xfree(constraints); + if (critical != NULL) + xfree(critical); + if (exts != NULL) + xfree(exts); if (sig_key != NULL) xfree(sig_key); if (sig != NULL) @@ -1151,8 +1207,11 @@ key_from_blob(const u_char *blob, u_int blen) type = key_type_from_name(ktype); switch (type) { - case KEY_RSA: case KEY_RSA_CERT: + (void)buffer_get_string_ptr_ret(&b, NULL); /* Skip nonce */ + /* FALLTHROUGH */ + case KEY_RSA: + case KEY_RSA_CERT_V00: key = key_new(type); if (buffer_get_bignum2_ret(&b, key->rsa->e) == -1 || buffer_get_bignum2_ret(&b, key->rsa->n) == -1) { @@ -1166,8 +1225,11 @@ key_from_blob(const u_char *blob, u_int blen) RSA_print_fp(stderr, key->rsa, 8); #endif break; - case KEY_DSA: case KEY_DSA_CERT: + (void)buffer_get_string_ptr_ret(&b, NULL); /* Skip nonce */ + /* FALLTHROUGH */ + case KEY_DSA: + case KEY_DSA_CERT_V00: key = key_new(type); if (buffer_get_bignum2_ret(&b, key->dsa->p) == -1 || buffer_get_bignum2_ret(&b, key->dsa->q) == -1 || @@ -1213,6 +1275,8 @@ key_to_blob(const Key *key, u_char **blobp, u_int *lenp) } buffer_init(&b); switch (key->type) { + case KEY_DSA_CERT_V00: + case KEY_RSA_CERT_V00: case KEY_DSA_CERT: case KEY_RSA_CERT: /* Use the existing blob */ @@ -1255,9 +1319,11 @@ key_sign( const u_char *data, u_int datalen) { switch (key->type) { + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: case KEY_DSA: return ssh_dss_sign(key, sigp, lenp, data, datalen); + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: case KEY_RSA: return ssh_rsa_sign(key, sigp, lenp, data, datalen); @@ -1281,9 +1347,11 @@ key_verify( return -1; switch (key->type) { + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: case KEY_DSA: return ssh_dss_verify(key, signature, signaturelen, data, datalen); + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: case KEY_RSA: return ssh_rsa_verify(key, signature, signaturelen, data, datalen); @@ -1306,6 +1374,7 @@ key_demote(const Key *k) pk->rsa = NULL; switch (k->type) { + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: key_cert_copy(k, pk); /* FALLTHROUGH */ @@ -1318,6 +1387,7 @@ key_demote(const Key *k) if ((pk->rsa->n = BN_dup(k->rsa->n)) == NULL) fatal("key_demote: BN_dup failed"); break; + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: key_cert_copy(k, pk); /* FALLTHROUGH */ @@ -1344,8 +1414,17 @@ key_demote(const Key *k) int key_is_cert(const Key *k) { - return k != NULL && - (k->type == KEY_RSA_CERT || k->type == KEY_DSA_CERT); + if (k == NULL) + return 0; + switch (k->type) { + case KEY_RSA_CERT_V00: + case KEY_DSA_CERT_V00: + case KEY_RSA_CERT: + case KEY_DSA_CERT: + return 1; + default: + return 0; + } } /* Return the cert-less equivalent to a certified key type */ @@ -1353,8 +1432,10 @@ int key_type_plain(int type) { switch (type) { + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: return KEY_RSA; + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: return KEY_DSA; default: @@ -1364,16 +1445,16 @@ key_type_plain(int type) /* Convert a KEY_RSA or KEY_DSA to their _CERT equivalent */ int -key_to_certified(Key *k) +key_to_certified(Key *k, int legacy) { switch (k->type) { case KEY_RSA: k->cert = cert_new(); - k->type = KEY_RSA_CERT; + k->type = legacy ? KEY_RSA_CERT_V00 : KEY_RSA_CERT; return 0; case KEY_DSA: k->cert = cert_new(); - k->type = KEY_DSA_CERT; + k->type = legacy ? KEY_DSA_CERT_V00 : KEY_DSA_CERT; return 0; default: error("%s: key has incorrect type %s", __func__, key_type(k)); @@ -1386,10 +1467,12 @@ int key_drop_cert(Key *k) { switch (k->type) { + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: cert_free(k->cert); k->type = KEY_RSA; return 0; + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: cert_free(k->cert); k->type = KEY_DSA; @@ -1430,13 +1513,21 @@ key_certify(Key *k, Key *ca) buffer_clear(&k->cert->certblob); buffer_put_cstring(&k->cert->certblob, key_ssh_name(k)); + /* -v01 certs put nonce first */ + if (k->type == KEY_DSA_CERT || k->type == KEY_RSA_CERT) { + arc4random_buf(&nonce, sizeof(nonce)); + buffer_put_string(&k->cert->certblob, nonce, sizeof(nonce)); + } + switch (k->type) { + case KEY_DSA_CERT_V00: case KEY_DSA_CERT: buffer_put_bignum2(&k->cert->certblob, k->dsa->p); buffer_put_bignum2(&k->cert->certblob, k->dsa->q); buffer_put_bignum2(&k->cert->certblob, k->dsa->g); buffer_put_bignum2(&k->cert->certblob, k->dsa->pub_key); break; + case KEY_RSA_CERT_V00: case KEY_RSA_CERT: buffer_put_bignum2(&k->cert->certblob, k->rsa->e); buffer_put_bignum2(&k->cert->certblob, k->rsa->n); @@ -1448,6 +1539,10 @@ key_certify(Key *k, Key *ca) return -1; } + /* -v01 certs have a serial number next */ + if (k->type == KEY_DSA_CERT || k->type == KEY_RSA_CERT) + buffer_put_int64(&k->cert->certblob, k->cert->serial); + buffer_put_int(&k->cert->certblob, k->cert->type); buffer_put_cstring(&k->cert->certblob, k->cert->key_id); @@ -1461,11 +1556,19 @@ key_certify(Key *k, Key *ca) buffer_put_int64(&k->cert->certblob, k->cert->valid_after); buffer_put_int64(&k->cert->certblob, k->cert->valid_before); buffer_put_string(&k->cert->certblob, - buffer_ptr(&k->cert->constraints), - buffer_len(&k->cert->constraints)); + buffer_ptr(&k->cert->critical), buffer_len(&k->cert->critical)); + + /* -v01 certs have non-critical options here */ + if (k->type == KEY_DSA_CERT || k->type == KEY_RSA_CERT) { + buffer_put_string(&k->cert->certblob, + buffer_ptr(&k->cert->extensions), + buffer_len(&k->cert->extensions)); + } + + /* -v00 certs put the nonce at the end */ + if (k->type == KEY_DSA_CERT_V00 || k->type == KEY_RSA_CERT_V00) + buffer_put_string(&k->cert->certblob, nonce, sizeof(nonce)); - arc4random_buf(&nonce, sizeof(nonce)); - buffer_put_string(&k->cert->certblob, nonce, sizeof(nonce)); buffer_put_string(&k->cert->certblob, NULL, 0); /* reserved */ buffer_put_string(&k->cert->certblob, ca_blob, ca_len); xfree(ca_blob); @@ -1536,3 +1639,15 @@ key_cert_check_authority(const Key *k, int want_host, int require_principal, } return 0; } + +int +key_cert_is_legacy(Key *k) +{ + switch (k->type) { + case KEY_DSA_CERT_V00: + case KEY_RSA_CERT_V00: + return 1; + default: + return 0; + } +} -- cgit v1.2.3 From 30da3447d2ef3329cb0eb083cdddf84532659454 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Mon, 10 May 2010 11:58:03 +1000 Subject: - djm@cvs.openbsd.org 2010/05/07 11:30:30 [auth-options.c auth-options.h auth.c auth.h auth2-pubkey.c] [key.c servconf.c servconf.h sshd.8 sshd_config.5] add some optional indirection to matching of principal names listed in certificates. Currently, a certificate must include the a user's name to be accepted for authentication. This change adds the ability to specify a list of certificate principal names that are acceptable. When authenticating using a CA trusted through ~/.ssh/authorized_keys, this adds a new principals="name1[,name2,...]" key option. For CAs listed through sshd_config's TrustedCAKeys option, a new config option "AuthorizedPrincipalsFile" specifies a per-user file containing the list of acceptable names. If either option is absent, the current behaviour of requiring the username to appear in principals continues to apply. These options are useful for role accounts, disjoint account namespaces and "user@realm"-style naming policies in certificates. feedback and ok markus@ --- ChangeLog | 22 +++++++++++++ auth-options.c | 43 +++++++++++++++++++++++- auth-options.h | 3 +- auth.c | 41 ++++++++++++++++------- auth.h | 4 ++- auth2-pubkey.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- key.c | 4 +-- servconf.c | 18 +++++++--- servconf.h | 3 +- sshd.8 | 15 +++++++-- sshd_config.5 | 41 +++++++++++++++++++++-- 11 files changed, 262 insertions(+), 34 deletions(-) (limited to 'key.c') diff --git a/ChangeLog b/ChangeLog index 684609459..090e2352c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -31,6 +31,28 @@ [sftp.c] restore mput and mget which got lost in the tab-completion changes. found by Kenneth Whitaker, ok djm@ + - djm@cvs.openbsd.org 2010/05/07 11:30:30 + [auth-options.c auth-options.h auth.c auth.h auth2-pubkey.c] + [key.c servconf.c servconf.h sshd.8 sshd_config.5] + add some optional indirection to matching of principal names listed + in certificates. Currently, a certificate must include the a user's name + to be accepted for authentication. This change adds the ability to + specify a list of certificate principal names that are acceptable. + + When authenticating using a CA trusted through ~/.ssh/authorized_keys, + this adds a new principals="name1[,name2,...]" key option. + + For CAs listed through sshd_config's TrustedCAKeys option, a new config + option "AuthorizedPrincipalsFile" specifies a per-user file containing + the list of acceptable names. + + If either option is absent, the current behaviour of requiring the + username to appear in principals continues to apply. + + These options are useful for role accounts, disjoint account namespaces + and "user@realm"-style naming policies in certificates. + + feedback and ok markus@ 20100423 - (dtucker) [configure.ac] Bug #1756: Check for the existence of a lib64 dir diff --git a/auth-options.c b/auth-options.c index 60d5f749b..57a67ec79 100644 --- a/auth-options.c +++ b/auth-options.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.c,v 1.50 2010/04/16 01:47:26 djm Exp $ */ +/* $OpenBSD: auth-options.c,v 1.51 2010/05/07 11:30:29 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -55,6 +55,9 @@ struct envstring *custom_environment = NULL; /* "tunnel=" option. */ int forced_tun_device = -1; +/* "principals=" option. */ +char *authorized_principals = NULL; + extern ServerOptions options; void @@ -76,6 +79,10 @@ auth_clear_options(void) xfree(forced_command); forced_command = NULL; } + if (authorized_principals) { + xfree(authorized_principals); + authorized_principals = NULL; + } forced_tun_device = -1; channel_clear_permitted_opens(); } @@ -141,6 +148,8 @@ auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) cp = "command=\""; if (strncasecmp(opts, cp, strlen(cp)) == 0) { opts += strlen(cp); + if (forced_command != NULL) + xfree(forced_command); forced_command = xmalloc(strlen(opts) + 1); i = 0; while (*opts) { @@ -167,6 +176,38 @@ auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) opts++; goto next_option; } + cp = "principals=\""; + if (strncasecmp(opts, cp, strlen(cp)) == 0) { + opts += strlen(cp); + if (authorized_principals != NULL) + xfree(authorized_principals); + authorized_principals = xmalloc(strlen(opts) + 1); + i = 0; + while (*opts) { + if (*opts == '"') + break; + if (*opts == '\\' && opts[1] == '"') { + opts += 2; + authorized_principals[i++] = '"'; + continue; + } + authorized_principals[i++] = *opts++; + } + if (!*opts) { + debug("%.100s, line %lu: missing end quote", + file, linenum); + auth_debug_add("%.100s, line %lu: missing end quote", + file, linenum); + xfree(authorized_principals); + authorized_principals = NULL; + goto bad_option; + } + authorized_principals[i] = '\0'; + auth_debug_add("principals: %.900s", + authorized_principals); + opts++; + goto next_option; + } cp = "environment=\""; if (options.permit_user_env && strncasecmp(opts, cp, strlen(cp)) == 0) { diff --git a/auth-options.h b/auth-options.h index 20f0dbe3e..7455c9454 100644 --- a/auth-options.h +++ b/auth-options.h @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.h,v 1.19 2010/04/16 01:47:26 djm Exp $ */ +/* $OpenBSD: auth-options.h,v 1.20 2010/05/07 11:30:29 djm Exp $ */ /* * Author: Tatu Ylonen @@ -31,6 +31,7 @@ extern char *forced_command; extern struct envstring *custom_environment; extern int forced_tun_device; extern int key_is_cert_authority; +extern char *authorized_principals; int auth_parse_options(struct passwd *, char *, char *, u_long); void auth_clear_options(void); diff --git a/auth.c b/auth.c index 89a936068..bec191a59 100644 --- a/auth.c +++ b/auth.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth.c,v 1.86 2010/03/05 02:58:11 djm Exp $ */ +/* $OpenBSD: auth.c,v 1.87 2010/05/07 11:30:29 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -366,6 +366,14 @@ authorized_keys_file2(struct passwd *pw) return expand_authorized_keys(options.authorized_keys_file2, pw); } +char * +authorized_principals_file(struct passwd *pw) +{ + if (options.authorized_principals_file == NULL) + return NULL; + return expand_authorized_keys(options.authorized_principals_file, pw); +} + /* return ok if key exists in sysfile or userfile */ HostStatus check_key_in_hostfiles(struct passwd *pw, Key *key, const char *host, @@ -477,21 +485,18 @@ secure_filename(FILE *f, const char *file, struct passwd *pw, return 0; } -FILE * -auth_openkeyfile(const char *file, struct passwd *pw, int strict_modes) +static FILE * +auth_openfile(const char *file, struct passwd *pw, int strict_modes, + int log_missing, char *file_type) { char line[1024]; struct stat st; int fd; FILE *f; - /* - * Open the file containing the authorized keys - * Fail quietly if file does not exist - */ if ((fd = open(file, O_RDONLY|O_NONBLOCK)) == -1) { - if (errno != ENOENT) - debug("Could not open keyfile '%s': %s", file, + if (log_missing || errno != ENOENT) + debug("Could not open %s '%s': %s", file_type, file, strerror(errno)); return NULL; } @@ -501,8 +506,8 @@ auth_openkeyfile(const char *file, struct passwd *pw, int strict_modes) return NULL; } if (!S_ISREG(st.st_mode)) { - logit("User %s authorized keys %s is not a regular file", - pw->pw_name, file); + logit("User %s %s %s is not a regular file", + pw->pw_name, file_type, file); close(fd); return NULL; } @@ -521,6 +526,20 @@ auth_openkeyfile(const char *file, struct passwd *pw, int strict_modes) return f; } + +FILE * +auth_openkeyfile(const char *file, struct passwd *pw, int strict_modes) +{ + return auth_openfile(file, pw, strict_modes, 1, "authorized keys"); +} + +FILE * +auth_openprincipals(const char *file, struct passwd *pw, int strict_modes) +{ + return auth_openfile(file, pw, strict_modes, 0, + "authorized principals"); +} + struct passwd * getpwnamallow(const char *user) { diff --git a/auth.h b/auth.h index a65b87dd1..77317aee6 100644 --- a/auth.h +++ b/auth.h @@ -1,4 +1,4 @@ -/* $OpenBSD: auth.h,v 1.65 2010/03/04 10:36:03 djm Exp $ */ +/* $OpenBSD: auth.h,v 1.66 2010/05/07 11:30:29 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. @@ -169,8 +169,10 @@ void abandon_challenge_response(Authctxt *); char *authorized_keys_file(struct passwd *); char *authorized_keys_file2(struct passwd *); +char *authorized_principals_file(struct passwd *); FILE *auth_openkeyfile(const char *, struct passwd *, int); +FILE *auth_openprincipals(const char *, struct passwd *, int); int auth_key_is_revoked(Key *); HostStatus diff --git a/auth2-pubkey.c b/auth2-pubkey.c index 83ecd6590..6b4a99725 100644 --- a/auth2-pubkey.c +++ b/auth2-pubkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth2-pubkey.c,v 1.23 2010/04/16 01:47:26 djm Exp $ */ +/* $OpenBSD: auth2-pubkey.c,v 1.24 2010/05/07 11:30:29 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -57,6 +57,7 @@ #include "monitor_wrap.h" #include "misc.h" #include "authfile.h" +#include "match.h" /* import */ extern ServerOptions options; @@ -176,6 +177,63 @@ done: return authenticated; } +static int +match_principals_option(const char *principal_list, struct KeyCert *cert) +{ + char *result; + u_int i; + + /* XXX percent_expand() sequences for authorized_principals? */ + + for (i = 0; i < cert->nprincipals; i++) { + if ((result = match_list(cert->principals[i], + principal_list, NULL)) != NULL) { + debug3("matched principal from key options \"%.100s\"", + result); + xfree(result); + return 1; + } + } + return 0; +} + +static int +match_principals_file(const char *file, struct passwd *pw, struct KeyCert *cert) +{ + FILE *f; + char line[SSH_MAX_PUBKEY_BYTES], *cp; + u_long linenum = 0; + u_int i; + + temporarily_use_uid(pw); + debug("trying authorized principals file %s", file); + if ((f = auth_openprincipals(file, pw, options.strict_modes)) == NULL) { + restore_uid(); + return 0; + } + while (read_keyfile_line(f, file, line, sizeof(line), &linenum) != -1) { + /* Skip leading whitespace, empty and comment lines. */ + for (cp = line; *cp == ' ' || *cp == '\t'; cp++) + ; + if (!*cp || *cp == '\n' || *cp == '#') + continue; + line[strcspn(line, "\n")] = '\0'; + + for (i = 0; i < cert->nprincipals; i++) { + if (strcmp(cp, cert->principals[i]) == 0) { + debug3("matched principal from file \"%.100s\"", + cert->principals[i]); + fclose(f); + restore_uid(); + return 1; + } + } + } + fclose(f); + restore_uid(); + return 0; +} + /* return 1 if user allows given key */ static int user_key_allowed2(struct passwd *pw, Key *key, char *file) @@ -244,13 +302,26 @@ user_key_allowed2(struct passwd *pw, Key *key, char *file) SSH_FP_HEX); debug("matching CA found: file %s, line %lu, %s %s", file, linenum, key_type(found), fp); - if (key_cert_check_authority(key, 0, 0, pw->pw_name, - &reason) != 0) { + /* + * If the user has specified a list of principals as + * a key option, then prefer that list to matching + * their username in the certificate principals list. + */ + if (authorized_principals != NULL && + !match_principals_option(authorized_principals, + key->cert)) { + reason = "Certificate does not contain an " + "authorized principal"; + fail_reason: xfree(fp); error("%s", reason); auth_debug_add("%s", reason); continue; } + if (key_cert_check_authority(key, 0, 0, + authorized_principals == NULL ? pw->pw_name : NULL, + &reason) != 0) + goto fail_reason; if (auth_cert_options(key, pw) != 0) { xfree(fp); continue; @@ -284,7 +355,7 @@ user_key_allowed2(struct passwd *pw, Key *key, char *file) static int user_cert_trusted_ca(struct passwd *pw, Key *key) { - char *ca_fp; + char *ca_fp, *principals_file = NULL; const char *reason; int ret = 0; @@ -301,11 +372,24 @@ user_cert_trusted_ca(struct passwd *pw, Key *key) options.trusted_user_ca_keys); goto out; } - if (key_cert_check_authority(key, 0, 1, pw->pw_name, &reason) != 0) { - error("%s", reason); - auth_debug_add("%s", reason); - goto out; + /* + * If AuthorizedPrincipals is in use, then compare the certificate + * principals against the names in that file rather than matching + * against the username. + */ + if ((principals_file = authorized_principals_file(pw)) != NULL) { + if (!match_principals_file(principals_file, pw, key->cert)) { + reason = "Certificate does not contain an " + "authorized principal"; + fail_reason: + error("%s", reason); + auth_debug_add("%s", reason); + goto out; + } } + if (key_cert_check_authority(key, 0, 1, + principals_file == NULL ? pw->pw_name : NULL, &reason) != 0) + goto fail_reason; if (auth_cert_options(key, pw) != 0) goto out; @@ -315,6 +399,8 @@ user_cert_trusted_ca(struct passwd *pw, Key *key) ret = 1; out: + if (principals_file != NULL) + xfree(principals_file); if (ca_fp != NULL) xfree(ca_fp); return ret; diff --git a/key.c b/key.c index 34f678b38..c4d9d5771 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.87 2010/04/16 01:47:26 djm Exp $ */ +/* $OpenBSD: key.c,v 1.88 2010/05/07 11:30:29 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1623,7 +1623,7 @@ key_cert_check_authority(const Key *k, int want_host, int require_principal, *reason = "Certificate lacks principal list"; return -1; } - } else { + } else if (name != NULL) { principal_matches = 0; for (i = 0; i < k->cert->nprincipals; i++) { if (strcmp(name, k->cert->principals[i]) == 0) { diff --git a/servconf.c b/servconf.c index 7d027ddb9..c556986e3 100644 --- a/servconf.c +++ b/servconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: servconf.c,v 1.207 2010/03/25 23:38:28 djm Exp $ */ +/* $OpenBSD: servconf.c,v 1.208 2010/05/07 11:30:29 djm Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -131,6 +131,7 @@ initialize_server_options(ServerOptions *options) options->zero_knowledge_password_authentication = -1; options->revoked_keys_file = NULL; options->trusted_user_ca_keys = NULL; + options->authorized_principals_file = NULL; } void @@ -310,7 +311,7 @@ typedef enum { sMatch, sPermitOpen, sForceCommand, sChrootDirectory, sUsePrivilegeSeparation, sAllowAgentForwarding, sZeroKnowledgePasswordAuthentication, sHostCertificate, - sRevokedKeys, sTrustedUserCAKeys, + sRevokedKeys, sTrustedUserCAKeys, sAuthorizedPrincipalsFile, sDeprecated, sUnsupported } ServerOpCodes; @@ -432,6 +433,7 @@ static struct { { "hostcertificate", sHostCertificate, SSHCFG_GLOBAL }, { "revokedkeys", sRevokedKeys, SSHCFG_ALL }, { "trustedusercakeys", sTrustedUserCAKeys, SSHCFG_ALL }, + { "authorizedprincipalsfile", sAuthorizedPrincipalsFile, SSHCFG_GLOBAL }, { NULL, sBadOption, 0 } }; @@ -1218,10 +1220,14 @@ process_server_config_line(ServerOptions *options, char *line, * AuthorizedKeysFile /etc/ssh_keys/%u */ case sAuthorizedKeysFile: + charptr = &options->authorized_keys_file; + goto parse_tilde_filename; case sAuthorizedKeysFile2: - charptr = (opcode == sAuthorizedKeysFile) ? - &options->authorized_keys_file : - &options->authorized_keys_file2; + charptr = &options->authorized_keys_file2; + goto parse_tilde_filename; + case sAuthorizedPrincipalsFile: + charptr = &options->authorized_principals_file; + parse_tilde_filename: arg = strdelim(&cp); if (!arg || *arg == '\0') fatal("%s line %d: missing file name.", @@ -1682,6 +1688,8 @@ dump_config(ServerOptions *o) dump_cfg_string(sChrootDirectory, o->chroot_directory); dump_cfg_string(sTrustedUserCAKeys, o->trusted_user_ca_keys); dump_cfg_string(sRevokedKeys, o->revoked_keys_file); + dump_cfg_string(sAuthorizedPrincipalsFile, + o->authorized_principals_file); /* string arguments requiring a lookup */ dump_cfg_string(sLogLevel, log_level_name(o->log_level)); diff --git a/servconf.h b/servconf.h index 860009f9c..45d2a2ae3 100644 --- a/servconf.h +++ b/servconf.h @@ -1,4 +1,4 @@ -/* $OpenBSD: servconf.h,v 1.92 2010/03/04 10:36:03 djm Exp $ */ +/* $OpenBSD: servconf.h,v 1.93 2010/05/07 11:30:30 djm Exp $ */ /* * Author: Tatu Ylonen @@ -156,6 +156,7 @@ typedef struct { char *chroot_directory; char *revoked_keys_file; char *trusted_user_ca_keys; + char *authorized_principals_file; } ServerOptions; void initialize_server_options(ServerOptions *); diff --git a/sshd.8 b/sshd.8 index 5f1966005..6eb49238a 100644 --- a/sshd.8 +++ b/sshd.8 @@ -34,8 +34,8 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.\" $OpenBSD: sshd.8,v 1.255 2010/03/05 06:50:35 jmc Exp $ -.Dd $Mdocdate: March 5 2010 $ +.\" $OpenBSD: sshd.8,v 1.256 2010/05/07 11:30:30 djm Exp $ +.Dd $Mdocdate: May 7 2010 $ .Dt SSHD 8 .Os .Sh NAME @@ -602,6 +602,17 @@ Multiple options may be applied separated by commas. No pattern matching is performed on the specified hostnames, they must be literal domains or addresses. +.It Cm principals="principals" +On a +.Cm cert-authority +line, specifies allowed principals for certificate authentication as a +comma-separated list. +At least one name from the list must appear in the certificate's +list of principals for the certificate to be accepted. +This option is ignored for keys that are not marked as trusted certificate +signers using the +.Cm cert-authority +option. .It Cm tunnel="n" Force a .Xr tun 4 diff --git a/sshd_config.5 b/sshd_config.5 index 2f5410281..a5260d358 100644 --- a/sshd_config.5 +++ b/sshd_config.5 @@ -34,8 +34,8 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.\" $OpenBSD: sshd_config.5,v 1.120 2010/03/04 23:17:25 djm Exp $ -.Dd $Mdocdate: March 4 2010 $ +.\" $OpenBSD: sshd_config.5,v 1.121 2010/05/07 11:30:30 djm Exp $ +.Dd $Mdocdate: May 7 2010 $ .Dt SSHD_CONFIG 5 .Os .Sh NAME @@ -167,6 +167,43 @@ is taken to be an absolute path or one relative to the user's home directory. The default is .Dq .ssh/authorized_keys . +.It Cm AuthorizedPrincipalsFile +Specifies a file that lists principal names that are accepted for +certificate authentication. +When using certificates signed by a key listed in +.Cm TrustedUserCAKeys , +this file lists names, one of which must appear in the certificate for it +to be accepted for authentication. +Names are listed one per line; empty lines and comments starting with +.Ql # +are ignored. +.Pp +.Cm AuthorizedPrincipalsFile +may contain tokens of the form %T which are substituted during connection +setup. +The following tokens are defined: %% is replaced by a literal '%', +%h is replaced by the home directory of the user being authenticated, and +%u is replaced by the username of that user. +After expansion, +.Cm AuthorizedPrincipalsFile +is taken to be an absolute path or one relative to the user's home +directory. +.Pp +The default is not to use a principals file - in this case, the username +of the user must appear in a certificate's principals list for it to be +accepted. +Note that +.Cm AuthorizedPrincipalsFile +is only used when authentication proceeds using a CA listed in +.Cm TrustedUserCAKeys +and is not consulted for certification authorities trusted via +.Pa ~/.ssh/authorized_keys , +though the +.Cm principals= +key option offers a similar facility (see +.Xr sshd 8 +for details). +.Pp .It Cm Banner The contents of the specified file are sent to the remote user before authentication is allowed. -- cgit v1.2.3 From 8a0268f1b3f62292d4124f8d158e0587c4f7c330 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 16 Jul 2010 13:57:51 +1000 Subject: - djm@cvs.openbsd.org 2010/07/13 11:52:06 [auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c] [packet.c ssh-rsa.c] implement a timing_safe_cmp() function to compare memory without leaking timing information by short-circuiting like memcmp() and use it for some of the more sensitive comparisons (though nothing high-value was readily attackable anyway); "looks ok" markus@ --- ChangeLog | 7 +++++++ auth-rsa.c | 4 ++-- channels.c | 4 ++-- jpake.c | 4 ++-- key.c | 5 +++-- misc.c | 14 +++++++++++++- misc.h | 3 ++- monitor.c | 16 ++++++++-------- packet.c | 4 ++-- ssh-rsa.c | 7 ++++--- 10 files changed, 45 insertions(+), 23 deletions(-) (limited to 'key.c') diff --git a/ChangeLog b/ChangeLog index f652e6183..d650f63f8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,6 +22,13 @@ Hostname %h.example.org "I like it" markus@ + - djm@cvs.openbsd.org 2010/07/13 11:52:06 + [auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c] + [packet.c ssh-rsa.c] + implement a timing_safe_cmp() function to compare memory without leaking + timing information by short-circuiting like memcmp() and use it for + some of the more sensitive comparisons (though nothing high-value was + readily attackable anyway); "looks ok" markus@ 20100714 - (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass diff --git a/auth-rsa.c b/auth-rsa.c index ef6767bfb..71abadf6c 100644 --- a/auth-rsa.c +++ b/auth-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-rsa.c,v 1.76 2010/05/11 02:58:04 djm Exp $ */ +/* $OpenBSD: auth-rsa.c,v 1.77 2010/07/13 11:52:06 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -116,7 +116,7 @@ auth_rsa_verify_response(Key *key, BIGNUM *challenge, u_char response[16]) MD5_Final(mdbuf, &md); /* Verify that the response is the original challenge. */ - if (memcmp(response, mdbuf, 16) != 0) { + if (timing_safe_cmp(response, mdbuf, 16) != 0) { /* Wrong answer. */ return (0); } diff --git a/channels.c b/channels.c index fe08257df..e52946bcd 100644 --- a/channels.c +++ b/channels.c @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.c,v 1.306 2010/06/25 07:20:04 djm Exp $ */ +/* $OpenBSD: channels.c,v 1.307 2010/07/13 11:52:06 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -917,7 +917,7 @@ x11_open_helper(Buffer *b) } /* Check if authentication data matches our fake data. */ if (data_len != x11_fake_data_len || - memcmp(ucp + 12 + ((proto_len + 3) & ~3), + timing_safe_cmp(ucp + 12 + ((proto_len + 3) & ~3), x11_fake_data, x11_fake_data_len) != 0) { debug2("X11 auth data does not match fake data."); return -1; diff --git a/jpake.c b/jpake.c index 130661069..50cf5c82e 100644 --- a/jpake.c +++ b/jpake.c @@ -1,4 +1,4 @@ -/* $OpenBSD: jpake.c,v 1.2 2009/03/05 07:18:19 djm Exp $ */ +/* $OpenBSD: jpake.c,v 1.3 2010/07/13 11:52:06 djm Exp $ */ /* * Copyright (c) 2008 Damien Miller. All rights reserved. * @@ -434,7 +434,7 @@ jpake_check_confirm(const BIGNUM *k, if (peer_confirm_hash_len != expected_confirm_hash_len) error("%s: confirmation length mismatch (my %u them %u)", __func__, expected_confirm_hash_len, peer_confirm_hash_len); - else if (memcmp(peer_confirm_hash, expected_confirm_hash, + else if (timing_safe_cmp(peer_confirm_hash, expected_confirm_hash, expected_confirm_hash_len) == 0) success = 1; bzero(expected_confirm_hash, expected_confirm_hash_len); diff --git a/key.c b/key.c index c4d9d5771..4e31c84e4 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.88 2010/05/07 11:30:29 djm Exp $ */ +/* $OpenBSD: key.c,v 1.89 2010/07/13 11:52:06 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -52,6 +52,7 @@ #include "uuencode.h" #include "buffer.h" #include "log.h" +#include "misc.h" #include "ssh2.h" static struct KeyCert * @@ -227,7 +228,7 @@ cert_compare(struct KeyCert *a, struct KeyCert *b) return 0; if (buffer_len(&a->certblob) != buffer_len(&b->certblob)) return 0; - if (memcmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), + if (timing_safe_cmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), buffer_len(&a->certblob)) != 0) return 0; return 1; diff --git a/misc.c b/misc.c index 4500b7a37..3b98e3fc2 100644 --- a/misc.c +++ b/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.77 2010/07/02 04:32:44 djm Exp $ */ +/* $OpenBSD: misc.c,v 1.78 2010/07/13 11:52:06 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2005,2006 Damien Miller. All rights reserved. @@ -850,6 +850,18 @@ ms_to_timeval(struct timeval *tv, int ms) tv->tv_usec = (ms % 1000) * 1000; } +int +timing_safe_cmp(const void *_s1, const void *_s2, size_t n) +{ + u_char *s1 = (u_char *)_s1; + u_char *s2 = (u_char *)_s2; + int ret = 0; + + for (; n > 0; n--, s1++, s2++) + ret |= *s1 ^ *s2; + return ret; +} + void sock_set_v6only(int s) { diff --git a/misc.h b/misc.h index 32073acd4..7a02a03a5 100644 --- a/misc.h +++ b/misc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.41 2010/01/09 23:04:13 dtucker Exp $ */ +/* $OpenBSD: misc.h,v 1.42 2010/07/13 11:52:06 djm Exp $ */ /* * Author: Tatu Ylonen @@ -36,6 +36,7 @@ void sanitise_stdfd(void); void ms_subtract_diff(struct timeval *, int *); void ms_to_timeval(struct timeval *, int); void sock_set_v6only(int); +int timing_safe_cmp(const void *, const void *, size_t); struct passwd *pwcopy(struct passwd *); const char *ssh_gai_strerror(int); diff --git a/monitor.c b/monitor.c index 334aedde5..920728398 100644 --- a/monitor.c +++ b/monitor.c @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor.c,v 1.106 2010/03/07 11:57:13 dtucker Exp $ */ +/* $OpenBSD: monitor.c,v 1.107 2010/07/13 11:52:06 djm Exp $ */ /* * Copyright 2002 Niels Provos * Copyright 2002 Markus Friedl @@ -518,7 +518,7 @@ monitor_allowed_key(u_char *blob, u_int bloblen) { /* make sure key is allowed */ if (key_blob == NULL || key_bloblen != bloblen || - memcmp(key_blob, blob, key_bloblen)) + timing_safe_cmp(key_blob, blob, key_bloblen)) return (0); return (1); } @@ -1103,14 +1103,14 @@ monitor_valid_userblob(u_char *data, u_int datalen) len = buffer_len(&b); if ((session_id2 == NULL) || (len < session_id2_len) || - (memcmp(p, session_id2, session_id2_len) != 0)) + (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) fail++; buffer_consume(&b, session_id2_len); } else { p = buffer_get_string(&b, &len); if ((session_id2 == NULL) || (len != session_id2_len) || - (memcmp(p, session_id2, session_id2_len) != 0)) + (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) fail++; xfree(p); } @@ -1158,7 +1158,7 @@ monitor_valid_hostbasedblob(u_char *data, u_int datalen, char *cuser, p = buffer_get_string(&b, &len); if ((session_id2 == NULL) || (len != session_id2_len) || - (memcmp(p, session_id2, session_id2_len) != 0)) + (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) fail++; xfree(p); @@ -1682,9 +1682,9 @@ mm_get_kex(Buffer *m) kex = xcalloc(1, sizeof(*kex)); kex->session_id = buffer_get_string(m, &kex->session_id_len); - if ((session_id2 == NULL) || - (kex->session_id_len != session_id2_len) || - (memcmp(kex->session_id, session_id2, session_id2_len) != 0)) + if (session_id2 == NULL || + kex->session_id_len != session_id2_len || + timing_safe_cmp(kex->session_id, session_id2, session_id2_len) != 0) fatal("mm_get_get: internal error: bad session id"); kex->we_need = buffer_get_int(m); kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server; diff --git a/packet.c b/packet.c index 994e35b6d..5c7ec2b5f 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.166 2009/06/27 09:29:06 andreas Exp $ */ +/* $OpenBSD: packet.c,v 1.167 2010/07/13 11:52:06 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1307,7 +1307,7 @@ packet_read_poll2(u_int32_t *seqnr_p) macbuf = mac_compute(mac, active_state->p_read.seqnr, buffer_ptr(&active_state->incoming_packet), buffer_len(&active_state->incoming_packet)); - if (memcmp(macbuf, buffer_ptr(&active_state->input), + if (timing_safe_cmp(macbuf, buffer_ptr(&active_state->input), mac->mac_len) != 0) { logit("Corrupted MAC on input."); if (need > PACKET_MAX_SIZE) diff --git a/ssh-rsa.c b/ssh-rsa.c index bb9cc8e20..01f27f52c 100644 --- a/ssh-rsa.c +++ b/ssh-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-rsa.c,v 1.41 2010/04/16 01:47:26 djm Exp $ */ +/* $OpenBSD: ssh-rsa.c,v 1.42 2010/07/13 11:52:06 djm Exp $ */ /* * Copyright (c) 2000, 2003 Markus Friedl * @@ -30,6 +30,7 @@ #include "buffer.h" #include "key.h" #include "compat.h" +#include "misc.h" #include "ssh.h" static int openssh_RSA_verify(int, u_char *, u_int, u_char *, u_int, RSA *); @@ -249,11 +250,11 @@ openssh_RSA_verify(int type, u_char *hash, u_int hashlen, error("bad decrypted len: %d != %d + %d", len, hlen, oidlen); goto done; } - if (memcmp(decrypted, oid, oidlen) != 0) { + if (timing_safe_cmp(decrypted, oid, oidlen) != 0) { error("oid mismatch"); goto done; } - if (memcmp(decrypted + oidlen, hash, hlen) != 0) { + if (timing_safe_cmp(decrypted + oidlen, hash, hlen) != 0) { error("hash mismatch"); goto done; } -- cgit v1.2.3 From ea1651c98e1814f54c8d4b027b31f7de1c34989c Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 16 Jul 2010 13:58:37 +1000 Subject: - djm@cvs.openbsd.org 2010/07/13 23:13:16 [auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c packet.c] [ssh-rsa.c] s/timing_safe_cmp/timingsafe_bcmp/g --- ChangeLog | 4 ++++ auth-rsa.c | 4 ++-- channels.c | 4 ++-- jpake.c | 4 ++-- key.c | 4 ++-- misc.c | 4 ++-- misc.h | 4 ++-- monitor.c | 12 ++++++------ packet.c | 4 ++-- ssh-rsa.c | 6 +++--- 10 files changed, 27 insertions(+), 23 deletions(-) (limited to 'key.c') diff --git a/ChangeLog b/ChangeLog index d650f63f8..1434b3d3c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,10 @@ timing information by short-circuiting like memcmp() and use it for some of the more sensitive comparisons (though nothing high-value was readily attackable anyway); "looks ok" markus@ + - djm@cvs.openbsd.org 2010/07/13 23:13:16 + [auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c packet.c] + [ssh-rsa.c] + s/timing_safe_cmp/timingsafe_bcmp/g 20100714 - (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass diff --git a/auth-rsa.c b/auth-rsa.c index 71abadf6c..56702d130 100644 --- a/auth-rsa.c +++ b/auth-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-rsa.c,v 1.77 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: auth-rsa.c,v 1.78 2010/07/13 23:13:16 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -116,7 +116,7 @@ auth_rsa_verify_response(Key *key, BIGNUM *challenge, u_char response[16]) MD5_Final(mdbuf, &md); /* Verify that the response is the original challenge. */ - if (timing_safe_cmp(response, mdbuf, 16) != 0) { + if (timingsafe_bcmp(response, mdbuf, 16) != 0) { /* Wrong answer. */ return (0); } diff --git a/channels.c b/channels.c index e52946bcd..fd6244d48 100644 --- a/channels.c +++ b/channels.c @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.c,v 1.307 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: channels.c,v 1.308 2010/07/13 23:13:16 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -917,7 +917,7 @@ x11_open_helper(Buffer *b) } /* Check if authentication data matches our fake data. */ if (data_len != x11_fake_data_len || - timing_safe_cmp(ucp + 12 + ((proto_len + 3) & ~3), + timingsafe_bcmp(ucp + 12 + ((proto_len + 3) & ~3), x11_fake_data, x11_fake_data_len) != 0) { debug2("X11 auth data does not match fake data."); return -1; diff --git a/jpake.c b/jpake.c index 50cf5c82e..cdf65f509 100644 --- a/jpake.c +++ b/jpake.c @@ -1,4 +1,4 @@ -/* $OpenBSD: jpake.c,v 1.3 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: jpake.c,v 1.4 2010/07/13 23:13:16 djm Exp $ */ /* * Copyright (c) 2008 Damien Miller. All rights reserved. * @@ -434,7 +434,7 @@ jpake_check_confirm(const BIGNUM *k, if (peer_confirm_hash_len != expected_confirm_hash_len) error("%s: confirmation length mismatch (my %u them %u)", __func__, expected_confirm_hash_len, peer_confirm_hash_len); - else if (timing_safe_cmp(peer_confirm_hash, expected_confirm_hash, + else if (timingsafe_bcmp(peer_confirm_hash, expected_confirm_hash, expected_confirm_hash_len) == 0) success = 1; bzero(expected_confirm_hash, expected_confirm_hash_len); diff --git a/key.c b/key.c index 4e31c84e4..e4aa25c03 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.89 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: key.c,v 1.90 2010/07/13 23:13:16 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -228,7 +228,7 @@ cert_compare(struct KeyCert *a, struct KeyCert *b) return 0; if (buffer_len(&a->certblob) != buffer_len(&b->certblob)) return 0; - if (timing_safe_cmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), + if (timingsafe_bcmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), buffer_len(&a->certblob)) != 0) return 0; return 1; diff --git a/misc.c b/misc.c index 3b98e3fc2..52f814fa2 100644 --- a/misc.c +++ b/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.78 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: misc.c,v 1.79 2010/07/13 23:13:16 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2005,2006 Damien Miller. All rights reserved. @@ -851,7 +851,7 @@ ms_to_timeval(struct timeval *tv, int ms) } int -timing_safe_cmp(const void *_s1, const void *_s2, size_t n) +timingsafe_bcmp(const void *_s1, const void *_s2, size_t n) { u_char *s1 = (u_char *)_s1; u_char *s2 = (u_char *)_s2; diff --git a/misc.h b/misc.h index 7a02a03a5..bb799f616 100644 --- a/misc.h +++ b/misc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.42 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: misc.h,v 1.43 2010/07/13 23:13:16 djm Exp $ */ /* * Author: Tatu Ylonen @@ -36,7 +36,7 @@ void sanitise_stdfd(void); void ms_subtract_diff(struct timeval *, int *); void ms_to_timeval(struct timeval *, int); void sock_set_v6only(int); -int timing_safe_cmp(const void *, const void *, size_t); +int timingsafe_bcmp(const void *, const void *, size_t); struct passwd *pwcopy(struct passwd *); const char *ssh_gai_strerror(int); diff --git a/monitor.c b/monitor.c index 920728398..7acbeaa65 100644 --- a/monitor.c +++ b/monitor.c @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor.c,v 1.107 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: monitor.c,v 1.108 2010/07/13 23:13:16 djm Exp $ */ /* * Copyright 2002 Niels Provos * Copyright 2002 Markus Friedl @@ -518,7 +518,7 @@ monitor_allowed_key(u_char *blob, u_int bloblen) { /* make sure key is allowed */ if (key_blob == NULL || key_bloblen != bloblen || - timing_safe_cmp(key_blob, blob, key_bloblen)) + timingsafe_bcmp(key_blob, blob, key_bloblen)) return (0); return (1); } @@ -1103,14 +1103,14 @@ monitor_valid_userblob(u_char *data, u_int datalen) len = buffer_len(&b); if ((session_id2 == NULL) || (len < session_id2_len) || - (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) + (timingsafe_bcmp(p, session_id2, session_id2_len) != 0)) fail++; buffer_consume(&b, session_id2_len); } else { p = buffer_get_string(&b, &len); if ((session_id2 == NULL) || (len != session_id2_len) || - (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) + (timingsafe_bcmp(p, session_id2, session_id2_len) != 0)) fail++; xfree(p); } @@ -1158,7 +1158,7 @@ monitor_valid_hostbasedblob(u_char *data, u_int datalen, char *cuser, p = buffer_get_string(&b, &len); if ((session_id2 == NULL) || (len != session_id2_len) || - (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) + (timingsafe_bcmp(p, session_id2, session_id2_len) != 0)) fail++; xfree(p); @@ -1684,7 +1684,7 @@ mm_get_kex(Buffer *m) kex->session_id = buffer_get_string(m, &kex->session_id_len); if (session_id2 == NULL || kex->session_id_len != session_id2_len || - timing_safe_cmp(kex->session_id, session_id2, session_id2_len) != 0) + timingsafe_bcmp(kex->session_id, session_id2, session_id2_len) != 0) fatal("mm_get_get: internal error: bad session id"); kex->we_need = buffer_get_int(m); kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server; diff --git a/packet.c b/packet.c index 5c7ec2b5f..48f7fe613 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.167 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: packet.c,v 1.168 2010/07/13 23:13:16 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1307,7 +1307,7 @@ packet_read_poll2(u_int32_t *seqnr_p) macbuf = mac_compute(mac, active_state->p_read.seqnr, buffer_ptr(&active_state->incoming_packet), buffer_len(&active_state->incoming_packet)); - if (timing_safe_cmp(macbuf, buffer_ptr(&active_state->input), + if (timingsafe_bcmp(macbuf, buffer_ptr(&active_state->input), mac->mac_len) != 0) { logit("Corrupted MAC on input."); if (need > PACKET_MAX_SIZE) diff --git a/ssh-rsa.c b/ssh-rsa.c index 01f27f52c..e3f156156 100644 --- a/ssh-rsa.c +++ b/ssh-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-rsa.c,v 1.42 2010/07/13 11:52:06 djm Exp $ */ +/* $OpenBSD: ssh-rsa.c,v 1.43 2010/07/13 23:13:16 djm Exp $ */ /* * Copyright (c) 2000, 2003 Markus Friedl * @@ -250,11 +250,11 @@ openssh_RSA_verify(int type, u_char *hash, u_int hashlen, error("bad decrypted len: %d != %d + %d", len, hlen, oidlen); goto done; } - if (timing_safe_cmp(decrypted, oid, oidlen) != 0) { + if (timingsafe_bcmp(decrypted, oid, oidlen) != 0) { error("oid mismatch"); goto done; } - if (timing_safe_cmp(decrypted + oidlen, hash, hlen) != 0) { + if (timingsafe_bcmp(decrypted + oidlen, hash, hlen) != 0) { error("hash mismatch"); goto done; } -- cgit v1.2.3