Blame SOURCES/bind99-CVE-2017-3142+3143.patch

900526
diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
900526
index 00a0080..336c4da 100644
900526
--- a/lib/dns/dnssec.c
900526
+++ b/lib/dns/dnssec.c
900526
@@ -982,6 +982,8 @@ dns_dnssec_verifymessage(isc_buffer_t *source, dns_message_t *msg,
900526
 	mctx = msg->mctx;
900526
 
900526
 	msg->verify_attempted = 1;
900526
+	msg->verified_sig = 0;
900526
+	msg->sig0status = dns_tsigerror_badsig;
900526
 
900526
 	if (is_response(msg)) {
900526
 		if (msg->query.base == NULL)
900526
@@ -1076,6 +1078,7 @@ dns_dnssec_verifymessage(isc_buffer_t *source, dns_message_t *msg,
900526
 	}
900526
 
900526
 	msg->verified_sig = 1;
900526
+	msg->sig0status = dns_rcode_noerror;
900526
 
900526
 	dst_context_destroy(&ctx;;
900526
 	dns_rdata_freestruct(&sig);
900526
diff --git a/lib/dns/message.c b/lib/dns/message.c
900526
index 1417067..0621175 100644
900526
--- a/lib/dns/message.c
900526
+++ b/lib/dns/message.c
900526
@@ -3052,12 +3052,19 @@ dns_message_signer(dns_message_t *msg, dns_name_t *signer) {
900526
 
900526
 		result = dns_rdata_tostruct(&rdata, &tsig, NULL);
900526
 		INSIST(result == ISC_R_SUCCESS);
900526
-		if (msg->tsigstatus != dns_rcode_noerror)
900526
+		if (msg->verified_sig &&
900526
+		    msg->tsigstatus == dns_rcode_noerror &&
900526
+		    tsig.error == dns_rcode_noerror)
900526
+		{
900526
+			result = ISC_R_SUCCESS;
900526
+		} else if ((!msg->verified_sig) ||
900526
+			   (msg->tsigstatus != dns_rcode_noerror))
900526
+		{
900526
 			result = DNS_R_TSIGVERIFYFAILURE;
900526
-		else if (tsig.error != dns_rcode_noerror)
900526
+		} else {
900526
+			INSIST(tsig.error != dns_rcode_noerror);
900526
 			result = DNS_R_TSIGERRORSET;
900526
-		else
900526
-			result = ISC_R_SUCCESS;
900526
+		}
900526
 		dns_rdata_freestruct(&tsig);
900526
 
900526
 		if (msg->tsigkey == NULL) {
900526
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
900526
index 3239bff..7b91d1e 100644
900526
--- a/lib/dns/tsig.c
900526
+++ b/lib/dns/tsig.c
900526
@@ -941,11 +941,20 @@ dns_tsig_sign(dns_message_t *msg) {
900526
 		isc_buffer_putuint48(&otherbuf, tsig.timesigned);
900526
 	}
900526
 
900526
-	if (key->key != NULL && tsig.error != dns_tsigerror_badsig) {
900526
+	if ((key->key != NULL) &&
900526
+	    (tsig.error != dns_tsigerror_badsig) &&
900526
+	    (tsig.error != dns_tsigerror_badkey))
900526
+	{
900526
 		unsigned char header[DNS_MESSAGE_HEADERLEN];
900526
 		isc_buffer_t headerbuf;
900526
 		isc_uint16_t digestbits;
900526
 
900526
+		/*
900526
+		 * If it is a response, we assume that the request MAC
900526
+		 * has validated at this point. This is why we include a
900526
+		 * MAC length > 0 in the reply.
900526
+		 */
900526
+
900526
 		ret = dst_context_create3(key->key, mctx,
900526
 					  DNS_LOGCATEGORY_DNSSEC,
900526
 					  ISC_TRUE, &ctx;;
900526
@@ -953,7 +962,7 @@ dns_tsig_sign(dns_message_t *msg) {
900526
 			return (ret);
900526
 
900526
 		/*
900526
-		 * If this is a response, digest the query signature.
900526
+		 * If this is a response, digest the request's MAC.
900526
 		 */
900526
 		if (response) {
900526
 			dns_rdata_t querytsigrdata = DNS_RDATA_INIT;
900526
@@ -1083,6 +1092,17 @@ dns_tsig_sign(dns_message_t *msg) {
900526
 		dst_context_destroy(&ctx;;
900526
 		digestbits = dst_key_getbits(key->key);
900526
 		if (digestbits != 0) {
900526
+			/*
900526
+			 * XXXRAY: Is this correct? What is the
900526
+			 * expected behavior when digestbits is not an
900526
+			 * integral multiple of 8? It looks like bytes
900526
+			 * should either be (digestbits/8) or
900526
+			 * (digestbits+7)/8.
900526
+			 *
900526
+			 * In any case, for current algorithms,
900526
+			 * digestbits are an integral multiple of 8, so
900526
+			 * it has the same effect as (digestbits/8).
900526
+			 */
900526
 			unsigned int bytes = (digestbits + 1) / 8;
900526
 			if (response && bytes < querytsig.siglen)
900526
 				bytes = querytsig.siglen;
900526
@@ -1196,6 +1216,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg,
900526
 	REQUIRE(tsigkey == NULL || VALID_TSIG_KEY(tsigkey));
900526
 
900526
 	msg->verify_attempted = 1;
900526
+	msg->verified_sig = 0;
900526
+	msg->tsigstatus = dns_tsigerror_badsig;
900526
 
900526
 	if (msg->tcp_continuation) {
900526
 		if (tsigkey == NULL || msg->querytsig == NULL)
900526
@@ -1294,19 +1316,6 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg,
900526
 	key = tsigkey->key;
900526
 
900526
 	/*
900526
-	 * Is the time ok?
900526
-	 */
900526
-	if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) {
900526
-		msg->tsigstatus = dns_tsigerror_badtime;
900526
-		tsig_log(msg->tsigkey, 2, "signature has expired");
900526
-		return (DNS_R_CLOCKSKEW);
900526
-	} else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) {
900526
-		msg->tsigstatus = dns_tsigerror_badtime;
900526
-		tsig_log(msg->tsigkey, 2, "signature is in the future");
900526
-		return (DNS_R_CLOCKSKEW);
900526
-	}
900526
-
900526
-	/*
900526
 	 * Check digest length.
900526
 	 */
900526
 	alg = dst_key_alg(key);
900526
@@ -1315,31 +1324,19 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg,
900526
 		return (ret);
900526
 	if (alg == DST_ALG_HMACMD5 || alg == DST_ALG_HMACSHA1 ||
900526
 	    alg == DST_ALG_HMACSHA224 || alg == DST_ALG_HMACSHA256 ||
900526
-	    alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512) {
900526
-		isc_uint16_t digestbits = dst_key_getbits(key);
900526
+	    alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512)
900526
+	{
900526
 		if (tsig.siglen > siglen) {
900526
-			tsig_log(msg->tsigkey, 2, "signature length to big");
900526
+			tsig_log(msg->tsigkey, 2, "signature length too big");
900526
 			return (DNS_R_FORMERR);
900526
 		}
900526
 		if (tsig.siglen > 0 &&
900526
-		    (tsig.siglen < 10 || tsig.siglen < ((siglen + 1) / 2))) {
900526
+		    (tsig.siglen < 10 || tsig.siglen < ((siglen + 1) / 2)))
900526
+		{
900526
 			tsig_log(msg->tsigkey, 2,
900526
 				 "signature length below minimum");
900526
 			return (DNS_R_FORMERR);
900526
 		}
900526
-		if (tsig.siglen > 0 && digestbits != 0 &&
900526
-		    tsig.siglen < ((digestbits + 1) / 8)) {
900526
-			msg->tsigstatus = dns_tsigerror_badtrunc;
900526
-			tsig_log(msg->tsigkey, 2,
900526
-				 "truncated signature length too small");
900526
-			return (DNS_R_TSIGVERIFYFAILURE);
900526
-		}
900526
-		if (tsig.siglen > 0 && digestbits == 0 &&
900526
-		    tsig.siglen < siglen) {
900526
-			msg->tsigstatus = dns_tsigerror_badtrunc;
900526
-			tsig_log(msg->tsigkey, 2, "signature length too small");
900526
-			return (DNS_R_TSIGVERIFYFAILURE);
900526
-		}
900526
 	}
900526
 
900526
 	if (tsig.siglen > 0) {
900526
@@ -1451,34 +1448,92 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg,
900526
 
900526
 		ret = dst_context_verify(ctx, &sig_r);
900526
 		if (ret == DST_R_VERIFYFAILURE) {
900526
-			msg->tsigstatus = dns_tsigerror_badsig;
900526
 			ret = DNS_R_TSIGVERIFYFAILURE;
900526
 			tsig_log(msg->tsigkey, 2,
900526
 				 "signature failed to verify(1)");
900526
 			goto cleanup_context;
900526
-		} else if (ret != ISC_R_SUCCESS)
900526
+		} else if (ret != ISC_R_SUCCESS) {
900526
 			goto cleanup_context;
900526
-
900526
-		dst_context_destroy(&ctx;;
900526
+		}
900526
 	} else if (tsig.error != dns_tsigerror_badsig &&
900526
 		   tsig.error != dns_tsigerror_badkey) {
900526
-		msg->tsigstatus = dns_tsigerror_badsig;
900526
 		tsig_log(msg->tsigkey, 2, "signature was empty");
900526
 		return (DNS_R_TSIGVERIFYFAILURE);
900526
 	}
900526
 
900526
-	msg->tsigstatus = dns_rcode_noerror;
900526
+	/*
900526
+	 * Here at this point, the MAC has been verified. Even if any of
900526
+	 * the following code returns a TSIG error, the reply will be
900526
+	 * signed and WILL always include the request MAC in the digest
900526
+	 * computation.
900526
+	 */
900526
+
900526
+	/*
900526
+	 * Is the time ok?
900526
+	 */
900526
+	if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) {
900526
+		msg->tsigstatus = dns_tsigerror_badtime;
900526
+		tsig_log(msg->tsigkey, 2, "signature has expired");
900526
+		ret = DNS_R_CLOCKSKEW;
900526
+		goto cleanup_context;
900526
+	} else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) {
900526
+		msg->tsigstatus = dns_tsigerror_badtime;
900526
+		tsig_log(msg->tsigkey, 2, "signature is in the future");
900526
+		ret = DNS_R_CLOCKSKEW;
900526
+		goto cleanup_context;
900526
+	}
900526
+
900526
+	if (
900526
+#ifndef PK11_MD5_DISABLE
900526
+	    alg == DST_ALG_HMACMD5 ||
900526
+#endif
900526
+	    alg == DST_ALG_HMACSHA1 ||
900526
+	    alg == DST_ALG_HMACSHA224 || alg == DST_ALG_HMACSHA256 ||
900526
+	    alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512)
900526
+	{
900526
+		isc_uint16_t digestbits = dst_key_getbits(key);
900526
+
900526
+		/*
900526
+		 * XXXRAY: Is this correct? What is the expected
900526
+		 * behavior when digestbits is not an integral multiple
900526
+		 * of 8? It looks like bytes should either be
900526
+		 * (digestbits/8) or (digestbits+7)/8.
900526
+		 *
900526
+		 * In any case, for current algorithms, digestbits are
900526
+		 * an integral multiple of 8, so it has the same effect
900526
+		 * as (digestbits/8).
900526
+		 */
900526
+		if (tsig.siglen > 0 && digestbits != 0 &&
900526
+		    tsig.siglen < ((digestbits + 1) / 8))
900526
+		{
900526
+			msg->tsigstatus = dns_tsigerror_badtrunc;
900526
+			tsig_log(msg->tsigkey, 2,
900526
+				 "truncated signature length too small");
900526
+			ret = DNS_R_TSIGVERIFYFAILURE;
900526
+			goto cleanup_context;
900526
+		}
900526
+		if (tsig.siglen > 0 && digestbits == 0 &&
900526
+		    tsig.siglen < siglen)
900526
+		{
900526
+			msg->tsigstatus = dns_tsigerror_badtrunc;
900526
+			tsig_log(msg->tsigkey, 2, "signature length too small");
900526
+			ret = DNS_R_TSIGVERIFYFAILURE;
900526
+			goto cleanup_context;
900526
+		}
900526
+	}
900526
 
900526
 	if (tsig.error != dns_rcode_noerror) {
900526
+		msg->tsigstatus = tsig.error;
900526
 		if (tsig.error == dns_tsigerror_badtime)
900526
-			return (DNS_R_CLOCKSKEW);
900526
+			ret = DNS_R_CLOCKSKEW;
900526
 		else
900526
-			return (DNS_R_TSIGERRORSET);
900526
+			ret = DNS_R_TSIGERRORSET;
900526
+		goto cleanup_context;
900526
 	}
900526
 
900526
+	msg->tsigstatus = dns_rcode_noerror;
900526
 	msg->verified_sig = 1;
900526
-
900526
-	return (ISC_R_SUCCESS);
900526
+	ret = ISC_R_SUCCESS;
900526
 
900526
 cleanup_context:
900526
 	if (ctx != NULL)
900526
@@ -1503,6 +1558,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 	isc_uint16_t addcount, id;
900526
 	isc_boolean_t has_tsig = ISC_FALSE;
900526
 	isc_mem_t *mctx;
900526
+	unsigned int siglen;
900526
+	unsigned int alg;
900526
 
900526
 	REQUIRE(source != NULL);
900526
 	REQUIRE(msg != NULL);
900526
@@ -1510,12 +1567,16 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 	REQUIRE(msg->tcp_continuation == 1);
900526
 	REQUIRE(msg->querytsig != NULL);
900526
 
900526
+	msg->verified_sig = 0;
900526
+	msg->tsigstatus = dns_tsigerror_badsig;
900526
+
900526
 	if (!is_response(msg))
900526
 		return (DNS_R_EXPECTEDRESPONSE);
900526
 
900526
 	mctx = msg->mctx;
900526
 
900526
 	tsigkey = dns_message_gettsigkey(msg);
900526
+	key = tsigkey->key;
900526
 
900526
 	/*
900526
 	 * Extract and parse the previous TSIG
900526
@@ -1548,7 +1609,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 		 * Do the key name and algorithm match that of the query?
900526
 		 */
900526
 		if (!dns_name_equal(keyname, &tsigkey->name) ||
900526
-		    !dns_name_equal(&tsig.algorithm, &querytsig.algorithm)) {
900526
+		    !dns_name_equal(&tsig.algorithm, &querytsig.algorithm))
900526
+		{
900526
 			msg->tsigstatus = dns_tsigerror_badkey;
900526
 			ret = DNS_R_TSIGVERIFYFAILURE;
900526
 			tsig_log(msg->tsigkey, 2,
900526
@@ -1557,27 +1619,40 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 		}
900526
 
900526
 		/*
900526
-		 * Is the time ok?
900526
+		 * Check digest length.
900526
 		 */
900526
-		isc_stdtime_get(&now;;
900526
-
900526
-		if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) {
900526
-			msg->tsigstatus = dns_tsigerror_badtime;
900526
-			tsig_log(msg->tsigkey, 2, "signature has expired");
900526
-			ret = DNS_R_CLOCKSKEW;
900526
-			goto cleanup_querystruct;
900526
-		} else if (now + msg->timeadjust <
900526
-			   tsig.timesigned - tsig.fudge) {
900526
-			msg->tsigstatus = dns_tsigerror_badtime;
900526
-			tsig_log(msg->tsigkey, 2,
900526
-				 "signature is in the future");
900526
-			ret = DNS_R_CLOCKSKEW;
900526
+		alg = dst_key_alg(key);
900526
+		ret = dst_key_sigsize(key, &siglen);
900526
+		if (ret != ISC_R_SUCCESS)
900526
 			goto cleanup_querystruct;
900526
+		if (
900526
+#ifndef PK11_MD5_DISABLE
900526
+			alg == DST_ALG_HMACMD5 ||
900526
+#endif
900526
+			alg == DST_ALG_HMACSHA1 ||
900526
+			alg == DST_ALG_HMACSHA224 ||
900526
+			alg == DST_ALG_HMACSHA256 ||
900526
+			alg == DST_ALG_HMACSHA384 ||
900526
+			alg == DST_ALG_HMACSHA512)
900526
+		{
900526
+			if (tsig.siglen > siglen) {
900526
+				tsig_log(tsigkey, 2,
900526
+					 "signature length too big");
900526
+				ret = DNS_R_FORMERR;
900526
+				goto cleanup_querystruct;
900526
+			}
900526
+			if (tsig.siglen > 0 &&
900526
+			    (tsig.siglen < 10 ||
900526
+			     tsig.siglen < ((siglen + 1) / 2)))
900526
+			{
900526
+				tsig_log(tsigkey, 2,
900526
+					 "signature length below minimum");
900526
+				ret = DNS_R_FORMERR;
900526
+				goto cleanup_querystruct;
900526
+			}
900526
 		}
900526
 	}
900526
 
900526
-	key = tsigkey->key;
900526
-
900526
 	if (msg->tsigctx == NULL) {
900526
 		ret = dst_context_create3(key, mctx,
900526
 					  DNS_LOGCATEGORY_DNSSEC,
900526
@@ -1670,10 +1745,12 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 		sig_r.length = tsig.siglen;
900526
 		if (tsig.siglen == 0) {
900526
 			if (tsig.error != dns_rcode_noerror) {
900526
-				if (tsig.error == dns_tsigerror_badtime)
900526
+				msg->tsigstatus = tsig.error;
900526
+				if (tsig.error == dns_tsigerror_badtime) {
900526
 					ret = DNS_R_CLOCKSKEW;
900526
-				else
900526
+				} else {
900526
 					ret = DNS_R_TSIGERRORSET;
900526
+				}
900526
 			} else {
900526
 				tsig_log(msg->tsigkey, 2,
900526
 					 "signature is empty");
900526
@@ -1684,29 +1761,111 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) {
900526
 
900526
 		ret = dst_context_verify(msg->tsigctx, &sig_r);
900526
 		if (ret == DST_R_VERIFYFAILURE) {
900526
-			msg->tsigstatus = dns_tsigerror_badsig;
900526
 			tsig_log(msg->tsigkey, 2,
900526
 				 "signature failed to verify(2)");
900526
 			ret = DNS_R_TSIGVERIFYFAILURE;
900526
 			goto cleanup_context;
900526
+		} else if (ret != ISC_R_SUCCESS) {
900526
+			goto cleanup_context;
900526
 		}
900526
-		else if (ret != ISC_R_SUCCESS)
900526
+
900526
+		/*
900526
+		 * Here at this point, the MAC has been verified. Even
900526
+		 * if any of the following code returns a TSIG error,
900526
+		 * the reply will be signed and WILL always include the
900526
+		 * request MAC in the digest computation.
900526
+		 */
900526
+
900526
+		/*
900526
+		 * Is the time ok?
900526
+		 */
900526
+		isc_stdtime_get(&now;;
900526
+
900526
+		if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) {
900526
+			msg->tsigstatus = dns_tsigerror_badtime;
900526
+			tsig_log(msg->tsigkey, 2, "signature has expired");
900526
+			ret = DNS_R_CLOCKSKEW;
900526
+			goto cleanup_context;
900526
+		} else if (now + msg->timeadjust <
900526
+			   tsig.timesigned - tsig.fudge)
900526
+		{
900526
+			msg->tsigstatus = dns_tsigerror_badtime;
900526
+			tsig_log(msg->tsigkey, 2,
900526
+				 "signature is in the future");
900526
+			ret = DNS_R_CLOCKSKEW;
900526
 			goto cleanup_context;
900526
+		}
900526
 
900526
-		dst_context_destroy(&msg->tsigctx);
900526
+		alg = dst_key_alg(key);
900526
+		ret = dst_key_sigsize(key, &siglen);
900526
+		if (ret != ISC_R_SUCCESS)
900526
+			goto cleanup_context;
900526
+		if (
900526
+#ifndef PK11_MD5_DISABLE
900526
+			alg == DST_ALG_HMACMD5 ||
900526
+#endif
900526
+			alg == DST_ALG_HMACSHA1 ||
900526
+			alg == DST_ALG_HMACSHA224 ||
900526
+			alg == DST_ALG_HMACSHA256 ||
900526
+			alg == DST_ALG_HMACSHA384 ||
900526
+			alg == DST_ALG_HMACSHA512)
900526
+		{
900526
+			isc_uint16_t digestbits = dst_key_getbits(key);
900526
+
900526
+			/*
900526
+			 * XXXRAY: Is this correct? What is the
900526
+			 * expected behavior when digestbits is not an
900526
+			 * integral multiple of 8? It looks like bytes
900526
+			 * should either be (digestbits/8) or
900526
+			 * (digestbits+7)/8.
900526
+			 *
900526
+			 * In any case, for current algorithms,
900526
+			 * digestbits are an integral multiple of 8, so
900526
+			 * it has the same effect as (digestbits/8).
900526
+			 */
900526
+			if (tsig.siglen > 0 && digestbits != 0 &&
900526
+			    tsig.siglen < ((digestbits + 1) / 8))
900526
+			{
900526
+				msg->tsigstatus = dns_tsigerror_badtrunc;
900526
+				tsig_log(msg->tsigkey, 2,
900526
+					 "truncated signature length "
900526
+					 "too small");
900526
+				ret = DNS_R_TSIGVERIFYFAILURE;
900526
+				goto cleanup_context;
900526
+			}
900526
+			if (tsig.siglen > 0 && digestbits == 0 &&
900526
+			    tsig.siglen < siglen)
900526
+			{
900526
+				msg->tsigstatus = dns_tsigerror_badtrunc;
900526
+				tsig_log(msg->tsigkey, 2,
900526
+					 "signature length too small");
900526
+				ret = DNS_R_TSIGVERIFYFAILURE;
900526
+				goto cleanup_context;
900526
+			}
900526
+		}
900526
+
900526
+		if (tsig.error != dns_rcode_noerror) {
900526
+			msg->tsigstatus = tsig.error;
900526
+			if (tsig.error == dns_tsigerror_badtime)
900526
+				ret = DNS_R_CLOCKSKEW;
900526
+			else
900526
+				ret = DNS_R_TSIGERRORSET;
900526
+			goto cleanup_context;
900526
+		}
900526
 	}
900526
 
900526
 	msg->tsigstatus = dns_rcode_noerror;
900526
-	return (ISC_R_SUCCESS);
900526
+	msg->verified_sig = 1;
900526
+	ret = ISC_R_SUCCESS;
900526
 
900526
  cleanup_context:
900526
-	dst_context_destroy(&msg->tsigctx);
900526
+	if (msg->tsigctx != NULL)
900526
+		dst_context_destroy(&msg->tsigctx);
900526
 
900526
  cleanup_querystruct:
900526
 	dns_rdata_freestruct(&querytsig);
900526
 
900526
 	return (ret);
900526
-
900526
 }
900526
 
900526
 isc_result_t