From: =?utf-8?b?T25kxZllaiBTdXLDvQ==?= Date: Tue, 21 Jul 2020 14:42:47 +0200 Subject: Fix crash in pk11_numbits() when native-pkcs11 is used origin: https://gitlab.isc.org/isc-projects/bind9/commit/8d807cc21655eaa6e6a08afafeec3682c0f3f2ab Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2020-8623 When pk11_numbits() is passed a user provided input that contains all zeroes (via crafted DNS message), it would crash with assertion failure. Fix that by properly handling such input. [Salvatore Bonaccorso: Backport to 99.11.5.P4 which does not contain 9d15323e2484 ("Add small tweaks to the code to fix compilation when ISC assertions are disabled")] --- lib/dns/pkcs11dh_link.c | 15 ++++++-- lib/dns/pkcs11dsa_link.c | 8 ++++- lib/dns/pkcs11rsa_link.c | 79 +++++++++++++++++++++++++++++++---------- lib/isc/include/pk11/internal.h | 3 +- lib/isc/pk11.c | 60 ++++++++++++++++++++----------- 5 files changed, 121 insertions(+), 44 deletions(-) diff --git a/lib/dns/pkcs11dh_link.c b/lib/dns/pkcs11dh_link.c index e2b60ea..4cd8e32 100644 --- a/lib/dns/pkcs11dh_link.c +++ b/lib/dns/pkcs11dh_link.c @@ -748,6 +748,7 @@ pkcs11dh_fromdns(dst_key_t *key, isc_buffer_t *data) { CK_BYTE *prime = NULL, *base = NULL, *pub = NULL; CK_ATTRIBUTE *attr; int special = 0; + unsigned int bits; isc_result_t result; isc_buffer_remainingregion(data, &r); @@ -852,7 +853,11 @@ pkcs11dh_fromdns(dst_key_t *key, isc_buffer_t *data) { pub = r.base; isc_region_consume(&r, publen); - key->key_size = pk11_numbits(prime, plen_); + result = pk11_numbits(prime, plen_, &bits); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + key->key_size = bits; dh->repr = (CK_ATTRIBUTE *) isc_mem_get(key->mctx, sizeof(*attr) * 3); if (dh->repr == NULL) @@ -1012,6 +1017,7 @@ pkcs11dh_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { dst_private_t priv; isc_result_t ret; int i; + unsigned int bits; pk11_object_t *dh = NULL; CK_ATTRIBUTE *attr; isc_mem_t *mctx; @@ -1082,7 +1088,12 @@ pkcs11dh_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { attr = pk11_attribute_bytype(dh, CKA_PRIME); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; return (ISC_R_SUCCESS); diff --git a/lib/dns/pkcs11dsa_link.c b/lib/dns/pkcs11dsa_link.c index 12d707a..24d4c14 100644 --- a/lib/dns/pkcs11dsa_link.c +++ b/lib/dns/pkcs11dsa_link.c @@ -983,6 +983,7 @@ pkcs11dsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { dst_private_t priv; isc_result_t ret; int i; + unsigned int bits; pk11_object_t *dsa = NULL; CK_ATTRIBUTE *attr; isc_mem_t *mctx = key->mctx; @@ -1072,7 +1073,12 @@ pkcs11dsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { attr = pk11_attribute_bytype(dsa, CKA_PRIME); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; return (ISC_R_SUCCESS); diff --git a/lib/dns/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c index eb782c8..57ee41d 100644 --- a/lib/dns/pkcs11rsa_link.c +++ b/lib/dns/pkcs11rsa_link.c @@ -330,6 +330,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, key->key_alg == DST_ALG_RSASHA256 || key->key_alg == DST_ALG_RSASHA512); #endif + REQUIRE(maxbits <= RSA_MAX_PUBEXP_BITS); /* * Reject incorrect RSA key lengths. @@ -373,6 +374,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, for (attr = pk11_attribute_first(rsa); attr != NULL; attr = pk11_attribute_next(rsa, attr)) + { switch (attr->type) { case CKA_MODULUS: INSIST(keyTemplate[5].type == attr->type); @@ -393,12 +395,16 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, memmove(keyTemplate[6].pValue, attr->pValue, attr->ulValueLen); keyTemplate[6].ulValueLen = attr->ulValueLen; - if (pk11_numbits(attr->pValue, - attr->ulValueLen) > maxbits && - maxbits != 0) + unsigned int bits; + ret = pk11_numbits(attr->pValue, attr->ulValueLen, + &bits); + if (ret != ISC_R_SUCCESS || + (bits > maxbits && maxbits != 0)) { DST_RET(DST_R_VERIFYFAILURE); + } break; } + } pk11_ctx->object = CK_INVALID_HANDLE; pk11_ctx->ontoken = false; PK11_RET(pkcs_C_CreateObject, @@ -1063,6 +1069,7 @@ pkcs11rsa_verify(dst_context_t *dctx, const isc_region_t *sig) { keyTemplate[5].ulValueLen = attr->ulValueLen; break; case CKA_PUBLIC_EXPONENT: + unsigned int bits; INSIST(keyTemplate[6].type == attr->type); keyTemplate[6].pValue = isc_mem_get(dctx->mctx, attr->ulValueLen); @@ -1071,10 +1078,12 @@ pkcs11rsa_verify(dst_context_t *dctx, const isc_region_t *sig) { memmove(keyTemplate[6].pValue, attr->pValue, attr->ulValueLen); keyTemplate[6].ulValueLen = attr->ulValueLen; - if (pk11_numbits(attr->pValue, - attr->ulValueLen) - > RSA_MAX_PUBEXP_BITS) + ret = pk11_numbits(attr->pValue, attr->ulValueLen, + &bits); + if (ret != ISC_R_SUCCESS || bits > RSA_MAX_PUBEXP_BITS) + { DST_RET(DST_R_VERIFYFAILURE); + } break; } pk11_ctx->object = CK_INVALID_HANDLE; @@ -1451,6 +1460,8 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { CK_BYTE *exponent = NULL, *modulus = NULL; CK_ATTRIBUTE *attr; unsigned int length; + unsigned int bits; + isc_result_t ret = ISC_R_SUCCESS; isc_buffer_remainingregion(data, &r); if (r.length == 0) @@ -1468,9 +1479,7 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { if (e_bytes == 0) { if (r.length < 2) { - isc_safe_memwipe(rsa, sizeof(*rsa)); - isc_mem_put(key->mctx, rsa, sizeof(*rsa)); - return (DST_R_INVALIDPUBLICKEY); + DST_RET(DST_R_INVALIDPUBLICKEY); } e_bytes = (*r.base) << 8; isc_region_consume(&r, 1); @@ -1479,16 +1488,18 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { } if (r.length < e_bytes) { - isc_safe_memwipe(rsa, sizeof(*rsa)); - isc_mem_put(key->mctx, rsa, sizeof(*rsa)); - return (DST_R_INVALIDPUBLICKEY); + DST_RET(DST_R_INVALIDPUBLICKEY); } exponent = r.base; isc_region_consume(&r, e_bytes); modulus = r.base; mod_bytes = r.length; - key->key_size = pk11_numbits(modulus, mod_bytes); + ret = pk11_numbits(modulus, mod_bytes, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; isc_buffer_forward(data, length); @@ -1538,9 +1549,12 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { rsa->repr, rsa->attrcnt * sizeof(*attr)); } + ret = ISC_R_NOMEMORY; + + err: isc_safe_memwipe(rsa, sizeof(*rsa)); isc_mem_put(key->mctx, rsa, sizeof(*rsa)); - return (ISC_R_NOMEMORY); + return (ret); } static isc_result_t @@ -1719,6 +1733,7 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label, pk11_object_t *pubrsa; pk11_context_t *pk11_ctx = NULL; isc_result_t ret; + unsigned int bits; if (label == NULL) return (DST_R_NOENGINE); @@ -1803,7 +1818,11 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label, attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; return (ISC_R_SUCCESS); @@ -1889,6 +1908,7 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { CK_ATTRIBUTE *attr; isc_mem_t *mctx = key->mctx; const char *engine = NULL, *label = NULL; + unsigned int bits; /* read private key file */ ret = dst__privstruct_parse(key, DST_ALG_RSA, lexer, mctx, &priv); @@ -2032,12 +2052,22 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT); INSIST(attr != NULL); - if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS) + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + if (bits > RSA_MAX_PUBEXP_BITS) { DST_RET(ISC_R_RANGE); + } dst__privstruct_free(&priv, mctx); isc_safe_memwipe(&priv, sizeof(priv)); @@ -2072,6 +2102,7 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label, pk11_context_t *pk11_ctx = NULL; isc_result_t ret; unsigned int i; + unsigned int bits; UNUSED(pin); @@ -2166,12 +2197,22 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label, attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT); INSIST(attr != NULL); - if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS) + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + if (bits > RSA_MAX_PUBEXP_BITS) { DST_RET(ISC_R_RANGE); + } attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; pk11_return_session(pk11_ctx); isc_safe_memwipe(pk11_ctx, sizeof(*pk11_ctx)); diff --git a/lib/isc/include/pk11/internal.h b/lib/isc/include/pk11/internal.h index aa8907a..7cc8ec8 100644 --- a/lib/isc/include/pk11/internal.h +++ b/lib/isc/include/pk11/internal.h @@ -25,7 +25,8 @@ void pk11_mem_put(void *ptr, size_t size); CK_SLOT_ID pk11_get_best_token(pk11_optype_t optype); -unsigned int pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt); +isc_result_t +pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits); CK_ATTRIBUTE *pk11_attribute_first(const pk11_object_t *obj); diff --git a/lib/isc/pk11.c b/lib/isc/pk11.c index c5d2310..b2ab6be 100644 --- a/lib/isc/pk11.c +++ b/lib/isc/pk11.c @@ -962,13 +962,15 @@ pk11_get_best_token(pk11_optype_t optype) { return (token->slotid); } -unsigned int -pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) { +isc_result_t +pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits) { unsigned int bitcnt, i; CK_BYTE top; - if (bytecnt == 0) - return (0); + if (bytecnt == 0) { + *bits = 0; + return (ISC_R_SUCCESS); + } bitcnt = bytecnt * 8; for (i = 0; i < bytecnt; i++) { top = data[i]; @@ -976,25 +978,41 @@ pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) { bitcnt -= 8; continue; } - if (top & 0x80) - return (bitcnt); - if (top & 0x40) - return (bitcnt - 1); - if (top & 0x20) - return (bitcnt - 2); - if (top & 0x10) - return (bitcnt - 3); - if (top & 0x08) - return (bitcnt - 4); - if (top & 0x04) - return (bitcnt - 5); - if (top & 0x02) - return (bitcnt - 6); - if (top & 0x01) - return (bitcnt - 7); + if (top & 0x80) { + *bits = bitcnt; + return (ISC_R_SUCCESS); + } + if (top & 0x40) { + *bits = bitcnt - 1; + return (ISC_R_SUCCESS); + } + if (top & 0x20) { + *bits = bitcnt - 2; + return (ISC_R_SUCCESS); + } + if (top & 0x10) { + *bits = bitcnt - 3; + return (ISC_R_SUCCESS); + } + if (top & 0x08) { + *bits = bitcnt - 4; + return (ISC_R_SUCCESS); + } + if (top & 0x04) { + *bits = bitcnt - 5; + return (ISC_R_SUCCESS); + } + if (top & 0x02) { + *bits = bitcnt - 6; + return (ISC_R_SUCCESS); + } + if (top & 0x01) { + *bits = bitcnt - 7; + return (ISC_R_SUCCESS); + } break; } - INSIST(0); + return (ISC_R_RANGE); } CK_ATTRIBUTE *