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

Proposed by Andrew Starr-Bochicchio on 2013-05-20
Status: Merged
Approved by: John A Meinel on 2013-05-20
Approved revision: 6574
Merged at revision: 6574
Proposed branch: lp:~andrewsomething/bzr/CVE-2013-2099
Merge into: lp:bzr
Diff against target: 48 lines (+24/-1)
2 files modified
bzrlib/tests/test_https_urllib.py (+16/-0)
bzrlib/transport/http/_urllib2_wrappers.py (+8/-1)
To merge this branch: bzr merge lp:~andrewsomething/bzr/CVE-2013-2099
Reviewer Review Type Date Requested Status
bzr-core 2013-05-20 Pending
Review via email: mp+164767@code.launchpad.net

Commit message

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

To post a comment you must log in.
Jelmer Vernooij (jelmer) wrote :

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) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
88 self.assertRaises(ValueError,88 self.assertRaises(ValueError,
89 _urllib2_wrappers.match_hostname, {}, "example.com")89 _urllib2_wrappers.match_hostname, {}, "example.com")
9090
91 def test_wildcards_in_cert(self):
92 def ok(cert, hostname):
93 _urllib2_wrappers.match_hostname(cert, hostname)
94
95 # Python Issue #17980: avoid denials of service by refusing more than
96 # one wildcard per fragment.
97 cert = {'subject': ((('commonName', 'a*b.com'),),)}
98 ok(cert, 'axxb.com')
99 cert = {'subject': ((('commonName', 'a*b.co*'),),)}
100 ok(cert, 'axxb.com')
101 cert = {'subject': ((('commonName', 'a*b*.com'),),)}
102 try:
103 _urllib2_wrappers.match_hostname(cert, 'axxbxxc.com')
104 except ValueError as e:
105 self.assertIn("too many wildcards", str(e))
106
91 def test_no_valid_attributes(self):107 def test_no_valid_attributes(self):
92 self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname,108 self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname,
93 {"Problem": "Solved"}, "example.com")109 {"Problem": "Solved"}, "example.com")
94110
=== 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 @@
400400
401# These two methods were imported from Python 3.2's ssl module401# These two methods were imported from Python 3.2's ssl module
402402
403def _dnsname_to_pat(dn):403def _dnsname_to_pat(dn, max_wildcards=1):
404 pats = []404 pats = []
405 for frag in dn.split(r'.'):405 for frag in dn.split(r'.'):
406 if frag.count('*') > max_wildcards:
407 # Python Issue #17980: avoid denials of service by refusing more
408 # than one wildcard per fragment. A survery of established
409 # policy among SSL implementations showed it to be a
410 # reasonable choice.
411 raise ValueError(
412 "too many wildcards in certificate DNS name: " + repr(dn))
406 if frag == '*':413 if frag == '*':
407 # When '*' is a fragment by itself, it matches a non-empty dotless414 # When '*' is a fragment by itself, it matches a non-empty dotless
408 # fragment.415 # fragment.