From 43448a770a6f1d126c71785b93dee6358ed216a2b6de644193a6fe333eddd1c7 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Fri, 30 Jun 2017 10:58:48 +0000 Subject: [PATCH 1/2] Accepting request 507232 from home:simotek:branches:network - Added bind-CVE-2017-3142-and-3143.patch to fix a security issue where an attacker with the ability to send and receive messages to an authoritative DNS server was able to circumvent TSIG authentication of AXFR requests. A server that relies solely on TSIG keys for protection with no other ACL protection could be manipulated into (1) providing an AXFR of a zone to an unauthorized recipient and (2) accepting bogus Notify packets. [bsc#1046554, CVE-2017-3142, bsc#1046555, CVE-2017-3143] OBS-URL: https://build.opensuse.org/request/show/507232 OBS-URL: https://build.opensuse.org/package/show/network/bind?expand=0&rev=211 --- bind-CVE-2017-3142-and-3143.patch | 496 ++++++++++++++++++++++++++++++ bind.changes | 12 + bind.spec | 2 + 3 files changed, 510 insertions(+) create mode 100644 bind-CVE-2017-3142-and-3143.patch diff --git a/bind-CVE-2017-3142-and-3143.patch b/bind-CVE-2017-3142-and-3143.patch new file mode 100644 index 0000000..2f4735c --- /dev/null +++ b/bind-CVE-2017-3142-and-3143.patch @@ -0,0 +1,496 @@ +Index: bind-9.10.4-P5/lib/dns/dnssec.c +=================================================================== +--- bind-9.10.4-P5.orig/lib/dns/dnssec.c ++++ bind-9.10.4-P5/lib/dns/dnssec.c +@@ -978,6 +978,8 @@ dns_dnssec_verifymessage(isc_buffer_t *s + mctx = msg->mctx; + + msg->verify_attempted = 1; ++ msg->verified_sig = 0; ++ msg->sig0status = dns_tsigerror_badsig; + + if (is_response(msg)) { + if (msg->query.base == NULL) +@@ -1073,6 +1075,7 @@ dns_dnssec_verifymessage(isc_buffer_t *s + } + + msg->verified_sig = 1; ++ msg->sig0status = dns_rcode_noerror; + + dst_context_destroy(&ctx); + dns_rdata_freestruct(&sig); +Index: bind-9.10.4-P5/lib/dns/message.c +=================================================================== +--- bind-9.10.4-P5.orig/lib/dns/message.c ++++ bind-9.10.4-P5/lib/dns/message.c +@@ -3055,12 +3055,19 @@ dns_message_signer(dns_message_t *msg, d + + result = dns_rdata_tostruct(&rdata, &tsig, NULL); + INSIST(result == ISC_R_SUCCESS); +- if (msg->tsigstatus != dns_rcode_noerror) ++ if (msg->verified_sig && ++ msg->tsigstatus == dns_rcode_noerror && ++ tsig.error == dns_rcode_noerror) ++ { ++ result = ISC_R_SUCCESS; ++ } else if ((!msg->verified_sig) || ++ (msg->tsigstatus != dns_rcode_noerror)) ++ { + result = DNS_R_TSIGVERIFYFAILURE; +- else if (tsig.error != dns_rcode_noerror) ++ } else { ++ INSIST(tsig.error != dns_rcode_noerror); + result = DNS_R_TSIGERRORSET; +- else +- result = ISC_R_SUCCESS; ++ } + dns_rdata_freestruct(&tsig); + + if (msg->tsigkey == NULL) { +Index: bind-9.10.4-P5/lib/dns/tsig.c +=================================================================== +--- bind-9.10.4-P5.orig/lib/dns/tsig.c ++++ bind-9.10.4-P5/lib/dns/tsig.c +@@ -942,11 +942,20 @@ dns_tsig_sign(dns_message_t *msg) { + isc_buffer_putuint48(&otherbuf, tsig.timesigned); + } + +- if (key->key != NULL && tsig.error != dns_tsigerror_badsig) { ++ if ((key->key != NULL) && ++ (tsig.error != dns_tsigerror_badsig) && ++ (tsig.error != dns_tsigerror_badkey)) ++ { + unsigned char header[DNS_MESSAGE_HEADERLEN]; + isc_buffer_t headerbuf; + isc_uint16_t digestbits; + ++ /* ++ * If it is a response, we assume that the request MAC ++ * has validated at this point. This is why we include a ++ * MAC length > 0 in the reply. ++ */ ++ + ret = dst_context_create3(key->key, mctx, + DNS_LOGCATEGORY_DNSSEC, + ISC_TRUE, &ctx); +@@ -954,7 +963,7 @@ dns_tsig_sign(dns_message_t *msg) { + return (ret); + + /* +- * If this is a response, digest the query signature. ++ * If this is a response, digest the request's MAC. + */ + if (response) { + dns_rdata_t querytsigrdata = DNS_RDATA_INIT; +@@ -1084,6 +1093,17 @@ dns_tsig_sign(dns_message_t *msg) { + dst_context_destroy(&ctx); + digestbits = dst_key_getbits(key->key); + if (digestbits != 0) { ++ /* ++ * XXXRAY: Is this correct? What is the ++ * expected behavior when digestbits is not an ++ * integral multiple of 8? It looks like bytes ++ * should either be (digestbits/8) or ++ * (digestbits+7)/8. ++ * ++ * In any case, for current algorithms, ++ * digestbits are an integral multiple of 8, so ++ * it has the same effect as (digestbits/8). ++ */ + unsigned int bytes = (digestbits + 1) / 8; + if (response && bytes < querytsig.siglen) + bytes = querytsig.siglen; +@@ -1193,6 +1213,8 @@ dns_tsig_verify(isc_buffer_t *source, dn + REQUIRE(tsigkey == NULL || VALID_TSIG_KEY(tsigkey)); + + msg->verify_attempted = 1; ++ msg->verified_sig = 0; ++ msg->tsigstatus = dns_tsigerror_badsig; + + if (msg->tcp_continuation) { + if (tsigkey == NULL || msg->querytsig == NULL) +@@ -1291,19 +1313,6 @@ dns_tsig_verify(isc_buffer_t *source, dn + key = tsigkey->key; + + /* +- * Is the time ok? +- */ +- if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { +- msg->tsigstatus = dns_tsigerror_badtime; +- tsig_log(msg->tsigkey, 2, "signature has expired"); +- return (DNS_R_CLOCKSKEW); +- } else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) { +- msg->tsigstatus = dns_tsigerror_badtime; +- tsig_log(msg->tsigkey, 2, "signature is in the future"); +- return (DNS_R_CLOCKSKEW); +- } +- +- /* + * Check digest length. + */ + alg = dst_key_alg(key); +@@ -1312,31 +1321,19 @@ dns_tsig_verify(isc_buffer_t *source, dn + return (ret); + if (alg == DST_ALG_HMACMD5 || alg == DST_ALG_HMACSHA1 || + alg == DST_ALG_HMACSHA224 || alg == DST_ALG_HMACSHA256 || +- alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512) { +- isc_uint16_t digestbits = dst_key_getbits(key); ++ alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512) ++ { + if (tsig.siglen > siglen) { + tsig_log(msg->tsigkey, 2, "signature length too big"); + return (DNS_R_FORMERR); + } + if (tsig.siglen > 0 && +- (tsig.siglen < 10 || tsig.siglen < ((siglen + 1) / 2))) { ++ (tsig.siglen < 10 || tsig.siglen < ((siglen + 1) / 2))) ++ { + tsig_log(msg->tsigkey, 2, + "signature length below minimum"); + return (DNS_R_FORMERR); + } +- if (tsig.siglen > 0 && digestbits != 0 && +- tsig.siglen < ((digestbits + 1) / 8)) { +- msg->tsigstatus = dns_tsigerror_badtrunc; +- tsig_log(msg->tsigkey, 2, +- "truncated signature length too small"); +- return (DNS_R_TSIGVERIFYFAILURE); +- } +- if (tsig.siglen > 0 && digestbits == 0 && +- tsig.siglen < siglen) { +- msg->tsigstatus = dns_tsigerror_badtrunc; +- tsig_log(msg->tsigkey, 2, "signature length too small"); +- return (DNS_R_TSIGVERIFYFAILURE); +- } + } + + if (tsig.siglen > 0) { +@@ -1451,34 +1448,92 @@ dns_tsig_verify(isc_buffer_t *source, dn + + ret = dst_context_verify(ctx, &sig_r); + if (ret == DST_R_VERIFYFAILURE) { +- msg->tsigstatus = dns_tsigerror_badsig; + ret = DNS_R_TSIGVERIFYFAILURE; + tsig_log(msg->tsigkey, 2, + "signature failed to verify(1)"); + goto cleanup_context; +- } else if (ret != ISC_R_SUCCESS) ++ } else if (ret != ISC_R_SUCCESS) { + goto cleanup_context; +- +- dst_context_destroy(&ctx); ++ } + } else if (tsig.error != dns_tsigerror_badsig && + tsig.error != dns_tsigerror_badkey) { +- msg->tsigstatus = dns_tsigerror_badsig; + tsig_log(msg->tsigkey, 2, "signature was empty"); + return (DNS_R_TSIGVERIFYFAILURE); + } + +- msg->tsigstatus = dns_rcode_noerror; ++ /* ++ * Here at this point, the MAC has been verified. Even if any of ++ * the following code returns a TSIG error, the reply will be ++ * signed and WILL always include the request MAC in the digest ++ * computation. ++ */ ++ ++ /* ++ * Is the time ok? ++ */ ++ if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { ++ msg->tsigstatus = dns_tsigerror_badtime; ++ tsig_log(msg->tsigkey, 2, "signature has expired"); ++ ret = DNS_R_CLOCKSKEW; ++ goto cleanup_context; ++ } else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) { ++ msg->tsigstatus = dns_tsigerror_badtime; ++ tsig_log(msg->tsigkey, 2, "signature is in the future"); ++ ret = DNS_R_CLOCKSKEW; ++ goto cleanup_context; ++ } ++ ++ if ( ++#ifndef PK11_MD5_DISABLE ++ alg == DST_ALG_HMACMD5 || ++#endif ++ alg == DST_ALG_HMACSHA1 || ++ alg == DST_ALG_HMACSHA224 || alg == DST_ALG_HMACSHA256 || ++ alg == DST_ALG_HMACSHA384 || alg == DST_ALG_HMACSHA512) ++ { ++ isc_uint16_t digestbits = dst_key_getbits(key); ++ ++ /* ++ * XXXRAY: Is this correct? What is the expected ++ * behavior when digestbits is not an integral multiple ++ * of 8? It looks like bytes should either be ++ * (digestbits/8) or (digestbits+7)/8. ++ * ++ * In any case, for current algorithms, digestbits are ++ * an integral multiple of 8, so it has the same effect ++ * as (digestbits/8). ++ */ ++ if (tsig.siglen > 0 && digestbits != 0 && ++ tsig.siglen < ((digestbits + 1) / 8)) ++ { ++ msg->tsigstatus = dns_tsigerror_badtrunc; ++ tsig_log(msg->tsigkey, 2, ++ "truncated signature length too small"); ++ ret = DNS_R_TSIGVERIFYFAILURE; ++ goto cleanup_context; ++ } ++ if (tsig.siglen > 0 && digestbits == 0 && ++ tsig.siglen < siglen) ++ { ++ msg->tsigstatus = dns_tsigerror_badtrunc; ++ tsig_log(msg->tsigkey, 2, "signature length too small"); ++ ret = DNS_R_TSIGVERIFYFAILURE; ++ goto cleanup_context; ++ } ++ } + + if (tsig.error != dns_rcode_noerror) { ++ msg->tsigstatus = tsig.error; + if (tsig.error == dns_tsigerror_badtime) +- return (DNS_R_CLOCKSKEW); ++ ret = DNS_R_CLOCKSKEW; + else +- return (DNS_R_TSIGERRORSET); ++ ret = DNS_R_TSIGERRORSET; ++ goto cleanup_context; + } + ++ msg->tsigstatus = dns_rcode_noerror; + msg->verified_sig = 1; +- +- return (ISC_R_SUCCESS); ++ ret = ISC_R_SUCCESS; + + cleanup_context: + if (ctx != NULL) +@@ -1503,6 +1558,8 @@ tsig_verify_tcp(isc_buffer_t *source, dn + isc_uint16_t addcount, id; + isc_boolean_t has_tsig = ISC_FALSE; + isc_mem_t *mctx; ++ unsigned int siglen; ++ unsigned int alg; + + REQUIRE(source != NULL); + REQUIRE(msg != NULL); +@@ -1510,12 +1567,16 @@ tsig_verify_tcp(isc_buffer_t *source, dn + REQUIRE(msg->tcp_continuation == 1); + REQUIRE(msg->querytsig != NULL); + ++ msg->verified_sig = 0; ++ msg->tsigstatus = dns_tsigerror_badsig; ++ + if (!is_response(msg)) + return (DNS_R_EXPECTEDRESPONSE); + + mctx = msg->mctx; + + tsigkey = dns_message_gettsigkey(msg); ++ key = tsigkey->key; + + /* + * Extract and parse the previous TSIG +@@ -1548,7 +1609,8 @@ tsig_verify_tcp(isc_buffer_t *source, dn + * Do the key name and algorithm match that of the query? + */ + if (!dns_name_equal(keyname, &tsigkey->name) || +- !dns_name_equal(&tsig.algorithm, &querytsig.algorithm)) { ++ !dns_name_equal(&tsig.algorithm, &querytsig.algorithm)) ++ { + msg->tsigstatus = dns_tsigerror_badkey; + ret = DNS_R_TSIGVERIFYFAILURE; + tsig_log(msg->tsigkey, 2, +@@ -1557,27 +1619,40 @@ tsig_verify_tcp(isc_buffer_t *source, dn + } + + /* +- * Is the time ok? ++ * Check digest length. + */ +- isc_stdtime_get(&now); +- +- if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { +- msg->tsigstatus = dns_tsigerror_badtime; +- tsig_log(msg->tsigkey, 2, "signature has expired"); +- ret = DNS_R_CLOCKSKEW; +- goto cleanup_querystruct; +- } else if (now + msg->timeadjust < +- tsig.timesigned - tsig.fudge) { +- msg->tsigstatus = dns_tsigerror_badtime; +- tsig_log(msg->tsigkey, 2, +- "signature is in the future"); +- ret = DNS_R_CLOCKSKEW; ++ alg = dst_key_alg(key); ++ ret = dst_key_sigsize(key, &siglen); ++ if (ret != ISC_R_SUCCESS) + goto cleanup_querystruct; ++ if ( ++#ifndef PK11_MD5_DISABLE ++ alg == DST_ALG_HMACMD5 || ++#endif ++ alg == DST_ALG_HMACSHA1 || ++ alg == DST_ALG_HMACSHA224 || ++ alg == DST_ALG_HMACSHA256 || ++ alg == DST_ALG_HMACSHA384 || ++ alg == DST_ALG_HMACSHA512) ++ { ++ if (tsig.siglen > siglen) { ++ tsig_log(tsigkey, 2, ++ "signature length too big"); ++ ret = DNS_R_FORMERR; ++ goto cleanup_querystruct; ++ } ++ if (tsig.siglen > 0 && ++ (tsig.siglen < 10 || ++ tsig.siglen < ((siglen + 1) / 2))) ++ { ++ tsig_log(tsigkey, 2, ++ "signature length below minimum"); ++ ret = DNS_R_FORMERR; ++ goto cleanup_querystruct; ++ } + } + } + +- key = tsigkey->key; +- + if (msg->tsigctx == NULL) { + ret = dst_context_create3(key, mctx, + DNS_LOGCATEGORY_DNSSEC, +@@ -1673,10 +1748,12 @@ tsig_verify_tcp(isc_buffer_t *source, dn + sig_r.length = tsig.siglen; + if (tsig.siglen == 0) { + if (tsig.error != dns_rcode_noerror) { +- if (tsig.error == dns_tsigerror_badtime) ++ msg->tsigstatus = tsig.error; ++ if (tsig.error == dns_tsigerror_badtime) { + ret = DNS_R_CLOCKSKEW; +- else ++ } else { + ret = DNS_R_TSIGERRORSET; ++ } + } else { + tsig_log(msg->tsigkey, 2, + "signature is empty"); +@@ -1687,29 +1764,111 @@ tsig_verify_tcp(isc_buffer_t *source, dn + + ret = dst_context_verify(msg->tsigctx, &sig_r); + if (ret == DST_R_VERIFYFAILURE) { +- msg->tsigstatus = dns_tsigerror_badsig; + tsig_log(msg->tsigkey, 2, + "signature failed to verify(2)"); + ret = DNS_R_TSIGVERIFYFAILURE; + goto cleanup_context; ++ } else if (ret != ISC_R_SUCCESS) { ++ goto cleanup_context; ++ } ++ ++ /* ++ * Here at this point, the MAC has been verified. Even ++ * if any of the following code returns a TSIG error, ++ * the reply will be signed and WILL always include the ++ * request MAC in the digest computation. ++ */ ++ ++ /* ++ * Is the time ok? ++ */ ++ isc_stdtime_get(&now); ++ ++ if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { ++ msg->tsigstatus = dns_tsigerror_badtime; ++ tsig_log(msg->tsigkey, 2, "signature has expired"); ++ ret = DNS_R_CLOCKSKEW; ++ goto cleanup_context; ++ } else if (now + msg->timeadjust < ++ tsig.timesigned - tsig.fudge) ++ { ++ msg->tsigstatus = dns_tsigerror_badtime; ++ tsig_log(msg->tsigkey, 2, ++ "signature is in the future"); ++ ret = DNS_R_CLOCKSKEW; ++ goto cleanup_context; + } +- else if (ret != ISC_R_SUCCESS) ++ ++ alg = dst_key_alg(key); ++ ret = dst_key_sigsize(key, &siglen); ++ if (ret != ISC_R_SUCCESS) + goto cleanup_context; ++ if ( ++#ifndef PK11_MD5_DISABLE ++ alg == DST_ALG_HMACMD5 || ++#endif ++ alg == DST_ALG_HMACSHA1 || ++ alg == DST_ALG_HMACSHA224 || ++ alg == DST_ALG_HMACSHA256 || ++ alg == DST_ALG_HMACSHA384 || ++ alg == DST_ALG_HMACSHA512) ++ { ++ isc_uint16_t digestbits = dst_key_getbits(key); + +- dst_context_destroy(&msg->tsigctx); ++ /* ++ * XXXRAY: Is this correct? What is the ++ * expected behavior when digestbits is not an ++ * integral multiple of 8? It looks like bytes ++ * should either be (digestbits/8) or ++ * (digestbits+7)/8. ++ * ++ * In any case, for current algorithms, ++ * digestbits are an integral multiple of 8, so ++ * it has the same effect as (digestbits/8). ++ */ ++ if (tsig.siglen > 0 && digestbits != 0 && ++ tsig.siglen < ((digestbits + 1) / 8)) ++ { ++ msg->tsigstatus = dns_tsigerror_badtrunc; ++ tsig_log(msg->tsigkey, 2, ++ "truncated signature length " ++ "too small"); ++ ret = DNS_R_TSIGVERIFYFAILURE; ++ goto cleanup_context; ++ } ++ if (tsig.siglen > 0 && digestbits == 0 && ++ tsig.siglen < siglen) ++ { ++ msg->tsigstatus = dns_tsigerror_badtrunc; ++ tsig_log(msg->tsigkey, 2, ++ "signature length too small"); ++ ret = DNS_R_TSIGVERIFYFAILURE; ++ goto cleanup_context; ++ } ++ } ++ ++ if (tsig.error != dns_rcode_noerror) { ++ msg->tsigstatus = tsig.error; ++ if (tsig.error == dns_tsigerror_badtime) ++ ret = DNS_R_CLOCKSKEW; ++ else ++ ret = DNS_R_TSIGERRORSET; ++ goto cleanup_context; ++ } + } + + msg->tsigstatus = dns_rcode_noerror; +- return (ISC_R_SUCCESS); ++ msg->verified_sig = 1; ++ ret = ISC_R_SUCCESS; + + cleanup_context: +- dst_context_destroy(&msg->tsigctx); ++ if (msg->tsigctx != NULL) ++ dst_context_destroy(&msg->tsigctx); + + cleanup_querystruct: + dns_rdata_freestruct(&querytsig); + + return (ret); +- + } + + isc_result_t diff --git a/bind.changes b/bind.changes index cf6e40e..e63ed24 100644 --- a/bind.changes +++ b/bind.changes @@ -1,3 +1,15 @@ +------------------------------------------------------------------- +Fri Jun 30 07:12:50 UTC 2017 - sflees@suse.de + +- Added bind-CVE-2017-3142-and-3143.patch to fix a security issue + where an attacker with the ability to send and receive messages + to an authoritative DNS server was able to circumvent TSIG + authentication of AXFR requests. A server that relies solely on + TSIG keys for protection with no other ACL protection could be + manipulated into (1) providing an AXFR of a zone to an + unauthorized recipient and (2) accepting bogus Notify packets. + [bsc#1046554, CVE-2017-3142, bsc#1046555, CVE-2017-3143] + ------------------------------------------------------------------- Sat May 20 11:46:44 UTC 2017 - dimstar@opensuse.org diff --git a/bind.spec b/bind.spec index 5586d48..3a2f7f6 100644 --- a/bind.spec +++ b/bind.spec @@ -47,6 +47,7 @@ Patch53: bind-sdb-ldap.patch Patch101: runidn.diff Patch102: idnkit-powerpc-ltconfig.patch Patch103: bind-CVE-2017-3135.patch +Patch104: bind-CVE-2017-3142-and-3143.patch BuildRequires: krb5-devel BuildRequires: libcap-devel BuildRequires: libmysqlclient-devel @@ -384,6 +385,7 @@ Name Domain (BIND) DNS server is found in the package named bind. %patch101 -p1 %patch102 -p1 %patch103 -p1 +%patch104 -p1 # use the year from source gzip header instead of current one to make reproducible rpms year=$(perl -e 'sysread(STDIN, $h, 8); print (1900+(gmtime(unpack("l",substr($h,4))))[5])' < %{S:0}) From 4215a9d83ee6e525e0b528d60b5d883ef1a8c4e2395489c57bf8745d9fea0420 Mon Sep 17 00:00:00 2001 From: OBS User mrdocs Date: Mon, 3 Jul 2017 22:11:50 +0000 Subject: [PATCH 2/2] Accepting request 507735 from home:dimstar:Factory - Run systemctl daemon-reload even when this is not build with systemd support: if installing bind on a systemd service and not reloading systemd daemon, then the service 'named' is not known right after package installation, causing confusion. OBS-URL: https://build.opensuse.org/request/show/507735 OBS-URL: https://build.opensuse.org/package/show/network/bind?expand=0&rev=212 --- bind.changes | 8 ++++++++ bind.spec | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/bind.changes b/bind.changes index e63ed24..03f48ce 100644 --- a/bind.changes +++ b/bind.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Jul 3 08:47:39 UTC 2017 - dimstar@opensuse.org + +- Run systemctl daemon-reload even when this is not build with + systemd support: if installing bind on a systemd service and not + reloading systemd daemon, then the service 'named' is not known + right after package installation, causing confusion. + ------------------------------------------------------------------- Fri Jun 30 07:12:50 UTC 2017 - sflees@suse.de diff --git a/bind.spec b/bind.spec index 3a2f7f6..744bdd3 100644 --- a/bind.spec +++ b/bind.spec @@ -734,6 +734,13 @@ if [ -f ${NAMED_ACTIVE_FILE} ]; then sbin/insserv named test ! -s ${NAMED_ACTIVE_FILE} && rm -f ${NAMED_ACTIVE_FILE} fi +if [ -x %{_bindir}/systemctl ]; then +# make sure systemctl knows about the service even though it's not a systemd service +# Without this, systemctl status named would return +# Unit named.service could not be found. +# until systemctl daemon-reload has been executed + %{_bindir}/systemctl daemon-reload || : +fi %endif %postun