Bazaar

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

  1. CVE-2013-2099
  2. Merge into bzr.dev
Proposed by Andrew Starr-Bochicchio
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
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 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.
Revision history for this message
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

Revision history for this message
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
1=== modified file 'bzrlib/tests/test_https_urllib.py'
2--- bzrlib/tests/test_https_urllib.py 2012-01-31 16:36:53 +0000
3+++ bzrlib/tests/test_https_urllib.py 2013-05-20 16:40:34 +0000
4@@ -88,6 +88,22 @@
5 self.assertRaises(ValueError,
6 _urllib2_wrappers.match_hostname, {}, "example.com")
7
8+ def test_wildcards_in_cert(self):
9+ def ok(cert, hostname):
10+ _urllib2_wrappers.match_hostname(cert, hostname)
11+
12+ # Python Issue #17980: avoid denials of service by refusing more than
13+ # one wildcard per fragment.
14+ cert = {'subject': ((('commonName', 'a*b.com'),),)}
15+ ok(cert, 'axxb.com')
16+ cert = {'subject': ((('commonName', 'a*b.co*'),),)}
17+ ok(cert, 'axxb.com')
18+ cert = {'subject': ((('commonName', 'a*b*.com'),),)}
19+ try:
20+ _urllib2_wrappers.match_hostname(cert, 'axxbxxc.com')
21+ except ValueError as e:
22+ self.assertIn("too many wildcards", str(e))
23+
24 def test_no_valid_attributes(self):
25 self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname,
26 {"Problem": "Solved"}, "example.com")
27
28=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
29--- bzrlib/transport/http/_urllib2_wrappers.py 2012-06-10 22:48:08 +0000
30+++ bzrlib/transport/http/_urllib2_wrappers.py 2013-05-20 16:40:34 +0000
31@@ -400,9 +400,16 @@
32
33 # These two methods were imported from Python 3.2's ssl module
34
35-def _dnsname_to_pat(dn):
36+def _dnsname_to_pat(dn, max_wildcards=1):
37 pats = []
38 for frag in dn.split(r'.'):
39+ if frag.count('*') > max_wildcards:
40+ # Python Issue #17980: avoid denials of service by refusing more
41+ # than one wildcard per fragment. A survery of established
42+ # policy among SSL implementations showed it to be a
43+ # reasonable choice.
44+ raise ValueError(
45+ "too many wildcards in certificate DNS name: " + repr(dn))
46 if frag == '*':
47 # When '*' is a fragment by itself, it matches a non-empty dotless
48 # fragment.