ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Fix parsing ECDSA keys

Browse Source 
raw_key_to_ossl() assumes fixed ECDSA private key sizes (32 bytes for
ECDSAP256SHA256, 48 bytes for ECDSAP384SHA384).  Meanwhile, in rare
cases, ECDSAP256SHA256 private keys are representable in 31 bytes or
less (similarly for ECDSAP384SHA384) and that is how they are then
stored in the "PrivateKey" field of the key file.  Nevertheless,
raw_key_to_ossl() always calls BN_bin2bn() with a fixed length argument,
which in the cases mentioned above leads to erroneously interpreting
uninitialized memory as a part of the private key.  This results in the
latter being malformed and broken signatures being generated.  Address
by using the key length provided by the caller rather than a fixed one.
Apply the same change to public key parsing code for consistency, adding
an INSIST() to prevent buffer overruns.
This commit is contained in:
Michał Kępień
2021-11-19 10:32:21 +01:00
committed by Mark Andrews
parent dbeea1afa0
commit a482a6b204
1 changed files with 6 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
lib/dns/opensslecdsa_link.c
View File

@@ -66,24 +66,16 @@ raw_key_to_ossl(unsigned int key_alg, int private, const unsigned char *key,
OSSL_PARAM *params = NULL;
EVP_PKEY_CTX *ctx = NULL;
BIGNUM *priv = NULL;
size_t len = 0;
unsigned char buf[DNS_KEY_ECDSA384SIZE + 1];
if (key_alg == DST_ALG_ECDSA256) {
groupname = "P-256";
len = private ? DNS_KEY_ECDSA256SIZE / 2 : DNS_KEY_ECDSA256SIZE;
} else if (key_alg == DST_ALG_ECDSA384) {
groupname = "P-384";
len = private ? DNS_KEY_ECDSA384SIZE / 2 : DNS_KEY_ECDSA384SIZE;
} else {
DST_RET(ISC_R_NOTIMPLEMENTED);
}
ret = (private ? DST_R_INVALIDPRIVATEKEY : DST_R_INVALIDPUBLICKEY);
if (*key_len < len) {
DST_RET(ret);
}
bld = OSSL_PARAM_BLD_new();
if (bld == NULL) {
DST_RET(dst__openssl_toresult2("OSSL_PARAM_BLD_new",
@@ -98,7 +90,7 @@ raw_key_to_ossl(unsigned int key_alg, int private, const unsigned char *key,
}
if (private) {
priv = BN_bin2bn(key, len, NULL);
priv = BN_bin2bn(key, *key_len, NULL);
if (priv == NULL) {
DST_RET(dst__openssl_toresult2("BN_bin2bn",
DST_R_OPENSSLFAILURE));
@@ -111,11 +103,12 @@ raw_key_to_ossl(unsigned int key_alg, int private, const unsigned char *key,
DST_R_OPENSSLFAILURE));
}
} else {
INSIST(*key_len < sizeof(buf));
buf[0] = POINT_CONVERSION_UNCOMPRESSED;
memmove(buf + 1, key, len);
memmove(buf + 1, key, *key_len);
status = OSSL_PARAM_BLD_push_octet_string(
bld, OSSL_PKEY_PARAM_PUB_KEY, buf, 1 + len);
bld, OSSL_PKEY_PARAM_PUB_KEY, buf, 1 + *key_len);
if (status != 1) {
DST_RET(dst__openssl_toresult2("OSSL_PARAM_BLD_push_"
"octet_string",
@@ -146,7 +139,6 @@ raw_key_to_ossl(unsigned int key_alg, int private, const unsigned char *key,
DST_R_OPENSSLFAILURE));
}
*key_len = len;
ret = ISC_R_SUCCESS;
err:
@@ -1184,8 +1176,6 @@ opensslecdsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
#if OPENSSL_VERSION_NUMBER < 0x30000000L
EC_KEY *eckey = NULL;
EC_KEY *pubeckey = NULL;
#else
size_t len;
#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
const char *engine = NULL;
const char *label = NULL;
@@ -1257,14 +1247,9 @@ opensslecdsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
key->keydata.pkey = NULL;
}
if (key->key_alg == DST_ALG_ECDSA256) {
len = DNS_KEY_ECDSA256SIZE / 2;
} else {
len = DNS_KEY_ECDSA384SIZE / 2;
}
ret = raw_key_to_ossl(key->key_alg, 1,
priv.elements[privkey_index].data, &len,
priv.elements[privkey_index].data,
&priv.elements[privkey_index].length,
&key->keydata.pkey);
#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */