upstream: Refactor private key parsing. Eliminates a fair bit of
duplicated code and fixes oss-fuzz#20074 (NULL deref) caused by a missing key type check in the ECDSA_CERT parsing path. feedback and ok markus@ OpenBSD-Commit-ID: 4711981d88afb7196d228f7baad9be1d3b20f9c9
This commit is contained in:
parent
b6a4013647
commit
3779b50ee9
187
sshkey.c
187
sshkey.c
|
@ -1,4 +1,4 @@
|
||||||
/* $OpenBSD: sshkey.c,v 1.107 2020/04/08 00:08:46 djm Exp $ */
|
/* $OpenBSD: sshkey.c,v 1.108 2020/04/11 10:16:11 djm Exp $ */
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2000, 2001 Markus Friedl. All rights reserved.
|
* Copyright (c) 2000, 2001 Markus Friedl. All rights reserved.
|
||||||
* Copyright (c) 2008 Alexander von Gernler. All rights reserved.
|
* Copyright (c) 2008 Alexander von Gernler. All rights reserved.
|
||||||
|
@ -3394,38 +3394,52 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
if ((r = sshbuf_get_cstring(buf, &tname, NULL)) != 0)
|
if ((r = sshbuf_get_cstring(buf, &tname, NULL)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
type = sshkey_type_from_name(tname);
|
type = sshkey_type_from_name(tname);
|
||||||
switch (type) {
|
if (sshkey_type_is_cert(type)) {
|
||||||
#ifdef WITH_OPENSSL
|
/*
|
||||||
case KEY_DSA:
|
* Certificate key private keys begin with the certificate
|
||||||
|
* itself. Make sure this matches the type of the enclosing
|
||||||
|
* private key.
|
||||||
|
*/
|
||||||
|
if ((r = sshkey_froms(buf, &k)) != 0)
|
||||||
|
goto out;
|
||||||
|
if (k->type != type) {
|
||||||
|
r = SSH_ERR_KEY_CERT_MISMATCH;
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
/* For ECDSA keys, the group must match too */
|
||||||
|
if (k->type == KEY_ECDSA &&
|
||||||
|
k->ecdsa_nid != sshkey_ecdsa_nid_from_name(tname)) {
|
||||||
|
r = SSH_ERR_KEY_CERT_MISMATCH;
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
if ((k = sshkey_new(type)) == NULL) {
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
r = SSH_ERR_ALLOC_FAIL;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
switch (type) {
|
||||||
|
#ifdef WITH_OPENSSL
|
||||||
|
case KEY_DSA:
|
||||||
if ((r = sshbuf_get_bignum2(buf, &dsa_p)) != 0 ||
|
if ((r = sshbuf_get_bignum2(buf, &dsa_p)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &dsa_q)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &dsa_q)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &dsa_g)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &dsa_g)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &dsa_pub_key)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &dsa_pub_key)) != 0)
|
||||||
(r = sshbuf_get_bignum2(buf, &dsa_priv_key)) != 0)
|
|
||||||
goto out;
|
goto out;
|
||||||
if (!DSA_set0_pqg(k->dsa, dsa_p, dsa_q, dsa_g)) {
|
if (!DSA_set0_pqg(k->dsa, dsa_p, dsa_q, dsa_g)) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
dsa_p = dsa_q = dsa_g = NULL; /* transferred */
|
dsa_p = dsa_q = dsa_g = NULL; /* transferred */
|
||||||
if (!DSA_set0_key(k->dsa, dsa_pub_key, dsa_priv_key)) {
|
if (!DSA_set0_key(k->dsa, dsa_pub_key, NULL)) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
dsa_pub_key = dsa_priv_key = NULL; /* transferred */
|
dsa_pub_key = NULL; /* transferred */
|
||||||
break;
|
/* FALLTHROUGH */
|
||||||
case KEY_DSA_CERT:
|
case KEY_DSA_CERT:
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
if ((r = sshbuf_get_bignum2(buf, &dsa_priv_key)) != 0)
|
||||||
(r = sshbuf_get_bignum2(buf, &dsa_priv_key)) != 0)
|
|
||||||
goto out;
|
goto out;
|
||||||
if (k->type != type) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (!DSA_set0_key(k->dsa, NULL, dsa_priv_key)) {
|
if (!DSA_set0_key(k->dsa, NULL, dsa_priv_key)) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3434,10 +3448,6 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
break;
|
break;
|
||||||
# ifdef OPENSSL_HAS_ECC
|
# ifdef OPENSSL_HAS_ECC
|
||||||
case KEY_ECDSA:
|
case KEY_ECDSA:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((k->ecdsa_nid = sshkey_ecdsa_nid_from_name(tname)) == -1) {
|
if ((k->ecdsa_nid = sshkey_ecdsa_nid_from_name(tname)) == -1) {
|
||||||
r = SSH_ERR_INVALID_ARGUMENT;
|
r = SSH_ERR_INVALID_ARGUMENT;
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3453,27 +3463,12 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
if ((r = sshbuf_get_eckey(buf, k->ecdsa)) != 0 ||
|
if ((r = sshbuf_get_eckey(buf, k->ecdsa)) != 0)
|
||||||
(r = sshbuf_get_bignum2(buf, &exponent)))
|
|
||||||
goto out;
|
goto out;
|
||||||
if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1) {
|
/* FALLTHROUGH */
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshkey_ec_validate_public(EC_KEY_get0_group(k->ecdsa),
|
|
||||||
EC_KEY_get0_public_key(k->ecdsa))) != 0 ||
|
|
||||||
(r = sshkey_ec_validate_private(k->ecdsa)) != 0)
|
|
||||||
goto out;
|
|
||||||
break;
|
|
||||||
case KEY_ECDSA_CERT:
|
case KEY_ECDSA_CERT:
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
if ((r = sshbuf_get_bignum2(buf, &exponent)) != 0)
|
||||||
(r = sshbuf_get_bignum2(buf, &exponent)) != 0)
|
|
||||||
goto out;
|
goto out;
|
||||||
if (k->type != type ||
|
|
||||||
k->ecdsa_nid != sshkey_ecdsa_nid_from_name(tname)) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1) {
|
if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3484,10 +3479,6 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
goto out;
|
goto out;
|
||||||
break;
|
break;
|
||||||
case KEY_ECDSA_SK:
|
case KEY_ECDSA_SK:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((k->ecdsa_nid = sshkey_ecdsa_nid_from_name(tname)) == -1) {
|
if ((k->ecdsa_nid = sshkey_ecdsa_nid_from_name(tname)) == -1) {
|
||||||
r = SSH_ERR_INVALID_ARGUMENT;
|
r = SSH_ERR_INVALID_ARGUMENT;
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3520,8 +3511,6 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
goto out;
|
goto out;
|
||||||
break;
|
break;
|
||||||
case KEY_ECDSA_SK_CERT:
|
case KEY_ECDSA_SK_CERT:
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0)
|
|
||||||
goto out;
|
|
||||||
if ((k->sk_key_handle = sshbuf_new()) == NULL ||
|
if ((k->sk_key_handle = sshbuf_new()) == NULL ||
|
||||||
(k->sk_reserved = sshbuf_new()) == NULL) {
|
(k->sk_reserved = sshbuf_new()) == NULL) {
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
r = SSH_ERR_ALLOC_FAIL;
|
||||||
|
@ -3539,43 +3528,21 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
break;
|
break;
|
||||||
# endif /* OPENSSL_HAS_ECC */
|
# endif /* OPENSSL_HAS_ECC */
|
||||||
case KEY_RSA:
|
case KEY_RSA:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshbuf_get_bignum2(buf, &rsa_n)) != 0 ||
|
if ((r = sshbuf_get_bignum2(buf, &rsa_n)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_e)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &rsa_e)) != 0)
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_d)) != 0 ||
|
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_iqmp)) != 0 ||
|
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_p)) != 0 ||
|
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_q)) != 0)
|
|
||||||
goto out;
|
goto out;
|
||||||
if (!RSA_set0_key(k->rsa, rsa_n, rsa_e, rsa_d)) {
|
if (!RSA_set0_key(k->rsa, rsa_n, rsa_e, NULL)) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
rsa_n = rsa_e = rsa_d = NULL; /* transferred */
|
rsa_n = rsa_e = NULL; /* transferred */
|
||||||
if (!RSA_set0_factors(k->rsa, rsa_p, rsa_q)) {
|
/* FALLTHROUGH */
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
rsa_p = rsa_q = NULL; /* transferred */
|
|
||||||
if ((r = check_rsa_length(k->rsa)) != 0)
|
|
||||||
goto out;
|
|
||||||
if ((r = ssh_rsa_complete_crt_parameters(k, rsa_iqmp)) != 0)
|
|
||||||
goto out;
|
|
||||||
break;
|
|
||||||
case KEY_RSA_CERT:
|
case KEY_RSA_CERT:
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
if ((r = sshbuf_get_bignum2(buf, &rsa_d)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_d)) != 0 ||
|
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_iqmp)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &rsa_iqmp)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_p)) != 0 ||
|
(r = sshbuf_get_bignum2(buf, &rsa_p)) != 0 ||
|
||||||
(r = sshbuf_get_bignum2(buf, &rsa_q)) != 0)
|
(r = sshbuf_get_bignum2(buf, &rsa_q)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
if (k->type != type) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (!RSA_set0_key(k->rsa, NULL, NULL, rsa_d)) {
|
if (!RSA_set0_key(k->rsa, NULL, NULL, rsa_d)) {
|
||||||
r = SSH_ERR_LIBCRYPTO_ERROR;
|
r = SSH_ERR_LIBCRYPTO_ERROR;
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3593,10 +3560,7 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
break;
|
break;
|
||||||
#endif /* WITH_OPENSSL */
|
#endif /* WITH_OPENSSL */
|
||||||
case KEY_ED25519:
|
case KEY_ED25519:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
case KEY_ED25519_CERT:
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0 ||
|
if ((r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0 ||
|
||||||
(r = sshbuf_get_string(buf, &ed25519_sk, &sklen)) != 0)
|
(r = sshbuf_get_string(buf, &ed25519_sk, &sklen)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3606,30 +3570,10 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
}
|
}
|
||||||
k->ed25519_pk = ed25519_pk;
|
k->ed25519_pk = ed25519_pk;
|
||||||
k->ed25519_sk = ed25519_sk;
|
k->ed25519_sk = ed25519_sk;
|
||||||
ed25519_pk = ed25519_sk = NULL;
|
|
||||||
break;
|
|
||||||
case KEY_ED25519_CERT:
|
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
|
||||||
(r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0 ||
|
|
||||||
(r = sshbuf_get_string(buf, &ed25519_sk, &sklen)) != 0)
|
|
||||||
goto out;
|
|
||||||
if (k->type != type) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (pklen != ED25519_PK_SZ || sklen != ED25519_SK_SZ) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
k->ed25519_pk = ed25519_pk;
|
|
||||||
k->ed25519_sk = ed25519_sk;
|
|
||||||
ed25519_pk = ed25519_sk = NULL; /* transferred */
|
ed25519_pk = ed25519_sk = NULL; /* transferred */
|
||||||
break;
|
break;
|
||||||
case KEY_ED25519_SK:
|
case KEY_ED25519_SK:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
case KEY_ED25519_SK_CERT:
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0)
|
if ((r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
if (pklen != ED25519_PK_SZ) {
|
if (pklen != ED25519_PK_SZ) {
|
||||||
|
@ -3648,40 +3592,11 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
(r = sshbuf_get_stringb(buf, k->sk_reserved)) != 0)
|
(r = sshbuf_get_stringb(buf, k->sk_reserved)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
k->ed25519_pk = ed25519_pk;
|
k->ed25519_pk = ed25519_pk;
|
||||||
ed25519_pk = NULL;
|
|
||||||
break;
|
|
||||||
case KEY_ED25519_SK_CERT:
|
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
|
||||||
(r = sshbuf_get_string(buf, &ed25519_pk, &pklen)) != 0)
|
|
||||||
goto out;
|
|
||||||
if (k->type != type) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (pklen != ED25519_PK_SZ) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((k->sk_key_handle = sshbuf_new()) == NULL ||
|
|
||||||
(k->sk_reserved = sshbuf_new()) == NULL) {
|
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshbuf_get_cstring(buf, &k->sk_application,
|
|
||||||
NULL)) != 0 ||
|
|
||||||
(r = sshbuf_get_u8(buf, &k->sk_flags)) != 0 ||
|
|
||||||
(r = sshbuf_get_stringb(buf, k->sk_key_handle)) != 0 ||
|
|
||||||
(r = sshbuf_get_stringb(buf, k->sk_reserved)) != 0)
|
|
||||||
goto out;
|
|
||||||
k->ed25519_pk = ed25519_pk;
|
|
||||||
ed25519_pk = NULL; /* transferred */
|
ed25519_pk = NULL; /* transferred */
|
||||||
break;
|
break;
|
||||||
#ifdef WITH_XMSS
|
#ifdef WITH_XMSS
|
||||||
case KEY_XMSS:
|
case KEY_XMSS:
|
||||||
if ((k = sshkey_new(type)) == NULL) {
|
case KEY_XMSS_CERT:
|
||||||
r = SSH_ERR_ALLOC_FAIL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if ((r = sshbuf_get_cstring(buf, &xmss_name, NULL)) != 0 ||
|
if ((r = sshbuf_get_cstring(buf, &xmss_name, NULL)) != 0 ||
|
||||||
(r = sshkey_xmss_init(k, xmss_name)) != 0 ||
|
(r = sshkey_xmss_init(k, xmss_name)) != 0 ||
|
||||||
(r = sshbuf_get_string(buf, &xmss_pk, &pklen)) != 0 ||
|
(r = sshbuf_get_string(buf, &xmss_pk, &pklen)) != 0 ||
|
||||||
|
@ -3699,28 +3614,6 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
|
||||||
if ((r = sshkey_xmss_deserialize_state_opt(k, buf)) != 0)
|
if ((r = sshkey_xmss_deserialize_state_opt(k, buf)) != 0)
|
||||||
goto out;
|
goto out;
|
||||||
break;
|
break;
|
||||||
case KEY_XMSS_CERT:
|
|
||||||
if ((r = sshkey_froms(buf, &k)) != 0 ||
|
|
||||||
(r = sshbuf_get_cstring(buf, &xmss_name, NULL)) != 0 ||
|
|
||||||
(r = sshbuf_get_string(buf, &xmss_pk, &pklen)) != 0 ||
|
|
||||||
(r = sshbuf_get_string(buf, &xmss_sk, &sklen)) != 0)
|
|
||||||
goto out;
|
|
||||||
if (k->type != type || strcmp(xmss_name, k->xmss_name) != 0) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
if (pklen != sshkey_xmss_pklen(k) ||
|
|
||||||
sklen != sshkey_xmss_sklen(k)) {
|
|
||||||
r = SSH_ERR_INVALID_FORMAT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
k->xmss_pk = xmss_pk;
|
|
||||||
k->xmss_sk = xmss_sk;
|
|
||||||
xmss_pk = xmss_sk = NULL;
|
|
||||||
/* optional internal state */
|
|
||||||
if ((r = sshkey_xmss_deserialize_state_opt(k, buf)) != 0)
|
|
||||||
goto out;
|
|
||||||
break;
|
|
||||||
#endif /* WITH_XMSS */
|
#endif /* WITH_XMSS */
|
||||||
default:
|
default:
|
||||||
r = SSH_ERR_KEY_TYPE_UNKNOWN;
|
r = SSH_ERR_KEY_TYPE_UNKNOWN;
|
||||||
|
|
Loading…
Reference in New Issue