From 4ba0d54794814ec0de1ec87987d0c3b89379b436 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Tue, 3 Jul 2018 11:39:54 +0000 Subject: upstream: Improve strictness and control over RSA-SHA2 signature In ssh, when an agent fails to return a RSA-SHA2 signature when requested and falls back to RSA-SHA1 instead, retry the signature to ensure that the public key algorithm sent in the SSH_MSG_USERAUTH matches the one in the signature itself. In sshd, strictly enforce that the public key algorithm sent in the SSH_MSG_USERAUTH message matches what appears in the signature. Make the sshd_config PubkeyAcceptedKeyTypes and HostbasedAcceptedKeyTypes options control accepted signature algorithms (previously they selected supported key types). This allows these options to ban RSA-SHA1 in favour of RSA-SHA2. Add new signature algorithms "rsa-sha2-256-cert-v01@openssh.com" and "rsa-sha2-512-cert-v01@openssh.com" to force use of RSA-SHA2 signatures with certificate keys. feedback and ok markus@ OpenBSD-Commit-ID: c6e9f6d45eed8962ad502d315d7eaef32c419dde --- sshconnect2.c | 345 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 216 insertions(+), 129 deletions(-) (limited to 'sshconnect2.c') diff --git a/sshconnect2.c b/sshconnect2.c index d8ae6eb3a..920376408 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshconnect2.c,v 1.271 2018/06/26 02:02:36 djm Exp $ */ +/* $OpenBSD: sshconnect2.c,v 1.272 2018/07/03 11:39:54 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2008 Damien Miller. All rights reserved. @@ -315,7 +315,7 @@ int input_gssapi_errtok(int, u_int32_t, struct ssh *); void userauth(Authctxt *, char *); -static int sign_and_send_pubkey(Authctxt *, Identity *); +static int sign_and_send_pubkey(struct ssh *ssh, Authctxt *, Identity *); static void pubkey_prepare(Authctxt *); static void pubkey_cleanup(Authctxt *); static void pubkey_reset(Authctxt *); @@ -619,7 +619,7 @@ input_userauth_pk_ok(int type, u_int32_t seq, struct ssh *ssh) */ TAILQ_FOREACH_REVERSE(id, &authctxt->keys, idlist, next) { if (key_equal(key, id->key)) { - sent = sign_and_send_pubkey(authctxt, id); + sent = sign_and_send_pubkey(ssh, authctxt, id); break; } } @@ -986,73 +986,80 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh) return 0; } -static const char * -key_sign_encode(const struct sshkey *key) -{ - struct ssh *ssh = active_state; - - if (key->type == KEY_RSA) { - switch (ssh->kex->rsa_sha2) { - case 256: - return "rsa-sha2-256"; - case 512: - return "rsa-sha2-512"; - } - } - return key_ssh_name(key); -} - /* - * Some agents will return ssh-rsa signatures when asked to make a - * rsa-sha2-* signature. Check what they actually gave back and warn the - * user if the agent has returned an unexpected type. + * Select an algorithm for publickey signatures. + * Returns algorithm (caller must free) or NULL if no mutual algorithm found. + * + * Call with ssh==NULL to ignore server-sig-algs extension list and + * only attempt with the key's base signature type. */ -static int -check_sigtype(const struct sshkey *key, const u_char *sig, size_t len) +static char * +key_sig_algorithm(struct ssh *ssh, const struct sshkey *key) { - int r; - char *sigtype = NULL; - const char *alg = key_sign_encode(key); + char *allowed, *oallowed, *cp, *alg = NULL; - if (sshkey_is_cert(key)) - return 0; - if ((r = sshkey_sigtype(sig, len, &sigtype)) != 0) - return r; - if (strcmp(sigtype, alg) != 0) { - logit("warning: agent returned different signature type %s " - "(expected %s)", sigtype, alg); - } - free(sigtype); - /* Incorrect signature types aren't an error ... yet */ - return 0; + /* + * The signature algorithm will only differ from the key algorithm + * for RSA keys/certs and when the server advertises support for + * newer (SHA2) algorithms. + */ + if (ssh == NULL || ssh->kex->server_sig_algs == NULL || + (key->type != KEY_RSA && key->type != KEY_RSA_CERT)) { + /* Filter base key signature alg against our configuration */ + return match_list(key_ssh_name(key), + options.pubkey_key_types, NULL); + } + + /* + * For RSA keys/certs, since these might have a different sig type: + * find the first entry in PubkeyAcceptedKeyTypes of the right type + * that also appears in the supported signature algorithms list from + * the server. + */ + oallowed = allowed = xstrdup(options.pubkey_key_types); + while ((cp = strsep(&allowed, ",")) != NULL) { + if (sshkey_type_from_name(cp) != key->type) + continue; + alg = match_list(cp, ssh->kex->server_sig_algs, NULL); + if (alg != NULL) + break; + } + free(oallowed); + return alg; } static int identity_sign(struct identity *id, u_char **sigp, size_t *lenp, - const u_char *data, size_t datalen, u_int compat) + const u_char *data, size_t datalen, u_int compat, const char *alg) { struct sshkey *prv; int r; - /* the agent supports this key */ + /* The agent supports this key. */ if (id->key != NULL && id->agent_fd != -1) { - if ((r = ssh_agent_sign(id->agent_fd, id->key, sigp, lenp, - data, datalen, key_sign_encode(id->key), compat)) != 0 || - (r = check_sigtype(id->key, *sigp, *lenp)) != 0) - return r; - return 0; + return ssh_agent_sign(id->agent_fd, id->key, sigp, lenp, + data, datalen, alg, compat); } /* - * we have already loaded the private key or - * the private key is stored in external hardware + * We have already loaded the private key or the private key is + * stored in external hardware. */ if (id->key != NULL && - (id->isprivate || (id->key->flags & SSHKEY_FLAG_EXT))) - return (sshkey_sign(id->key, sigp, lenp, data, datalen, - key_sign_encode(id->key), compat)); + (id->isprivate || (id->key->flags & SSHKEY_FLAG_EXT))) { + if ((r = sshkey_sign(id->key, sigp, lenp, data, datalen, + alg, compat)) != 0) + return r; + /* + * PKCS#11 tokens may not support all signature algorithms, + * so check what we get back. + */ + if ((r = sshkey_check_sigtype(*sigp, *lenp, alg)) != 0) + return r; + return 0; + } - /* load the private key from the file */ + /* Load the private key from the file. */ if ((prv = load_identity_file(id)) == NULL) return SSH_ERR_KEY_NOT_FOUND; if (id->key != NULL && !sshkey_equal_public(prv, id->key)) { @@ -1060,8 +1067,7 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp, __func__, id->filename); return SSH_ERR_KEY_NOT_FOUND; } - r = sshkey_sign(prv, sigp, lenp, data, datalen, - key_sign_encode(prv), compat); + r = sshkey_sign(prv, sigp, lenp, data, datalen, alg, compat); sshkey_free(prv); return r; } @@ -1086,57 +1092,35 @@ id_filename_matches(Identity *id, Identity *private_id) } static int -sign_and_send_pubkey(Authctxt *authctxt, Identity *id) +sign_and_send_pubkey(struct ssh *ssh, Authctxt *authctxt, Identity *id) { - Buffer b; - Identity *private_id; - u_char *blob, *signature; - size_t slen; - u_int bloblen, skip = 0; - int matched, ret = -1, have_sig = 1; - char *fp; + struct sshbuf *b = NULL; + Identity *private_id, *sign_id = NULL; + u_char *signature = NULL; + size_t slen = 0, skip = 0; + int r, fallback_sigtype, sent = 0; + char *alg = NULL, *fp = NULL; + const char *loc = ""; if ((fp = sshkey_fingerprint(id->key, options.fingerprint_hash, SSH_FP_DEFAULT)) == NULL) return 0; - debug3("%s: %s %s", __func__, key_type(id->key), fp); - free(fp); - if (key_to_blob(id->key, &blob, &bloblen) == 0) { - /* we cannot handle this key */ - debug3("sign_and_send_pubkey: cannot handle key"); - return 0; - } - /* data to be signed */ - buffer_init(&b); - if (datafellows & SSH_OLD_SESSIONID) { - buffer_append(&b, session_id2, session_id2_len); - skip = session_id2_len; - } else { - buffer_put_string(&b, session_id2, session_id2_len); - skip = buffer_len(&b); - } - buffer_put_char(&b, SSH2_MSG_USERAUTH_REQUEST); - buffer_put_cstring(&b, authctxt->server_user); - buffer_put_cstring(&b, authctxt->service); - buffer_put_cstring(&b, authctxt->method->name); - buffer_put_char(&b, have_sig); - buffer_put_cstring(&b, key_sign_encode(id->key)); - buffer_put_string(&b, blob, bloblen); + debug3("%s: %s %s", __func__, sshkey_type(id->key), fp); /* * If the key is an certificate, try to find a matching private key * and use it to complete the signature. * If no such private key exists, fall back to trying the certificate * key itself in case it has a private half already loaded. + * This will try to set sign_id to the private key that will perform + * the signature. */ - if (key_is_cert(id->key)) { - matched = 0; + if (sshkey_is_cert(id->key)) { TAILQ_FOREACH(private_id, &authctxt->keys, next) { if (sshkey_equal_public(id->key, private_id->key) && id->key->type != private_id->key->type) { - id = private_id; - matched = 1; + sign_id = private_id; break; } } @@ -1147,18 +1131,18 @@ sign_and_send_pubkey(Authctxt *authctxt, Identity *id) * of keeping just a private key file and public * certificate on disk. */ - if (!matched && !id->isprivate && id->agent_fd == -1 && + if (sign_id == NULL && + !id->isprivate && id->agent_fd == -1 && (id->key->flags & SSHKEY_FLAG_EXT) == 0) { TAILQ_FOREACH(private_id, &authctxt->keys, next) { if (private_id->key == NULL && id_filename_matches(id, private_id)) { - id = private_id; - matched = 1; + sign_id = private_id; break; } } } - if (matched) { + if (sign_id != NULL) { debug2("%s: using private key \"%s\"%s for " "certificate", __func__, id->filename, id->agent_fd != -1 ? " from agent" : ""); @@ -1168,51 +1152,121 @@ sign_and_send_pubkey(Authctxt *authctxt, Identity *id) } } - /* generate signature */ - ret = identity_sign(id, &signature, &slen, - buffer_ptr(&b), buffer_len(&b), datafellows); - if (ret != 0) { - if (ret != SSH_ERR_KEY_NOT_FOUND) - error("%s: signing failed: %s", __func__, ssh_err(ret)); - free(blob); - buffer_free(&b); - return 0; + /* + * If the above didn't select another identity to do the signing + * then default to the one we started with. + */ + if (sign_id == NULL) + sign_id = id; + + /* assemble and sign data */ + for (fallback_sigtype = 0; fallback_sigtype <= 1; fallback_sigtype++) { + free(alg); + slen = 0; + signature = NULL; + if ((alg = key_sig_algorithm(fallback_sigtype ? NULL : ssh, + id->key)) == NULL) { + error("%s: no mutual signature supported", __func__); + goto out; + } + debug3("%s: signing using %s", __func__, alg); + + sshbuf_free(b); + if ((b = sshbuf_new()) == NULL) + fatal("%s: sshbuf_new failed", __func__); + if (datafellows & SSH_OLD_SESSIONID) { + if ((r = sshbuf_put(b, session_id2, + session_id2_len)) != 0) { + fatal("%s: sshbuf_put: %s", + __func__, ssh_err(r)); + } + } else { + if ((r = sshbuf_put_string(b, session_id2, + session_id2_len)) != 0) { + fatal("%s: sshbuf_put_string: %s", + __func__, ssh_err(r)); + } + } + skip = buffer_len(b); + if ((r = sshbuf_put_u8(b, SSH2_MSG_USERAUTH_REQUEST)) != 0 || + (r = sshbuf_put_cstring(b, authctxt->server_user)) != 0 || + (r = sshbuf_put_cstring(b, authctxt->service)) != 0 || + (r = sshbuf_put_cstring(b, authctxt->method->name)) != 0 || + (r = sshbuf_put_u8(b, 1)) != 0 || + (r = sshbuf_put_cstring(b, alg)) != 0 || + (r = sshkey_puts(id->key, b)) != 0) { + fatal("%s: assemble signed data: %s", + __func__, ssh_err(r)); + } + + /* generate signature */ + r = identity_sign(sign_id, &signature, &slen, + sshbuf_ptr(b), sshbuf_len(b), datafellows, alg); + if (r == 0) + break; + else if (r == SSH_ERR_KEY_NOT_FOUND) + goto out; /* soft failure */ + else if (r == SSH_ERR_SIGN_ALG_UNSUPPORTED && + !fallback_sigtype) { + if (sign_id->agent_fd != -1) + loc = "agent "; + else if ((sign_id->key->flags & SSHKEY_FLAG_EXT) != 0) + loc = "token "; + logit("%skey %s %s returned incorrect signature type", + loc, sshkey_type(id->key), fp); + continue; + } + error("%s: signing failed: %s", __func__, ssh_err(r)); + goto out; } -#ifdef DEBUG_PK - buffer_dump(&b); -#endif - free(blob); + if (slen == 0 || signature == NULL) /* shouldn't happen */ + fatal("%s: no signature", __func__); /* append signature */ - buffer_put_string(&b, signature, slen); - free(signature); + if ((r = sshbuf_put_string(b, signature, slen)) != 0) + fatal("%s: append signature: %s", __func__, ssh_err(r)); +#ifdef DEBUG_PK + sshbuf_dump(b, stderr); +#endif /* skip session id and packet type */ - if (buffer_len(&b) < skip + 1) - fatal("userauth_pubkey: internal error"); - buffer_consume(&b, skip + 1); + if ((r = sshbuf_consume(b, skip + 1)) != 0) + fatal("%s: consume: %s", __func__, ssh_err(r)); /* put remaining data from buffer into packet */ - packet_start(SSH2_MSG_USERAUTH_REQUEST); - packet_put_raw(buffer_ptr(&b), buffer_len(&b)); - buffer_free(&b); - packet_send(); + if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_REQUEST)) != 0 || + (r = sshpkt_putb(ssh, b)) != 0 || + (r = sshpkt_send(ssh)) != 0) + fatal("%s: enqueue request: %s", __func__, ssh_err(r)); - return 1; + /* success */ + sent = 1; + + out: + free(fp); + free(alg); + sshbuf_free(b); + freezero(signature, slen); + return sent; } static int -send_pubkey_test(Authctxt *authctxt, Identity *id) +send_pubkey_test(struct ssh *ssh, Authctxt *authctxt, Identity *id) { - u_char *blob; + u_char *blob = NULL; u_int bloblen, have_sig = 0; + char *alg = NULL; + int sent = 0; - debug3("send_pubkey_test"); + if ((alg = key_sig_algorithm(ssh, id->key)) == NULL) { + debug("%s: no mutual signature algorithm", __func__); + goto out; + } if (key_to_blob(id->key, &blob, &bloblen) == 0) { /* we cannot handle this key */ - debug3("send_pubkey_test: cannot handle key"); - return 0; + debug3("%s: cannot handle key", __func__); + goto out; } /* register callback for USERAUTH_PK_OK message */ dispatch_set(SSH2_MSG_USERAUTH_PK_OK, &input_userauth_pk_ok); @@ -1222,11 +1276,15 @@ send_pubkey_test(Authctxt *authctxt, Identity *id) packet_put_cstring(authctxt->service); packet_put_cstring(authctxt->method->name); packet_put_char(have_sig); - packet_put_cstring(key_sign_encode(id->key)); + packet_put_cstring(alg); packet_put_string(blob, bloblen); - free(blob); packet_send(); - return 1; + /* success */ + sent = 1; +out: + free(alg); + free(blob); + return sent; } static struct sshkey * @@ -1295,6 +1353,36 @@ load_identity_file(Identity *id) return private; } +static int +key_type_allowed_by_config(struct sshkey *key) +{ + if (match_pattern_list(sshkey_ssh_name(key), + options.pubkey_key_types, 0) == 1) + return 1; + + /* RSA keys/certs might be allowed by alternate signature types */ + switch (key->type) { + case KEY_RSA: + if (match_pattern_list("rsa-sha2-512", + options.pubkey_key_types, 0) == 1) + return 1; + if (match_pattern_list("rsa-sha2-256", + options.pubkey_key_types, 0) == 1) + return 1; + break; + case KEY_RSA_CERT: + if (match_pattern_list("rsa-sha2-512-cert-v01@openssh.com", + options.pubkey_key_types, 0) == 1) + return 1; + if (match_pattern_list("rsa-sha2-256-cert-v01@openssh.com", + options.pubkey_key_types, 0) == 1) + return 1; + break; + } + return 0; +} + + /* * try keys in the following order: * 1. certificates listed in the config file @@ -1419,9 +1507,7 @@ pubkey_prepare(Authctxt *authctxt) } /* finally, filter by PubkeyAcceptedKeyTypes */ TAILQ_FOREACH_SAFE(id, preferred, next, id2) { - if (id->key != NULL && - match_pattern_list(sshkey_ssh_name(id->key), - options.pubkey_key_types, 0) != 1) { + if (id->key != NULL && !key_type_allowed_by_config(key)) { debug("Skipping %s key %s - " "not in PubkeyAcceptedKeyTypes", sshkey_ssh_name(id->key), id->filename); @@ -1479,6 +1565,7 @@ try_identity(Identity *id) int userauth_pubkey(Authctxt *authctxt) { + struct ssh *ssh = active_state; /* XXX */ Identity *id; int sent = 0; char *fp; @@ -1506,7 +1593,7 @@ userauth_pubkey(Authctxt *authctxt) debug("Offering public key: %s %s %s", sshkey_type(id->key), fp, id->filename); free(fp); - sent = send_pubkey_test(authctxt, id); + sent = send_pubkey_test(ssh, authctxt, id); } } else { debug("Trying private key: %s", id->filename); @@ -1514,7 +1601,7 @@ userauth_pubkey(Authctxt *authctxt) if (id->key != NULL) { if (try_identity(id)) { id->isprivate = 1; - sent = sign_and_send_pubkey( + sent = sign_and_send_pubkey(ssh, authctxt, id); } key_free(id->key); @@ -1735,7 +1822,7 @@ ssh_keysign(struct sshkey *key, u_char **sigp, size_t *lenp, int userauth_hostbased(Authctxt *authctxt) { - struct ssh *ssh = active_state; + struct ssh *ssh = active_state; /* XXX */ struct sshkey *private = NULL; struct sshbuf *b = NULL; u_char *sig = NULL, *keyblob = NULL; -- cgit v1.2.3