diff options
| -rw-r--r-- | doc/security.rst | 13 | ||||
| -rw-r--r-- | src/lib/utils/parsing.cpp | 184 | ||||
| -rw-r--r-- | src/tests/data/hostnames.vec | 70 | ||||
| -rw-r--r-- | src/tests/test_utils.cpp | 2 |
4 files changed, 191 insertions, 78 deletions
diff --git a/doc/security.rst b/doc/security.rst index a36173bc2..238c318fc 100644 --- a/doc/security.rst +++ b/doc/security.rst @@ -15,6 +15,19 @@ mail please use:: This key can be found in the file ``doc/pgpkey.txt`` or online at https://keybase.io/jacklloyd and on most PGP keyservers. +2018 +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* 2018-03-29 (CVE-2018-9127): Invalid wildcard match + + RFC 6125 wildcard matching was incorrectly implemented, so that a wildcard + certificate such as "b*.domain.com" would match any hosts "*b*.domain.com" + instead of just server names beginning with 'b'. The host and certificate + would still have to be in the same domain name. Reported by Fabian Weißberg of + Rohde and Schwarz Cybersecurity. + + Bug introduced in 2.2.0, fixed in 2.5.0 + 2017 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/lib/utils/parsing.cpp b/src/lib/utils/parsing.cpp index 9517cc673..cfae0cb70 100644 --- a/src/lib/utils/parsing.cpp +++ b/src/lib/utils/parsing.cpp @@ -1,6 +1,6 @@ /* * Various string utils and parsing functions -* (C) 1999-2007,2013,2014,2015 Jack Lloyd +* (C) 1999-2007,2013,2014,2015,2018 Jack Lloyd * (C) 2015 Simon Warta (Kullo GmbH) * (C) 2017 René Korthaus, Rohde & Schwarz Cybersecurity * @@ -11,6 +11,8 @@ #include <botan/exceptn.h> #include <botan/charset.h> #include <botan/loadstor.h> +#include <algorithm> +#include <cctype> #include <limits> #include <set> @@ -338,110 +340,138 @@ std::string replace_char(const std::string& str, char from_char, char to_char) return out; } -bool host_wildcard_match(const std::string& issued, const std::string& host) +namespace { + +std::string tolower_string(const std::string& in) { - if(issued == host) + std::string s = in; + for(size_t i = 0; i != s.size(); ++i) { - return true; + if(std::isalpha(static_cast<unsigned char>(s[i]))) + s[i] = std::tolower(static_cast<unsigned char>(s[i])); } + return s; + } + +} + +bool host_wildcard_match(const std::string& issued_, const std::string& host_) + { + const std::string issued = tolower_string(issued_); + const std::string host = tolower_string(host_); + + if(host.empty() || issued.empty()) + return false; + + /* + If there are embedded nulls in your issued name + Well I feel bad for you son + */ + if(std::count(issued.begin(), issued.end(), char(0)) > 0) + return false; - size_t stars = 0; - for(char c : issued) + // If more than one wildcard, then issued name is invalid + const size_t stars = std::count(issued.begin(), issued.end(), '*'); + if(stars > 1) + return false; + + // '*' is not a valid character in DNS names so should not appear on the host side + if(std::count(host.begin(), host.end(), '*') != 0) + return false; + + // Similarly a DNS name can't end in . + if(host[host.size() - 1] == '.') + return false; + + // And a host can't have an empty name component, so reject that + if(host.find("..") != std::string::npos) + return false; + + // Exact match: accept + if(issued == host) { - if(c == '*') - stars += 1; + return true; } - if(stars > 1) + /* + Otherwise it might be a wildcard + + If the issued size is strictly longer than the hostname size it + couldn't possibly be a match, even if the issued value is a + wildcard. The only exception is when the wildcard ends up empty + (eg www.example.com matches www*.example.com) + */ + if(issued.size() > host.size() + 1) { return false; } - // first try to match the base, then the left-most label - // which can contain exactly one wildcard at any position - if(issued.size() > 2) + // If no * at all then not a wildcard, and so not a match + if(stars != 1) { - size_t host_i = host.find('.'); - if(host_i == std::string::npos || host_i == host.size() - 1) - { - return false; - } - - size_t issued_i = issued.find('.'); - if(issued_i == std::string::npos || issued_i == issued.size() - 1) - { - return false; - } + return false; + } - const std::string host_base = host.substr(host_i + 1); - const std::string issued_base = issued.substr(issued_i + 1); + /* + Now walk through the issued string, making sure every character + matches. When we come to the (singular) '*', jump forward in the + hostname by the cooresponding amount. We know exactly how much + space the wildcard takes because it must be exactly `len(host) - + len(issued) + 1 chars`. - // if anything but the left-most label doesn't equal, - // we are already out here - if(host_base != issued_base) - { - return false; - } + We also verify that the '*' comes in the leftmost component, and + doesn't skip over any '.' in the hostname. + */ + size_t dots_seen = 0; + size_t host_idx = 0; - // compare the left-most labels - std::string host_prefix = host.substr(0, host_i); + for(size_t i = 0; i != issued.size(); ++i) + { + dots_seen += (issued[i] == '.'); - if(host_prefix.empty()) + if(issued[i] == '*') { - return false; - } + // Fail: wildcard can only come in leftmost component + if(dots_seen > 0) + { + return false; + } - const std::string issued_prefix = issued.substr(0, issued_i); + /* + Since there is only one * we know the tail of the issued and + hostname must be an exact match. In this case advance host_idx + to match. + */ + const size_t advance = (host.size() - issued.size() + 1); - // if split_on would work on strings with less than 2 items, - // the if/else block would not be necessary - if(issued_prefix == "*") - { - return true; - } + if(host_idx + advance > host.size()) // shouldn't happen + return false; - std::vector<std::string> p; + // Can't be any intervening .s that we would have skipped + if(std::count(host.begin() + host_idx, + host.begin() + host_idx + advance, '.') != 0) + return false; - if(issued_prefix[0] == '*') - { - p = std::vector<std::string>{"", issued_prefix.substr(1, issued_prefix.size())}; - } - else if(issued_prefix[issued_prefix.size()-1] == '*') - { - p = std::vector<std::string>{issued_prefix.substr(0, issued_prefix.size() - 1), ""}; + host_idx += advance; } else { - p = split_on(issued_prefix, '*'); - } - - if(p.size() != 2) - { - return false; - } - - // match anything before and after the wildcard character - const std::string first = p[0]; - const std::string last = p[1]; + if(issued[i] != host[host_idx]) + { + return false; + } - if(host_prefix.substr(0, first.size()) == first) - { - host_prefix.erase(0, first.size()); - } - - // nothing to match anymore - if(last.empty()) - { - return true; + host_idx += 1; } + } - if(host_prefix.size() >= last.size() && - host_prefix.substr(host_prefix.size() - last.size(), last.size()) == last) - { - return true; - } + // Wildcard issued name must have at least 3 components + if(dots_seen < 2) + { + return false; } - return false; + return true; } + } diff --git a/src/tests/data/hostnames.vec b/src/tests/data/hostnames.vec index 91296d2d8..56433af7b 100644 --- a/src/tests/data/hostnames.vec +++ b/src/tests/data/hostnames.vec @@ -40,7 +40,68 @@ Hostname = test.www.example.com Issued = *www.example.com Hostname = www.example.com +Issued = fluf*z.example.net +Hostname = flufbaz.example.net + +Issued = example.com +Hostname = EXAMPLE.com + +Issued = example.com +Hostname = EXAMPLE.COM + +Issued = *.example.COM +Hostname = a.EXAMPLE.com + +Issued = b*z.example.net +Hostname = bz.example.net + [Invalid] +Issued = +Hostname = empty.com + +Issued = empty.com +Hostname = + +Issued = +Hostname = + +Issued = * +Hostname = * + +Issued = * +Hostname = a + +Issued = *.com +Hostname = a.com + +Issued = *.com +Hostname = x.a.com + +# We reject hostnames with .. +Issued = a*..com +Hostname = aninvalidhostname..com + +Issued = f.*.com +Hostname = f.a.com + +Issued = ample.com +Hostname = example.com + +Issued = example.com +Hostname = ample.com + +Issued = b*z.example.net +Hostname = foobaz.example.net + +Issued = b*z.example.net +Hostname = z.example.net + +Issued = flufb*z.example.net +Hostname = foobaz.example.net + +Issued = b*z.example.net +Hostname = baz.example.com + Issued = example.com Hostname = www.example.com @@ -50,6 +111,9 @@ Hostname = example.com Issued = bar.*.example.net Hostname = bar.foo.example.net +Issued = bar.*.example.net +Hostname = bar..example.net + Issued = *.example.com Hostname = bar.foo.example.com @@ -59,6 +123,12 @@ Hostname = example.com Issued = foo*foo.example.com Hostname = foo.example.com +Issued = foo.exa*ple.com +Hostname = foo.example.com + +Issued = exa*ple.com +Hostname = example.com + Issued = **.example.com Hostname = foo.example.com diff --git a/src/tests/test_utils.cpp b/src/tests/test_utils.cpp index 5f3502eba..ee5f06187 100644 --- a/src/tests/test_utils.cpp +++ b/src/tests/test_utils.cpp @@ -531,7 +531,7 @@ class Hostname_Tests final : public Text_Based_Test Test::Result run_one_test(const std::string& type, const VarMap& vars) override { - Test::Result result("Hostname"); + Test::Result result("Hostname Matching"); const std::string issued = get_req_str(vars, "Issued"); const std::string hostname = get_req_str(vars, "Hostname"); |