diff --git a/CHANGES b/CHANGES index cabd1eac71..27cf14612f 100644 --- a/CHANGES +++ b/CHANGES @@ -14,7 +14,11 @@ 5481. [placeholder] -5480. [placeholder] +5480. [security] When BIND 9 was compiled with native PKCS#11 support, it + was possible to trigger an assertion failure in code + determining the number of bits in the PKCS#11 RSA public + key with a specially crafted packet. (CVE-2020-8623) + [GL #2037] 5479. [security] named could crash in certain query resolution scenarios where QNAME minimization and forwarding were both diff --git a/bin/tests/system/run.sh.in b/bin/tests/system/run.sh.in index c59a3e2cfd..06c8d72011 100644 --- a/bin/tests/system/run.sh.in +++ b/bin/tests/system/run.sh.in @@ -70,7 +70,7 @@ if ! $do_run; then if [ "$baseport" -eq 0 ]; then log_flags="$log_flags -p 5300" fi - env - PATH="$PATH" ${LD_LIBRARY_PATH:+"LD_LIBRARY_PATH=${LD_LIBRARY_PATH}"} TESTS="$*" TEST_SUITE_LOG=run.log LOG_DRIVER_FLAGS="--verbose yes --color-tests yes" LOG_FLAGS="$log_flags" make -e check + env - SLOT="$SLOT" SOFTHSM2_CONF="$SOFTHSM2_CONF" PATH="$PATH" ${LD_LIBRARY_PATH:+"LD_LIBRARY_PATH=${LD_LIBRARY_PATH}"} TESTS="$*" TEST_SUITE_LOG=run.log LOG_DRIVER_FLAGS="--verbose yes --color-tests yes" LOG_FLAGS="$log_flags" make -e check exit $? fi diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index f5fdc44bee..175a15b362 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -36,6 +36,14 @@ Security Fixes ISC would like to thank Dave Feldman, Jeff Warren, and Joel Cunningham of Oracle for bringing this vulnerability to our attention. [GL #2028] +- When BIND 9 was compiled with native PKCS#11 support, it was possible + to trigger an assertion failure in code determining the number of bits + in the PKCS#11 RSA public key with a specially crafted packet. This + was disclosed in CVE-2020-8623. + + ISC would like to thank Lyu Chiy for bringing this vulnerability to + our attention. [GL #2037] + Known Issues ~~~~~~~~~~~~ diff --git a/lib/dns/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c index fc5efe3bb8..c31ae33da9 100644 --- a/lib/dns/pkcs11rsa_link.c +++ b/lib/dns/pkcs11rsa_link.c @@ -292,6 +292,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, key->key_alg == DST_ALG_NSEC3RSASHA1 || key->key_alg == DST_ALG_RSASHA256 || key->key_alg == DST_ALG_RSASHA512); + REQUIRE(maxbits <= RSA_MAX_PUBEXP_BITS); /* * Reject incorrect RSA key lengths. @@ -334,6 +335,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); @@ -350,13 +352,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, @@ -957,14 +962,17 @@ 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); 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; @@ -1335,6 +1343,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) { @@ -1351,9 +1361,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); @@ -1362,16 +1370,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); @@ -1391,6 +1401,10 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { key->keydata.pkey = rsa; return (ISC_R_SUCCESS); +err: + isc_safe_memwipe(rsa, sizeof(*rsa)); + isc_mem_put(key->mctx, rsa, sizeof(*rsa)); + return (ret); } static isc_result_t @@ -1564,6 +1578,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); @@ -1642,7 +1657,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); @@ -1734,6 +1753,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); @@ -1871,12 +1891,20 @@ 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); } @@ -1911,6 +1939,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); @@ -1996,14 +2025,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 dfdf526eeb..05be8997d3 100644 --- a/lib/isc/include/pk11/internal.h +++ b/lib/isc/include/pk11/internal.h @@ -30,8 +30,8 @@ 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 bc910d7fdf..315f1170e9 100644 --- a/lib/isc/pk11.c +++ b/lib/isc/pk11.c @@ -652,13 +652,14 @@ 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); + *bits = 0; + return (ISC_R_SUCCESS); } bitcnt = bytecnt * 8; for (i = 0; i < bytecnt; i++) { @@ -668,33 +669,40 @@ pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) { continue; } if (top & 0x80) { - return (bitcnt); + *bits = bitcnt; + return (ISC_R_SUCCESS); } if (top & 0x40) { - return (bitcnt - 1); + *bits = bitcnt - 1; + return (ISC_R_SUCCESS); } if (top & 0x20) { - return (bitcnt - 2); + *bits = bitcnt - 2; + return (ISC_R_SUCCESS); } if (top & 0x10) { - return (bitcnt - 3); + *bits = bitcnt - 3; + return (ISC_R_SUCCESS); } if (top & 0x08) { - return (bitcnt - 4); + *bits = bitcnt - 4; + return (ISC_R_SUCCESS); } if (top & 0x04) { - return (bitcnt - 5); + *bits = bitcnt - 5; + return (ISC_R_SUCCESS); } if (top & 0x02) { - return (bitcnt - 6); + *bits = bitcnt - 6; + return (ISC_R_SUCCESS); } if (top & 0x01) { - return (bitcnt - 7); + *bits = bitcnt - 7; + return (ISC_R_SUCCESS); } break; } - INSIST(0); - ISC_UNREACHABLE(); + return (ISC_R_RANGE); } CK_ATTRIBUTE *