From df11c6cb80238fb43c4a5994161cc2e70521ffb4 Mon Sep 17 00:00:00 2001

From: Petr Mensik <pemensik@redhat.com>

Date: Tue, 18 Aug 2020 10:54:39 +0200

Subject: [PATCH] Fix CVE-2020-8623

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]

---

 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 c192b53..49ff58d 100644

--- a/lib/dns/pkcs11dh_link.c

+++ b/lib/dns/pkcs11dh_link.c

@@ -747,6 +747,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);

@@ -851,7 +852,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)

@@ -1011,6 +1016,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;

@@ -1081,7 +1087,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 8d0fc62..3a8a0bb 100644

--- a/lib/dns/pkcs11dsa_link.c

+++ b/lib/dns/pkcs11dsa_link.c

@@ -982,6 +982,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;

@@ -1071,7 +1072,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 af6008d..9a736dd 100644

--- a/lib/dns/pkcs11rsa_link.c

+++ b/lib/dns/pkcs11rsa_link.c

@@ -333,6 +333,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.

@@ -376,6 +377,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);

@@ -396,12 +398,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 = ISC_FALSE;

 	PK11_RET(pkcs_C_CreateObject,

@@ -1075,6 +1081,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);

@@ -1083,10 +1090,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;

@@ -1463,6 +1472,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)

@@ -1480,9 +1491,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);

@@ -1491,16 +1500,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);

@@ -1550,9 +1561,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

@@ -1731,6 +1745,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);

@@ -1815,7 +1830,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);

@@ -1901,6 +1920,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;;

@@ -2044,12 +2064,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));

@@ -2084,6 +2114,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);

@@ -2178,12 +2209,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 603712a..b9680bc 100644

--- a/lib/isc/include/pk11/internal.h

+++ b/lib/isc/include/pk11/internal.h

@@ -27,7 +27,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 48e1031..792b2e3 100644

--- a/lib/isc/pk11.c

+++ b/lib/isc/pk11.c

@@ -985,13 +985,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];

@@ -999,25 +1001,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 *

-- 

2.26.2