Merge lp:~andrewsomething/bzr/CVE-2013-2099 into lp:bzr

[Andrew Starr-Bochicchio]

Fix CVE 2013-2009. Avoid allowing multiple wildcards in a single SSL cert hostname segment.

[Jelmer Vernooij (jelmer)]

On Mon, May 20, 2013 at 04:41:27PM -0000, Andrew Starr-Bochicchio wrote:
> === modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
> --- bzrlib/transport/http/_urllib2_wrappers.py 2012-06-10 22:48:08 +0000
> +++ bzrlib/transport/http/_urllib2_wrappers.py 2013-05-20 16:40:34 +0000
> @@ -400,9 +400,16 @@
>
> # These two methods were imported from Python 3.2's ssl module
>
> -def _dnsname_to_pat(dn):
> +def _dnsname_to_pat(dn, max_wildcards=1):
> pats = []
> for frag in dn.split(r'.'):
> + if frag.count('*') > max_wildcards:
> + # Python Issue #17980: avoid denials of service by refusing more
> + # than one wildcard per fragment. A survery of established
> + # policy among SSL implementations showed it to be a
> + # reasonable choice.
> + raise ValueError(
> + "too many wildcards in certificate DNS name: " + repr(dn))
> if frag == '*':
> # When '*' is a fragment by itself, it matches a non-empty dotless
> # fragment.
s/survery/survey/ ?

Looks good otherwise.

I would "review approve" but don't have my GPG credentials on me.

Jelmer

[John A Meinel (jameinel)]

sent to pqm by email

=== modified file 'bzrlib/tests/test_https_urllib.py'
--- bzrlib/tests/test_https_urllib.py	2012-01-31 16:36:53 +0000
+++ bzrlib/tests/test_https_urllib.py	2013-05-20 16:40:34 +0000
@@ -88,6 +88,22 @@
         self.assertRaises(ValueError,
                           _urllib2_wrappers.match_hostname, {}, "example.com")
 
+    def test_wildcards_in_cert(self):
+        def ok(cert, hostname):
+            _urllib2_wrappers.match_hostname(cert, hostname)
+
+        # Python Issue #17980: avoid denials of service by refusing more than
+        # one wildcard per fragment.
+        cert = {'subject': ((('commonName', 'a*b.com'),),)}
+        ok(cert, 'axxb.com')
+        cert = {'subject': ((('commonName', 'a*b.co*'),),)}
+        ok(cert, 'axxb.com')
+        cert = {'subject': ((('commonName', 'a*b*.com'),),)}
+        try:
+            _urllib2_wrappers.match_hostname(cert, 'axxbxxc.com')
+        except ValueError as e:
+            self.assertIn("too many wildcards", str(e))
+
     def test_no_valid_attributes(self):
         self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname,
                           {"Problem": "Solved"}, "example.com")
 
=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- bzrlib/transport/http/_urllib2_wrappers.py	2012-06-10 22:48:08 +0000
+++ bzrlib/transport/http/_urllib2_wrappers.py	2013-05-20 16:40:34 +0000
@@ -400,9 +400,16 @@
 
 # These two methods were imported from Python 3.2's ssl module
 
-def _dnsname_to_pat(dn):
+def _dnsname_to_pat(dn, max_wildcards=1):
     pats = []
     for frag in dn.split(r'.'):
+        if frag.count('*') > max_wildcards:
+            # Python Issue #17980: avoid denials of service by refusing more
+            # than one wildcard per fragment.  A survery of established
+            # policy among SSL implementations showed it to be a
+            # reasonable choice.
+            raise ValueError(
+                "too many wildcards in certificate DNS name: " + repr(dn))
         if frag == '*':
             # When '*' is a fragment by itself, it matches a non-empty dotless
             # fragment.