diff --git a/bin/tests/system/chain/ans3/ans.pl b/bin/tests/system/chain/ans3/ans.pl deleted file mode 100644 index e42240be63..0000000000 --- a/bin/tests/system/chain/ans3/ans.pl +++ /dev/null @@ -1,143 +0,0 @@ -#!/usr/bin/env perl - -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -use strict; -use warnings; - -use IO::File; -use Getopt::Long; -use Net::DNS::Nameserver; - -my $pidf = new IO::File "ans.pid", "w" or die "cannot open pid file: $!"; -print $pidf "$$\n" or die "cannot write pid file: $!"; -$pidf->close or die "cannot close pid file: $!"; -sub rmpid { unlink "ans.pid"; exit 1; }; -sub term { }; - -$SIG{INT} = \&rmpid; -if ($Net::DNS::VERSION > 1.41) { - $SIG{TERM} = \&term; -} else { - $SIG{TERM} = \&rmpid; -} - -my $localaddr = "10.53.0.3"; - -my $localport = int($ENV{'PORT'}); -if (!$localport) { $localport = 5300; } - -my $verbose = 0; -my $ttl = 60; -my $zone = "example.broken"; -my $nsname = "ns3.$zone"; -my $synth = "synth-then-dname.$zone"; -my $synth2 = "synth2-then-dname.$zone"; - -sub reply_handler { - my ($qname, $qclass, $qtype, $peerhost, $query, $conn) = @_; - my ($rcode, @ans, @auth, @add); - - print ("request: $qname/$qtype\n"); - STDOUT->flush(); - - if ($qname eq "example.broken") { - if ($qtype eq "SOA") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass SOA . . 0 0 0 0 0"); - push @ans, $rr; - } elsif ($qtype eq "NS") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass NS $nsname"); - push @ans, $rr; - $rr = new Net::DNS::RR("$nsname $ttl $qclass A $localaddr"); - push @add, $rr; - } - $rcode = "NOERROR"; - } elsif ($qname eq "cname-to-$synth2") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.$synth2"); - push @ans, $rr; - $rr = new Net::DNS::RR("name.$synth2 $ttl $qclass CNAME name"); - push @ans, $rr; - $rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME ."); - push @ans, $rr; - $rcode = "NOERROR"; - } elsif ($qname eq "$synth" || $qname eq "$synth2") { - if ($qtype eq "DNAME") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass DNAME ."); - push @ans, $rr; - } - $rcode = "NOERROR"; - } elsif ($qname eq "name.$synth") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name."); - push @ans, $rr; - $rr = new Net::DNS::RR("$synth $ttl $qclass DNAME ."); - push @ans, $rr; - $rcode = "NOERROR"; - } elsif ($qname eq "name.$synth2") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name."); - push @ans, $rr; - $rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME ."); - push @ans, $rr; - $rcode = "NOERROR"; - # The following three code branches referring to the "example.dname" - # zone are necessary for the resolver variant of the CVE-2021-25215 - # regression test to work. A named instance cannot be used for - # serving the DNAME records below as a version of BIND vulnerable to - # CVE-2021-25215 would crash while answering the queries asked by - # the tested resolver. - } elsif ($qname eq "ns3.example.dname") { - if ($qtype eq "A") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass A 10.53.0.3"); - push @ans, $rr; - } - if ($qtype eq "AAAA") { - my $rr = new Net::DNS::RR("example.dname. $ttl $qclass SOA . . 0 0 0 0 $ttl"); - push @auth, $rr; - } - $rcode = "NOERROR"; - } elsif ($qname eq "self.example.self.example.dname") { - my $rr = new Net::DNS::RR("self.example.dname. $ttl $qclass DNAME dname."); - push @ans, $rr; - $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME self.example.dname."); - push @ans, $rr; - $rcode = "NOERROR"; - } elsif ($qname eq "self.example.dname") { - if ($qtype eq "DNAME") { - my $rr = new Net::DNS::RR("$qname $ttl $qclass DNAME dname."); - push @ans, $rr; - } - $rcode = "NOERROR"; - } else { - $rcode = "REFUSED"; - } - return ($rcode, \@ans, \@auth, \@add, { aa => 1 }); -} - -GetOptions( - 'port=i' => \$localport, - 'verbose!' => \$verbose, -); - -my $ns = Net::DNS::Nameserver->new( - LocalAddr => $localaddr, - LocalPort => $localport, - ReplyHandler => \&reply_handler, - Verbose => $verbose, -); - -if ($Net::DNS::VERSION >= 1.42) { - $ns->start_server(); - select(undef, undef, undef, undef); - $ns->stop_server(); - unlink "ans.pid"; -} else { - $ns->main_loop; -} diff --git a/bin/tests/system/chain/ans3/ans.py b/bin/tests/system/chain/ans3/ans.py new file mode 100644 index 0000000000..0a031c1145 --- /dev/null +++ b/bin/tests/system/chain/ans3/ans.py @@ -0,0 +1,217 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +############################################################################ +# ans.py: See README.anspy for details. +############################################################################ + +from __future__ import print_function +import os +import sys +import signal +import socket +import select +from datetime import datetime, timedelta +import functools + +import dns, dns.message, dns.query +from dns.rdatatype import * +from dns.rdataclass import * +from dns.rcode import * +from dns.name import * + + +############################################################################ +# Respond to a DNS query. +############################################################################ +def create_response(msg): + ttl = 60 + zone = "example.broken." + nsname = f"ns3.{zone}" + synth = f"synth-then-dname.{zone}" + synth2 = f"synth2-then-dname.{zone}" + + m = dns.message.from_wire(msg) + qname = m.question[0].name.to_text() + + # prepare the response and convert to wire format + r = dns.message.make_response(m) + + # get qtype + rrtype = m.question[0].rdtype + qtype = dns.rdatatype.to_text(rrtype) + print(f"request: {qname}/{qtype}") + + rcode = "NOERROR" + if qname == zone: + if qtype == "SOA": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, SOA, ". . 0 0 0 0 0")) + elif qtype == "NS": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, NS, nsname)) + r.additional.append(dns.rrset.from_text(nsname, ttl, IN, A, ip4)) + elif qname == f"cname-to-{synth2}": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, CNAME, f"name.{synth2}")) + r.answer.append(dns.rrset.from_text(f"name.{synth2}", ttl, IN, CNAME, "name.")) + r.answer.append(dns.rrset.from_text(synth2, ttl, IN, DNAME, ".")) + elif qname == f"{synth}" or qname == f"{synth2}": + if qtype == "DNAME": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, DNAME, ".")) + elif qname == f"name.{synth}": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, CNAME, "name.")) + r.answer.append(dns.rrset.from_text(synth, ttl, IN, DNAME, ".")) + elif qname == f"name.{synth2}": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, CNAME, "name.")) + r.answer.append(dns.rrset.from_text(synth2, ttl, IN, DNAME, ".")) + elif qname == "ns3.example.dname.": + # This and the next two code branches referring to the "example.dname" + # zone are necessary for the resolver variant of the CVE-2021-25215 + # regression test to work. A named instance cannot be used for + # serving the DNAME records below as a version of BIND vulnerable to + # CVE-2021-25215 would crash while answering the queries asked by + # the tested resolver. + if qtype == "A": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, A, ip4)) + elif qtype == "AAAA": + r.authority.append( + dns.rrset.from_text("example.dname.", ttl, IN, SOA, ". . 0 0 0 0 0") + ) + elif qname == "self.example.self..example.dname.": + r.answer.append( + dns.rrset.from_text("self.example.dname.", ttl, IN, DNAME, "dname.") + ) + r.answer.append( + dns.rrset.from_text(qname, ttl, IN, CNAME, "self.example.dname.") + ) + elif qname == "self.example.dname.": + if qtype == "DNAME": + r.answer.append(dns.rrset.from_text(qname, ttl, IN, DNAME, "dname.")) + else: + rcode = "REFUSED" + + r.flags |= dns.flags.AA + r.use_edns() + return r.to_wire() + + +def sigterm(signum, frame): + print("Shutting down now...") + os.remove("ans.pid") + running = False + sys.exit(0) + + +############################################################################ +# Main +# +# Set up responder and control channel, open the pid file, and start +# the main loop, listening for queries on the query channel or commands +# on the control channel and acting on them. +############################################################################ +ip4 = "10.53.0.3" +ip6 = "fd92:7065:b8e:ffff::3" + +try: + port = int(os.environ["PORT"]) +except: + port = 5300 + +query4_udp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) +query4_udp.bind((ip4, port)) + +query4_tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +query4_tcp.bind((ip4, port)) +query4_tcp.listen(1) +query4_tcp.settimeout(1) + +havev6 = True +try: + query6_udp = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) + try: + query6_udp.bind((ip6, port)) + except: + query6_udp.close() + havev6 = False + + query6_tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + query6_tcp.bind((ip4, port)) + query6_tcp.listen(1) + query6_tcp.settimeout(1) + except: + query6_tcp.close() + havev6 = False +except: + havev6 = False + +signal.signal(signal.SIGTERM, sigterm) + +f = open("ans.pid", "w") +pid = os.getpid() +print(pid, file=f) +f.close() + +running = True + +print("Listening on %s port %d" % (ip4, port)) +if havev6: + print("Listening on %s port %d" % (ip6, port)) +print("Ctrl-c to quit") + +if havev6: + input = [query4_udp, query4_tcp, query6_udp, query6_tcp] +else: + input = [query4_udp, query4_tcp] + +while running: + try: + inputready, outputready, exceptready = select.select(input, [], []) + except select.error as e: + break + except socket.error as e: + break + except KeyboardInterrupt: + break + + for s in inputready: + if s == query4_udp or s == query6_udp: + print("Query received on %s" % (ip4 if s == query4_udp else ip6)) + # Handle incoming queries + msg = s.recvfrom(65535) + rsp = create_response(msg[0]) + if rsp: + s.sendto(rsp, msg[1]) + elif s == query4_tcp or s == query6_tcp: + try: + conn, _ = s.accept() + if s == query4_tcp or s == query6_tcp: + print( + "TCP Query received on %s" % (ip4 if s == query4_tcp else ip6), + end=" ", + ) + # get TCP message length + msg = conn.recv(2) + if len(msg) != 2: + print("couldn't read TCP message length") + continue + length = struct.unpack(">H", msg[:2])[0] + msg = conn.recv(length) + if len(msg) != length: + print("couldn't read TCP message") + continue + rsp = create_response(msg) + if rsp: + conn.send(struct.pack(">H", len(rsp))) + conn.send(rsp) + conn.close() + except socket.error as e: + print("error: %s" % str(e)) + if not running: + break diff --git a/bin/tests/system/chain/ans4/ans.py b/bin/tests/system/chain/ans4/ans.py index 839067faa5..66f0193cac 100755 --- a/bin/tests/system/chain/ans4/ans.py +++ b/bin/tests/system/chain/ans4/ans.py @@ -316,16 +316,30 @@ try: except: ctrlport = 5300 -query4_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) -query4_socket.bind((ip4, port)) +query4_udp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) +query4_udp.bind((ip4, port)) + +query4_tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +query4_tcp.bind((ip4, port)) +query4_tcp.listen(1) +query4_tcp.settimeout(1) havev6 = True try: - query6_socket = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) + query6_udp = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) try: - query6_socket.bind((ip6, port)) + query6_udp.bind((ip6, port)) except: - query6_socket.close() + query6_udp.close() + havev6 = False + + query6_tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + query6_tcp.bind((ip4, port)) + query6_tcp.listen(1) + query6_tcp.settimeout(1) + except: + query6_tcp.close() havev6 = False except: havev6 = False @@ -350,9 +364,9 @@ print("Control channel on %s port %d" % (ip4, ctrlport)) print("Ctrl-c to quit") if havev6: - input = [query4_socket, query6_socket, ctrl_socket] + input = [query4_udp, query4_tcp, query6_udp, query6_tcp, ctrl_socket] else: - input = [query4_socket, ctrl_socket] + input = [query4_udp, query4_tcp, ctrl_socket] while running: try: @@ -375,12 +389,37 @@ while running: break ctl_channel(msg) conn.close() - if s == query4_socket or s == query6_socket: - print("Query received on %s" % (ip4 if s == query4_socket else ip6)) + elif s == query4_udp or s == query6_udp: + print("Query received on %s" % (ip4 if s == query4_udp else ip6)) # Handle incoming queries msg = s.recvfrom(65535) rsp = create_response(msg[0]) if rsp: s.sendto(rsp, msg[1]) + elif s == query4_tcp or s == query6_tcp: + try: + conn, _ = s.accept() + if s == query4_tcp or s == query6_tcp: + print( + "TCP Query received on %s" % (ip4 if s == query4_tcp else ip6), + end=" ", + ) + # get TCP message length + msg = conn.recv(2) + if len(msg) != 2: + print("couldn't read TCP message length") + continue + length = struct.unpack(">H", msg[:2])[0] + msg = conn.recv(length) + if len(msg) != length: + print("couldn't read TCP message") + continue + rsp = create_response(msg) + if rsp: + conn.send(struct.pack(">H", len(rsp))) + conn.send(rsp) + conn.close() + except socket.error as e: + print("error: %s" % str(e)) if not running: break diff --git a/doc/arm/changelog.rst b/doc/arm/changelog.rst index fca3263d94..32239a5889 100644 --- a/doc/arm/changelog.rst +++ b/doc/arm/changelog.rst @@ -18,6 +18,8 @@ Changelog development. Regular users should refer to :ref:`Release Notes ` for changes relevant to them. +.. include:: ../changelog/changelog-9.18.41.rst +.. include:: ../changelog/changelog-9.18.40.rst .. include:: ../changelog/changelog-9.18.39.rst .. include:: ../changelog/changelog-9.18.38.rst .. include:: ../changelog/changelog-9.18.37.rst diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index 7349a8b8f0..c63ca973b1 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -45,6 +45,8 @@ The list of known issues affecting the latest version in the 9.18 branch can be found at https://gitlab.isc.org/isc-projects/bind9/-/wikis/Known-Issues-in-BIND-9.18 +.. include:: ../notes/notes-9.18.41.rst +.. include:: ../notes/notes-9.18.40.rst .. include:: ../notes/notes-9.18.39.rst .. include:: ../notes/notes-9.18.38.rst .. include:: ../notes/notes-9.18.37.rst diff --git a/doc/changelog/changelog-9.18.40.rst b/doc/changelog/changelog-9.18.40.rst new file mode 100644 index 0000000000..4a8d883a9c --- /dev/null +++ b/doc/changelog/changelog-9.18.40.rst @@ -0,0 +1,18 @@ +.. Copyright (C) Internet Systems Consortium, Inc. ("ISC") +.. +.. SPDX-License-Identifier: MPL-2.0 +.. +.. This Source Code Form is subject to the terms of the Mozilla Public +.. License, v. 2.0. If a copy of the MPL was not distributed with this +.. file, you can obtain one at https://mozilla.org/MPL/2.0/. +.. +.. See the COPYRIGHT file distributed with this work for additional +.. information regarding copyright ownership. + +BIND 9.18.40 +------------ + +.. note:: + + The BIND 9.18.40 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. diff --git a/doc/changelog/changelog-9.18.41.rst b/doc/changelog/changelog-9.18.41.rst new file mode 100644 index 0000000000..3404e52958 --- /dev/null +++ b/doc/changelog/changelog-9.18.41.rst @@ -0,0 +1,148 @@ +.. Copyright (C) Internet Systems Consortium, Inc. ("ISC") +.. +.. SPDX-License-Identifier: MPL-2.0 +.. +.. This Source Code Form is subject to the terms of the Mozilla Public +.. License, v. 2.0. If a copy of the MPL was not distributed with this +.. file, you can obtain one at https://mozilla.org/MPL/2.0/. +.. +.. See the COPYRIGHT file distributed with this work for additional +.. information regarding copyright ownership. + +BIND 9.18.41 +------------ + +Security Fixes +~~~~~~~~~~~~~~ + +- [CVE-2025-8677] DNSSEC validation fails if matching but invalid DNSKEY + is found. ``85d08e06831`` + + Previously, if a matching but cryptographically invalid key was + encountered during DNSSEC validation, the key was skipped and not + counted towards validation failures. :iscman:`named` now treats such + DNSSEC keys as hard failures and the DNSSEC validation fails + immediately, instead of continuing with the next DNSKEYs in the RRset. + + ISC would like to thank Zuyao Xu and Xiang Li from the All-in-One + Security and Privacy Laboratory at Nankai University for bringing this + vulnerability to our attention. :gl:`#5343` + +- [CVE-2025-40778] Address various spoofing attacks. ``4c99ba5a462`` + + Previously, several issues could be exploited to poison a DNS cache + with spoofed records for zones which were not DNSSEC-signed or if the + resolver was configured to not do DNSSEC validation. These issues were + assigned CVE-2025-40778 and have now been fixed. + + As an additional layer of protection, :iscman:`named` no longer + accepts DNAME records or extraneous NS records in the AUTHORITY + section unless these are received via spoofing-resistant transport + (TCP, UDP with DNS cookies, TSIG, or SIG(0)). + + ISC would like to thank Yuxiao Wu, Yunyi Zhang, Baojun Liu, and Haixin + Duan from Tsinghua University for bringing this vulnerability to our + attention. :gl:`#5414` + +- [CVE-2025-40780] Cache-poisoning due to weak pseudo-random number + generator. ``f74fb05265b`` + + It was discovered during research for an upcoming academic paper that + a xoshiro128\*\* internal state can be recovered by an external 3rd + party, allowing the prediction of UDP ports and DNS IDs in outgoing + queries. This could lead to an attacker spoofing the DNS answers with + great efficiency and poisoning the DNS cache. + + The internal random generator has been changed to a cryptographically + secure pseudo-random generator. + + ISC would like to thank Prof. Amit Klein and Omer Ben Simhon from + Hebrew University of Jerusalem for bringing this vulnerability to our + attention. :gl:`#5484` + +New Features +~~~~~~~~~~~~ + +- Support for parsing HHIT and BRID records has been added. + ``d7d4e94d085`` + + :gl:`#5444` :gl:`!10933` + +Removed Features +~~~~~~~~~~~~~~~~ + +- Deprecate the "tkey-domain" statement. ``e28c95c1160`` + + Mark the :any:`tkey-domain` statement as deprecated since it is only + used by code implementing TKEY Mode 2 (Diffie-Hellman), which was + removed from newer BIND 9 branches. :gl:`#4204` :gl:`!10783` + +- Deprecate the "tkey-gssapi-credential" statement. ``2705307f818`` + + The :any:`tkey-gssapi-keytab` statement allows GSS-TSIG to be set up + in a simpler and more reliable way than using the + :any:`tkey-gssapi-credential` statement and setting environment + variables (e.g. ``KRB5_KTNAME``). Therefore, the + :any:`tkey-gssapi-credential` statement has been deprecated; + :any:`tkey-gssapi-keytab` should be used instead. + + For configurations currently using a combination of both + :any:`tkey-gssapi-keytab` *and* :any:`tkey-gssapi-credential`, the + latter should be dropped and the keytab pointed to by + :any:`tkey-gssapi-keytab` should now only contain the credential + previously specified by :any:`tkey-gssapi-credential`. :gl:`#4204` + :gl:`!10925` + +Feature Changes +~~~~~~~~~~~~~~~ + +- Update clang-format style with options added in newer versions. + ``1bc0f245c79`` + + Add and apply InsertBraces statement to add missing curly braces + around one-line statements and use + ControlStatementsExceptControlMacros for SpaceBeforeParens to remove + space between foreach macro and the brace, e.g. `FOREACH (x) {` + becomes `FOREACH(x) {`. :gl:`!10865` + +Bug Fixes +~~~~~~~~~ + +- Prevent spurious SERVFAILs for certain 0-TTL resource records. + ``f5a6a8be45f`` + + Under certain circumstances, BIND 9 can return SERVFAIL when updating + existing entries in the cache with new NS, A, AAAA, or DS records with + 0-TTL. :gl:`#5294` :gl:`!10899` + +- Use DNS_RDATACOMMON_INIT to hide branch differences. ``aef4682e4aa`` + + Initialization of the common members of rdata type structures varies + across branches. Standardize it by using the `DNS_RDATACOMMON_INIT` + macro for all types, so that new types are more likely to use it, and + hence backport more cleanly. :gl:`#5467` :gl:`!10833` + +- RPZ canonical warning displays zone entry incorrectly. ``3e787e98930`` + + When an IPv6 rpz prefix entry is entered incorrectly the log message + was just displaying the prefix rather than the full entry. This has + been corrected. :gl:`#5491` :gl:`!10931` + +- Missing DNSSEC information when CD bit is set in query. + ``990586f0496`` + + The RRSIGs for glue records were not being cached correctly for CD=1 + queries. This has been fixed. :gl:`#5502` :gl:`!10957` + +- Add and use __attribute__((nonnull)) in dnssec-signzone.c. + ``48c30cfcd08`` + + Clang 20 was spuriously warning about the possibility of passing a + NULL file pointer to `fprintf()`, which uses the 'nonnull' attribute. + To silence the warning, the functions calling `fprintf()` have been + marked with the same attribute to assure that NULL can't be passed to + them in the first place. + + Close #5487 :gl:`!10914` + + diff --git a/doc/notes/notes-9.18.40.rst b/doc/notes/notes-9.18.40.rst new file mode 100644 index 0000000000..4cb72e1d4e --- /dev/null +++ b/doc/notes/notes-9.18.40.rst @@ -0,0 +1,18 @@ +.. Copyright (C) Internet Systems Consortium, Inc. ("ISC") +.. +.. SPDX-License-Identifier: MPL-2.0 +.. +.. This Source Code Form is subject to the terms of the Mozilla Public +.. License, v. 2.0. If a copy of the MPL was not distributed with this +.. file, you can obtain one at https://mozilla.org/MPL/2.0/. +.. +.. See the COPYRIGHT file distributed with this work for additional +.. information regarding copyright ownership. + +Notes for BIND 9.18.40 +---------------------- + +.. note:: + + The BIND 9.18.40 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. diff --git a/doc/notes/notes-9.18.41.rst b/doc/notes/notes-9.18.41.rst new file mode 100644 index 0000000000..c55bd370ae --- /dev/null +++ b/doc/notes/notes-9.18.41.rst @@ -0,0 +1,106 @@ +.. Copyright (C) Internet Systems Consortium, Inc. ("ISC") +.. +.. SPDX-License-Identifier: MPL-2.0 +.. +.. This Source Code Form is subject to the terms of the Mozilla Public +.. License, v. 2.0. If a copy of the MPL was not distributed with this +.. file, you can obtain one at https://mozilla.org/MPL/2.0/. +.. +.. See the COPYRIGHT file distributed with this work for additional +.. information regarding copyright ownership. + +Notes for BIND 9.18.41 +---------------------- + +Security Fixes +~~~~~~~~~~~~~~ + +- DNSSEC validation fails if matching but invalid DNSKEY is found. + :cve:`2025-8677` + + Previously, if a matching but cryptographically invalid key was + encountered during DNSSEC validation, the key was skipped and not + counted towards validation failures. :iscman:`named` now treats such + DNSSEC keys as hard failures and the DNSSEC validation fails + immediately, instead of continuing with the next DNSKEYs in the RRset. + + ISC would like to thank Zuyao Xu and Xiang Li from the All-in-One + Security and Privacy Laboratory at Nankai University for bringing this + vulnerability to our attention. :gl:`#5343` + +- Address various spoofing attacks. :cve:`2025-40778` + + Previously, several issues could be exploited to poison a DNS cache + with spoofed records for zones which were not DNSSEC-signed or if the + resolver was configured to not do DNSSEC validation. These issues were + assigned CVE-2025-40778 and have now been fixed. + + As an additional layer of protection, :iscman:`named` no longer + accepts DNAME records or extraneous NS records in the AUTHORITY + section unless these are received via spoofing-resistant transport + (TCP, UDP with DNS cookies, TSIG, or SIG(0)). + + ISC would like to thank Yuxiao Wu, Yunyi Zhang, Baojun Liu, and Haixin + Duan from Tsinghua University for bringing this vulnerability to our + attention. :gl:`#5414` + +- Cache-poisoning due to weak pseudo-random number generator. + :cve:`2025-40780` + + It was discovered during research for an upcoming academic paper that + a xoshiro128\*\* internal state can be recovered by an external 3rd + party, allowing the prediction of UDP ports and DNS IDs in outgoing + queries. This could lead to an attacker spoofing the DNS answers with + great efficiency and poisoning the DNS cache. + + The internal random generator has been changed to a cryptographically + secure pseudo-random generator. + + ISC would like to thank Prof. Amit Klein and Omer Ben Simhon from + Hebrew University of Jerusalem for bringing this vulnerability to our + attention. :gl:`#5484` + +New Features +~~~~~~~~~~~~ + +- Support for parsing HHIT and BRID records has been added. + + :gl:`#5444` + +Removed Features +~~~~~~~~~~~~~~~~ + +- Deprecate the "tkey-domain" statement. + + Mark the :any:`tkey-domain` statement as deprecated since it is only + used by code implementing TKEY Mode 2 (Diffie-Hellman), which was + removed from newer BIND 9 branches. :gl:`#4204` + +- Deprecate the "tkey-gssapi-credential" statement. + + The :any:`tkey-gssapi-keytab` statement allows GSS-TSIG to be set up + in a simpler and more reliable way than using the + :any:`tkey-gssapi-credential` statement and setting environment + variables (e.g. ``KRB5_KTNAME``). Therefore, the + :any:`tkey-gssapi-credential` statement has been deprecated; + :any:`tkey-gssapi-keytab` should be used instead. + + For configurations currently using a combination of both + :any:`tkey-gssapi-keytab` *and* :any:`tkey-gssapi-credential`, the + latter should be dropped and the keytab pointed to by + :any:`tkey-gssapi-keytab` should now only contain the credential + previously specified by :any:`tkey-gssapi-credential`. :gl:`#4204` + +Bug Fixes +~~~~~~~~~ + +- Prevent spurious SERVFAILs for certain 0-TTL resource records. + + Under certain circumstances, BIND 9 can return SERVFAIL when updating + existing entries in the cache with new NS, A, AAAA, or DS records that have a + TTL of zero. :gl:`#5294` + +- Missing DNSSEC information when CD bit is set in query. + + The RRSIGs for glue records were not being cached correctly for CD=1 + queries. This has been fixed. :gl:`#5502` diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index b4c2c8ee9c..d90a0837df 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -284,6 +284,7 @@ struct dns_message { unsigned int tkey : 1; unsigned int rdclass_set : 1; unsigned int fuzzing : 1; + unsigned int has_dname : 1; unsigned int opt_reserved; unsigned int sig_reserved; @@ -1527,4 +1528,11 @@ dns_message_response_minttl(dns_message_t *msg, dns_ttl_t *pttl); * \li 'pttl != NULL'. */ +bool +dns_message_hasdname(dns_message_t *msg); +/*%< + * Return whether a DNAME was detected in the ANSWER section of a QUERY + * message when it was parsed. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/message.c b/lib/dns/message.c index 22d328c0f1..541a854db0 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -428,6 +428,7 @@ msginit(dns_message_t *m) { m->cc_bad = 0; m->tkey = 0; m->rdclass_set = 0; + m->has_dname = 0; m->querytsig = NULL; m->indent.string = "\t"; m->indent.count = 0; @@ -1654,6 +1655,11 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, */ msg->tsigname->attributes |= DNS_NAMEATTR_NOCOMPRESS; free_name = false; + } else if (rdtype == dns_rdatatype_dname && + sectionid == DNS_SECTION_ANSWER && + msg->opcode == dns_opcode_query) + { + msg->has_dname = 1; } rdataset = NULL; @@ -4798,3 +4804,9 @@ dns_message_response_minttl(dns_message_t *msg, dns_ttl_t *pttl) { return ISC_R_SUCCESS; } + +bool +dns_message_hasdname(dns_message_t *msg) { + REQUIRE(DNS_MESSAGE_VALID(msg)); + return msg->has_dname; +} diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index dc9c5b1c87..1bacec6470 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -808,6 +808,7 @@ typedef struct respctx { bool get_nameservers; /* get a new NS rrset at * zone cut? */ bool resend; /* resend this query? */ + bool secured; /* message was signed or had a valid cookie */ bool nextitem; /* invalid response; keep * listening for the correct one */ bool truncated; /* response was truncated */ @@ -7141,7 +7142,8 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset, bool external, * locally served zone. */ static inline bool -name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { +name_external(const dns_name_t *name, dns_rdatatype_t type, respctx_t *rctx) { + fetchctx_t *fctx = rctx->fctx; isc_result_t result; dns_forwarders_t *forwarders = NULL; dns_fixedname_t fixed, zfixed; @@ -7154,7 +7156,7 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { dns_namereln_t rel; apex = (ISDUALSTACK(fctx->addrinfo) || !ISFORWARDER(fctx->addrinfo)) - ? fctx->domain + ? rctx->ns_name != NULL ? rctx->ns_name : fctx->domain : fctx->fwdname; /* @@ -7263,7 +7265,7 @@ check_section(void *arg, const dns_name_t *addname, dns_rdatatype_t type, result = dns_message_findname(rctx->query->rmessage, section, addname, dns_rdatatype_any, 0, &name, NULL); if (result == ISC_R_SUCCESS) { - external = name_external(name, type, fctx); + external = name_external(name, type, rctx); if (type == dns_rdatatype_a) { for (rdataset = ISC_LIST_HEAD(name->list); rdataset != NULL; @@ -7893,6 +7895,47 @@ betterreferral(respctx_t *rctx) { return false; } +static bool +rctx_need_tcpretry(respctx_t *rctx) { + resquery_t *query = rctx->query; + if ((rctx->retryopts & DNS_FETCHOPT_TCP) != 0) { + /* TCP is already in the retry flags */ + return false; + } + + /* + * If the message was secured, no need to continue. + */ + if (rctx->secured) { + return false; + } + + /* + * Currently the only extra reason why we might need to + * retry a UDP response over TCP is a DNAME in the message. + */ + if (dns_message_hasdname(query->rmessage)) { + return true; + } + + return false; +} + +static isc_result_t +rctx_tcpretry(respctx_t *rctx) { + /* + * Do we need to retry a UDP response over TCP? + */ + if (rctx_need_tcpretry(rctx)) { + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + return ISC_R_SUCCESS; +} + /* * resquery_response(): * Handles responses received in response to iterative queries sent by @@ -8082,6 +8125,17 @@ resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) { return; } + /* + * Remember whether this message was signed or had a + * valid client cookie; if not, we may need to retry over + * TCP later. + */ + if (query->rmessage->cc_ok || query->rmessage->tsig != NULL || + query->rmessage->sig0 != NULL) + { + rctx.secured = true; + } + /* * The dispatcher should ensure we only get responses with QR * set. @@ -8093,10 +8147,7 @@ resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) { * TCP. This may be a misconfigured anycast server or an attempt * to send a spoofed response. Skip if we have a valid tsig. */ - if (dns_message_gettsig(query->rmessage, NULL) == NULL && - !query->rmessage->cc_ok && !query->rmessage->cc_bad && - (rctx.retryopts & DNS_FETCHOPT_TCP) == 0) - { + if (!rctx.secured && (rctx.retryopts & DNS_FETCHOPT_TCP) == 0) { unsigned char cookie[COOKIE_BUFFER_SIZE]; if (dns_adb_getcookie(fctx->adb, query->addrinfo, cookie, sizeof(cookie)) > CLIENT_COOKIE_SIZE) @@ -8108,8 +8159,7 @@ resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) { isc_log_write( dns_lctx, DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "missing expected cookie " - "from %s", + "missing expected cookie from %s", addrbuf); } rctx.retryopts |= DNS_FETCHOPT_TCP; @@ -8119,6 +8169,17 @@ resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) { } } + /* + * Check whether we need to retry over TCP for some other reason. + */ + result = rctx_tcpretry(&rctx); + if (result == ISC_R_COMPLETE) { + return; + } + + /* + * Check for EDNS issues. + */ rctx_edns(&rctx); /* @@ -8850,8 +8911,8 @@ rctx_answer_positive(respctx_t *rctx) { } /* - * Cache records in the authority section, if - * there are any suitable for caching. + * Cache records in the authority section, if there are + * any suitable for caching. */ rctx_authority_positive(rctx); @@ -8923,7 +8984,7 @@ rctx_answer_scan(respctx_t *rctx) { /* * Don't accept DNAME from parent namespace. */ - if (name_external(name, dns_rdatatype_dname, fctx)) { + if (name_external(name, dns_rdatatype_dname, rctx)) { continue; } @@ -9224,14 +9285,14 @@ rctx_answer_dname(respctx_t *rctx) { /* * rctx_authority_positive(): - * Examine the records in the authority section (if there are any) for a - * positive answer. We expect the names for all rdatasets in this - * section to be subdomains of the domain being queried; any that are - * not are skipped. We expect to find only *one* owner name; any names - * after the first one processed are ignored. We expect to find only - * rdatasets of type NS, RRSIG, or SIG; all others are ignored. Whatever - * remains can be cached at trust level authauthority or additional - * (depending on whether the AA bit was set on the answer). + * If a positive answer was received over TCP or secured with a cookie + * or TSIG, examine the authority section. We expect names for all + * rdatasets in this section to be subdomains of the domain being queried; + * any that are not are skipped. We expect to find only *one* owner name; + * any names after the first one processed are ignored. We expect to find + * only rdatasets of type NS; all others are ignored. Whatever remains can + * be cached at trust level authauthority or additional (depending on + * whether the AA bit was set on the answer). */ static void rctx_authority_positive(respctx_t *rctx) { @@ -9239,6 +9300,11 @@ rctx_authority_positive(respctx_t *rctx) { bool done = false; isc_result_t result; + /* If it's spoofable, don't cache it. */ + if (!rctx->secured && (rctx->query->options & DNS_FETCHOPT_TCP) == 0) { + return; + } + result = dns_message_firstname(rctx->query->rmessage, DNS_SECTION_AUTHORITY); while (!done && result == ISC_R_SUCCESS) { @@ -9247,7 +9313,9 @@ rctx_authority_positive(respctx_t *rctx) { dns_message_currentname(rctx->query->rmessage, DNS_SECTION_AUTHORITY, &name); - if (!name_external(name, dns_rdatatype_ns, fctx)) { + if (!name_external(name, dns_rdatatype_ns, rctx) && + dns_name_issubdomain(fctx->name, name)) + { dns_rdataset_t *rdataset = NULL; /* diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 55138d2590..12b2aed57c 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -435,6 +435,8 @@ fetch_callback_dnskey(isc_task_t *task, isc_event_t *event) { result = select_signing_key(val, rdataset); if (result == ISC_R_SUCCESS) { val->keyset = &val->frdataset; + } else { + val->failed = true; } } result = validate_answer(val, true); @@ -1174,6 +1176,8 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { goto done; } dst_key_free(&val->key); + } else { + break; } dns_rdata_reset(&rdata); result = dns_rdataset_next(rdataset); @@ -1291,13 +1295,15 @@ seek_dnskey(dns_validator_t *val) { "keyset with trust %s", dns_trust_totext(val->frdataset.trust)); result = select_signing_key(val, val->keyset); - if (result != ISC_R_SUCCESS) { + if (result == ISC_R_NOTFOUND) { /* - * Either the key we're looking for is not - * in the rrset, or something bad happened. - * Give up. + * The key we're looking for is not + * in the rrset */ result = DNS_R_CONTINUE; + } else if (result != ISC_R_SUCCESS) { + /* Something bad happened. Give up. */ + break; } } break; @@ -1358,17 +1364,17 @@ compute_keytag(dns_rdata_t *rdata) { /*% * Is the DNSKEY rrset in val->event->rdataset self-signed? */ -static bool +static isc_result_t selfsigned_dnskey(dns_validator_t *val) { dns_rdataset_t *rdataset = val->event->rdataset; dns_rdataset_t *sigrdataset = val->event->sigrdataset; dns_name_t *name = val->event->name; isc_result_t result; isc_mem_t *mctx = val->view->mctx; - bool answer = false; + bool match = false; if (rdataset->type != dns_rdatatype_dnskey) { - return false; + return DNS_R_NOKEYMATCH; } for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; @@ -1390,8 +1396,6 @@ selfsigned_dnskey(dns_validator_t *val) { result == ISC_R_SUCCESS; result = dns_rdataset_next(sigrdataset)) { - dst_key_t *dstkey = NULL; - dns_rdata_reset(&sigrdata); dns_rdataset_current(sigrdataset, &sigrdata); result = dns_rdata_tostruct(&sigrdata, &sig, NULL); @@ -1406,18 +1410,16 @@ selfsigned_dnskey(dns_validator_t *val) { /* * If the REVOKE bit is not set we have a - * theoretically self signed DNSKEY RRset. - * This will be verified later. + * theoretically self-signed DNSKEY RRset; + * this will be verified later. + * + * We don't return the answer yet, though, + * because we need to check the remaining keys + * and possbly remove them if they're revoked. */ if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) { - answer = true; - continue; - } - - result = dns_dnssec_keyfromrdata(name, &keyrdata, mctx, - &dstkey); - if (result != ISC_R_SUCCESS) { - continue; + match = true; + break; } /* @@ -1427,6 +1429,14 @@ selfsigned_dnskey(dns_validator_t *val) { if (DNS_TRUST_PENDING(rdataset->trust) && dns_view_istrusted(val->view, name, &key)) { + dst_key_t *dstkey = NULL; + + result = dns_dnssec_keyfromrdata( + name, &keyrdata, mctx, &dstkey); + if (result != ISC_R_SUCCESS) { + break; + } + result = dns_dnssec_verify( name, rdataset, dstkey, true, val->view->maxbits, mctx, &sigrdata, @@ -1439,6 +1449,8 @@ selfsigned_dnskey(dns_validator_t *val) { */ dns_view_untrust(val->view, name, &key); } + + dst_key_free(&dstkey); } else if (rdataset->trust >= dns_trust_secure) { /* * We trust this RRset so if the key is @@ -1446,12 +1458,14 @@ selfsigned_dnskey(dns_validator_t *val) { */ dns_view_untrust(val->view, name, &key); } - - dst_key_free(&dstkey); } } - return answer; + if (!match) { + return DNS_R_NOKEYMATCH; + } + + return ISC_R_SUCCESS; } /*% @@ -1688,10 +1702,7 @@ check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, val->event->name, keyrdata, val->view->mctx, &dstkey); if (result != ISC_R_SUCCESS) { - /* - * This really shouldn't happen, but... - */ - continue; + return result; } } result = verify(val, dstkey, &rdata, sig.keyid); @@ -3051,11 +3062,22 @@ validator_start(isc_task_t *task, isc_event_t *event) { INSIST(dns_rdataset_isassociated(val->event->rdataset)); INSIST(dns_rdataset_isassociated(val->event->sigrdataset)); - if (selfsigned_dnskey(val)) { + + result = selfsigned_dnskey(val); + switch (result) { + case ISC_R_SUCCESS: result = validate_dnskey(val); - } else { + break; + case DNS_R_NOKEYMATCH: result = validate_answer(val, false); + break; + default: + validator_log(val, ISC_LOG_INFO, + "invalid selfsigned DNSKEY: %s", + isc_result_totext(result)); + goto cleanup; } + if (result == DNS_R_NOVALIDSIG && (val->attributes & VALATTR_TRIEDVERIFY) == 0) { @@ -3124,6 +3146,7 @@ validator_start(isc_task_t *task, isc_event_t *event) { UNREACHABLE(); } +cleanup: if (result != DNS_R_WAIT) { want_destroy = exit_check(val); validator_done(val, result); diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h index 1e30d0c87d..fd55343778 100644 --- a/lib/isc/include/isc/random.h +++ b/lib/isc/include/isc/random.h @@ -20,7 +20,7 @@ #include /*! \file isc/random.h - * \brief Implements wrapper around a non-cryptographically secure + * \brief Implements wrapper around a cryptographically secure * pseudo-random number generator. * */ diff --git a/lib/isc/random.c b/lib/isc/random.c index fb0466953d..6f37f5dedf 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -31,176 +31,135 @@ */ #include -#include -#include -#include +#include -#include +#include #include -#include #include -#include #include #include "entropy_private.h" -/* - * The specific implementation for PRNG is included as a C file - * that has to provide a static variable named seed, and a function - * uint32_t next(void) that provides next random number. - * - * The implementation must be thread-safe. - */ +#define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t)) -/* - * Two contestants have been considered: the xoroshiro family of the - * functions by Villa&Blackman, and PCG by O'Neill. After - * consideration, the xoshiro128starstar function has been chosen as - * the uint32_t random number provider because it is very fast and has - * good enough properties for our usage pattern. - */ - -/* - * Written in 2018 by David Blackman and Sebastiano Vigna (vigna@acm.org) - * - * To the extent possible under law, the author has dedicated all - * copyright and related and neighboring rights to this software to the - * public domain worldwide. This software is distributed without any - * warranty. - * - * See . - */ - -/* - * This is xoshiro128** 1.0, our 32-bit all-purpose, rock-solid generator. - * It has excellent (sub-ns) speed, a state size (128 bits) that is large - * enough for mild parallelism, and it passes all tests we are aware of. - * - * For generating just single-precision (i.e., 32-bit) floating-point - * numbers, xoshiro128+ is even faster. - * - * The state must be seeded so that it is not everywhere zero. - */ -static thread_local uint32_t seed[4] = { 0 }; +thread_local static uint32_t isc__random_pool[ISC_RANDOM_BUFSIZE]; +thread_local static size_t isc__random_pos = ISC_RANDOM_BUFSIZE; static uint32_t -rotl(const uint32_t x, int k) { - return (x << k) | (x >> (32 - k)); -} - -static uint32_t -next(void) { - uint32_t result_starstar, t; - - result_starstar = rotl(seed[0] * 5, 7) * 9; - t = seed[1] << 9; - - seed[2] ^= seed[0]; - seed[3] ^= seed[1]; - seed[1] ^= seed[2]; - seed[0] ^= seed[3]; - - seed[2] ^= t; - - seed[3] = rotl(seed[3], 11); - - return result_starstar; -} - -static thread_local isc_once_t isc_random_once = ISC_ONCE_INIT; - -static void -isc_random_initialize(void) { - int useed[4] = { 0, 0, 0, 1 }; +random_u32(void) { #if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION /* - * Set a constant seed to help in problem reproduction should fuzzing - * find a crash or a hang. The seed array must be non-zero else - * xoshiro128starstar will generate an infinite series of zeroes. + * A fixed stream of numbers helps with problem reproduction when + * fuzzing. The first result needs to be non-zero as expected by + * random_test.c (it starts with ISC_RANDOM_BUFSIZE, see above). */ -#else /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ - isc_entropy_get(useed, sizeof(useed)); + return (uint32_t)(isc__random_pos++); #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ - memmove(seed, useed, sizeof(seed)); + + if (isc__random_pos == ISC_RANDOM_BUFSIZE) { + isc_entropy_get(isc__random_pool, sizeof(isc__random_pool)); + isc__random_pos = 0; + } + + return isc__random_pool[isc__random_pos++]; } uint8_t isc_random8(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return next() & 0xff; + return (uint8_t)random_u32(); } uint16_t isc_random16(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return next() & 0xffff; + return (uint16_t)random_u32(); } uint32_t isc_random32(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return next(); + return random_u32(); } void isc_random_buf(void *buf, size_t buflen) { - int i; - uint32_t r; + REQUIRE(buflen == 0 || buf != NULL); - REQUIRE(buf != NULL); - REQUIRE(buflen > 0); - - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - - for (i = 0; i + sizeof(r) <= buflen; i += sizeof(r)) { - r = next(); - memmove((uint8_t *)buf + i, &r, sizeof(r)); + if (buf == NULL || buflen == 0) { + return; } - r = next(); - memmove((uint8_t *)buf + i, &r, buflen % sizeof(r)); - return; + + isc_entropy_get(buf, buflen); } uint32_t -isc_random_uniform(uint32_t upper_bound) { - /* Copy of arc4random_uniform from OpenBSD */ - uint32_t r, min; - - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - - if (upper_bound < 2) { - return 0; - } - -#if (ULONG_MAX > 0xffffffffUL) - min = 0x100000000UL % upper_bound; -#else /* if (ULONG_MAX > 0xffffffffUL) */ - /* Calculate (2**32 % upper_bound) avoiding 64-bit math */ - if (upper_bound > 0x80000000) { - min = 1 + ~upper_bound; /* 2**32 - upper_bound */ - } else { - /* (2**32 - (x * 2)) % x == 2**32 % x when x <= 2**31 */ - min = ((0xffffffff - (upper_bound * 2)) + 1) % upper_bound; - } -#endif /* if (ULONG_MAX > 0xffffffffUL) */ - +isc_random_uniform(uint32_t limit) { /* - * This could theoretically loop forever but each retry has - * p > 0.5 (worst case, usually far better) of selecting a - * number inside the range we need, so it should rarely need - * to re-roll. + * Daniel Lemire's nearly-divisionless unbiased bounded random numbers. + * + * https://lemire.me/blog/?p=17551 + * + * The raw random number generator `next()` returns a 32-bit value. + * We do a 64-bit multiply `next() * limit` and treat the product as a + * 32.32 fixed-point value less than the limit. Our result will be the + * integer part (upper 32 bits), and we will use the fraction part + * (lower 32 bits) to determine whether or not we need to resample. */ - for (;;) { - r = next(); - if (r >= min) { - break; + uint64_t num = (uint64_t)random_u32() * (uint64_t)limit; + /* + * In the fast path, we avoid doing a division in most cases by + * comparing the fraction part of `num` with the limit, which is + * a slight over-estimate for the exact resample threshold. + */ + if ((uint32_t)(num) < limit) { + /* + * We are in the slow path where we re-do the approximate test + * more accurately. The exact threshold for the resample loop + * is the remainder after dividing the raw RNG limit `1 << 32` + * by the caller's limit. We use a trick to calculate it + * within 32 bits: + * + * (1 << 32) % limit + * == ((1 << 32) - limit) % limit + * == (uint32_t)(-limit) % limit + * + * This division is safe: we know that `limit` is strictly + * greater than zero because of the slow-path test above. + */ + uint32_t residue = (uint32_t)(-limit) % limit; + /* + * Unless we get one of `N = (1 << 32) - residue` valid + * values, we reject the sample. This `N` is a multiple of + * `limit`, so our results will be unbiased; and `N` is the + * largest multiple that fits in 32 bits, so rejections are as + * rare as possible. + * + * There are `limit` possible values for the integer part of + * our fixed-point number. Each one corresponds to `N/limit` + * or `N/limit + 1` possible fraction parts. For our result to + * be unbiased, every possible integer part must have the same + * number of possible valid fraction parts. So, when we get + * the superfluous value in the `N/limit + 1` cases, we need + * to reject and resample. + * + * Because of the multiplication, the possible values in the + * fraction part are equally spaced by `limit`, with varying + * gaps at each end of the fraction's 32-bit range. We will + * choose a range of size `N` (a multiple of `limit`) into + * which valid fraction values must fall, with the rest of the + * 32-bit range covered by the `residue`. Lemire's paper says + * that exactly `N/limit` possible values spaced apart by + * `limit` will fit into our size `N` valid range, regardless + * of the size of the end gaps, the phase alignment of the + * values, or the position of the range. + * + * So, when a fraction value falls in the `residue` outside + * our valid range, it is superfluous, and we resample. + */ + while ((uint32_t)(num) < residue) { + num = (uint64_t)random_u32() * (uint64_t)limit; } } - - return r % upper_bound; + /* + * Return the integer part (upper 32 bits). + */ + return (uint32_t)(num >> 32); } diff --git a/tests/isc/random_test.c b/tests/isc/random_test.c index ccba317023..52f219bb3a 100644 --- a/tests/isc/random_test.c +++ b/tests/isc/random_test.c @@ -321,7 +321,9 @@ random_test(pvalue_func_t *func, isc_random_func test_func) { } break; case ISC_RANDOM_BYTES: - isc_random_buf(values, sizeof(values)); + for (i = 0; i < ARRAY_SIZE(values); i++) { + values[i] = isc_random32(); + } break; case ISC_RANDOM_UNIFORM: uniform_values = (uint16_t *)values;