Merge lp:~vila/bzr/1538480-match-hostname into lp:bzr
- 1538480-match-hostname
- Merge into bzr.dev
Status: | Merged |
---|---|
Approved by: | Richard Wilbur |
Approved revision: | no longer in the source branch. |
Merged at revision: | 6614 |
Proposed branch: | lp:~vila/bzr/1538480-match-hostname |
Merge into: | lp:bzr |
Diff against target: |
364 lines (+75/-127) 4 files modified
bzrlib/errors.py (+1/-9) bzrlib/tests/test_https_urllib.py (+32/-28) bzrlib/transport/http/_urllib2_wrappers.py (+39/-90) doc/en/release-notes/bzr-2.7.txt (+3/-0) |
To merge this branch: | bzr merge lp:~vila/bzr/1538480-match-hostname |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Richard Wilbur | Approve | ||
Review via email: mp+284171@code.launchpad.net |
Commit message
Use ssl.match_hostname instead of our own.
Description of the change
Since match_hostname is now provided, use it instead of carrying an obsolete version.
Richard Wilbur (richard-wilbur) wrote : | # |
Vincent Ladeuil (vila) wrote : | # |
> Vincent, what you've done here is excellent and definitely an improvement over
> the contributed patch, exempli gratia, adding the not_ok function.
>
> What I'm curious about is this patch requires python >= 2.7.9. Do we plan to
> bump our python version requirement from 2.6 to 2.7.9?
Ha, the difficult question :)
So I went to https:/
And I'm seeing various projects dropping support for 2.6 too including testtools that is a requirement for bzr tests.
All in all, I think this means that bzr itself cannot support security fixes for python 2.6.
So I dropped the obsolete copy of python 3.2 (not supported anymore either).
In turns this means that people using a bzr 2.6 AND wanting https security support needs to upgrade to python 2.7.
In other words, we attempted to support cert checking with py2.6 at best as we could, we cannot anymore. This specific feature is controlled via the 'ssl.cert_reqs' config option which can be disabled if needed.
So with this patch, py27 users get better security, py26 users will need to verify their certs by other means and use the option to disable bzr checks but won't be lured into a false sense of security.
Apart from that scenario, py26 is still supported (but not regularly tested to the best of my knowledge).
>
> The other part of the patch checks for match_hostname in the ssl library and,
> if not found, imports it from backports.
I've never heard about a python 'backports' library, may be that's specific to the distribution the patch author is using ?
> 1. Is that a worthwhile exercise in light of python version requirements?
I don't think so, if we need to invest into a porting effort, I'd rather see work around forward porting to py3 than back porting to py2.6 ;)
On the other hand, we may want to officially stop supporting py26 in the future.
> 2. Is backports normally available or would we change our dependencies to get
> it?
Not that I know of.
Richard Wilbur (richard-wilbur) wrote : | # |
Could we then as part of this fix automatically disable the 'ssl.cert_reqs' config option if running under python 2.6 with a warning that the functionality is only available with python 2.x >= 2.7.9 or 3.x >= 3.3?
Vincent Ladeuil (vila) wrote : | # |
> Could we then as part of this fix automatically disable the 'ssl.cert_reqs'
> config option if running under python 2.6 with a warning that the
> functionality is only available with python 2.x >= 2.7.9 or 3.x >= 3.3?
/me facepalms
Of course !
Done.
Richard Wilbur (richard-wilbur) wrote : | # |
Thanks for that last revision. I think a descriptive warning is significantly better than an exception!
+2
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Preview Diff
1 | === modified file 'bzrlib/errors.py' | |||
2 | --- bzrlib/errors.py 2013-10-04 09:56:23 +0000 | |||
3 | +++ bzrlib/errors.py 2016-01-31 13:08:48 +0000 | |||
4 | @@ -1,4 +1,4 @@ | |||
6 | 1 | # Copyright (C) 2005-2011 Canonical Ltd | 1 | # Copyright (C) 2005-2013, 2016 Canonical Ltd |
7 | 2 | # | 2 | # |
8 | 3 | # This program is free software; you can redistribute it and/or modify | 3 | # This program is free software; you can redistribute it and/or modify |
9 | 4 | # it under the terms of the GNU General Public License as published by | 4 | # it under the terms of the GNU General Public License as published by |
10 | @@ -1686,14 +1686,6 @@ | |||
11 | 1686 | TransportError.__init__(self, msg, orig_error=orig_error) | 1686 | TransportError.__init__(self, msg, orig_error=orig_error) |
12 | 1687 | 1687 | ||
13 | 1688 | 1688 | ||
14 | 1689 | class CertificateError(TransportError): | ||
15 | 1690 | |||
16 | 1691 | _fmt = "Certificate error: %(error)s" | ||
17 | 1692 | |||
18 | 1693 | def __init__(self, error): | ||
19 | 1694 | self.error = error | ||
20 | 1695 | |||
21 | 1696 | |||
22 | 1697 | class InvalidHttpRange(InvalidHttpResponse): | 1689 | class InvalidHttpRange(InvalidHttpResponse): |
23 | 1698 | 1690 | ||
24 | 1699 | _fmt = "Invalid http range %(range)r for %(path)s: %(msg)s" | 1691 | _fmt = "Invalid http range %(range)r for %(path)s: %(msg)s" |
25 | 1700 | 1692 | ||
26 | === modified file 'bzrlib/tests/test_https_urllib.py' | |||
27 | --- bzrlib/tests/test_https_urllib.py 2013-05-20 16:38:11 +0000 | |||
28 | +++ bzrlib/tests/test_https_urllib.py 2016-01-31 13:08:48 +0000 | |||
29 | @@ -1,4 +1,4 @@ | |||
31 | 1 | # Copyright (C) 2011,2012 Canonical Ltd | 1 | # Copyright (C) 2011, 2012, 2013, 2016 Canonical Ltd |
32 | 2 | # | 2 | # |
33 | 3 | # This program is free software; you can redistribute it and/or modify | 3 | # This program is free software; you can redistribute it and/or modify |
34 | 4 | # it under the terms of the GNU General Public License as published by | 4 | # it under the terms of the GNU General Public License as published by |
35 | @@ -19,24 +19,21 @@ | |||
36 | 19 | """ | 19 | """ |
37 | 20 | 20 | ||
38 | 21 | import os | 21 | import os |
40 | 22 | import ssl | 22 | import sys |
41 | 23 | 23 | ||
42 | 24 | from bzrlib import ( | 24 | from bzrlib import ( |
43 | 25 | config, | 25 | config, |
44 | 26 | trace, | 26 | trace, |
46 | 27 | ) | 27 | ) |
47 | 28 | from bzrlib.errors import ( | 28 | from bzrlib.errors import ( |
48 | 29 | CertificateError, | ||
49 | 30 | ConfigOptionValueError, | 29 | ConfigOptionValueError, |
55 | 31 | ) | 30 | ) |
56 | 32 | from bzrlib.tests import ( | 31 | from bzrlib import tests |
52 | 33 | TestCase, | ||
53 | 34 | TestCaseInTempDir, | ||
54 | 35 | ) | ||
57 | 36 | from bzrlib.transport.http import _urllib2_wrappers | 32 | from bzrlib.transport.http import _urllib2_wrappers |
61 | 37 | 33 | from bzrlib.transport.http._urllib2_wrappers import ssl | |
62 | 38 | 34 | ||
63 | 39 | class CaCertsConfigTests(TestCaseInTempDir): | 35 | |
64 | 36 | class CaCertsConfigTests(tests.TestCaseInTempDir): | ||
65 | 40 | 37 | ||
66 | 41 | def get_stack(self, content): | 38 | def get_stack(self, content): |
67 | 42 | return config.MemoryStack(content.encode('utf-8')) | 39 | return config.MemoryStack(content.encode('utf-8')) |
68 | @@ -58,6 +55,7 @@ | |||
69 | 58 | self.overrideAttr(_urllib2_wrappers.opt_ssl_ca_certs, 'default', | 55 | self.overrideAttr(_urllib2_wrappers.opt_ssl_ca_certs, 'default', |
70 | 59 | os.path.join(self.test_dir, u"nonexisting.pem")) | 56 | os.path.join(self.test_dir, u"nonexisting.pem")) |
71 | 60 | self.warnings = [] | 57 | self.warnings = [] |
72 | 58 | |||
73 | 61 | def warning(*args): | 59 | def warning(*args): |
74 | 62 | self.warnings.append(args[0] % args[1:]) | 60 | self.warnings.append(args[0] % args[1:]) |
75 | 63 | self.overrideAttr(trace, 'warning', warning) | 61 | self.overrideAttr(trace, 'warning', warning) |
76 | @@ -67,7 +65,7 @@ | |||
77 | 67 | "is not valid for \"ssl.ca_certs\"") | 65 | "is not valid for \"ssl.ca_certs\"") |
78 | 68 | 66 | ||
79 | 69 | 67 | ||
81 | 70 | class CertReqsConfigTests(TestCaseInTempDir): | 68 | class CertReqsConfigTests(tests.TestCaseInTempDir): |
82 | 71 | 69 | ||
83 | 72 | def test_default(self): | 70 | def test_default(self): |
84 | 73 | stack = config.MemoryStack("") | 71 | stack = config.MemoryStack("") |
85 | @@ -82,35 +80,41 @@ | |||
86 | 82 | self.assertRaises(ConfigOptionValueError, stack.get, "ssl.cert_reqs") | 80 | self.assertRaises(ConfigOptionValueError, stack.get, "ssl.cert_reqs") |
87 | 83 | 81 | ||
88 | 84 | 82 | ||
90 | 85 | class MatchHostnameTests(TestCase): | 83 | class MatchHostnameTests(tests.TestCase): |
91 | 84 | |||
92 | 85 | def setUp(self): | ||
93 | 86 | super(MatchHostnameTests, self).setUp() | ||
94 | 87 | if sys.version_info < (2, 7, 9): | ||
95 | 88 | raise tests.TestSkipped( | ||
96 | 89 | 'python version too old to provide proper' | ||
97 | 90 | ' https hostname verification') | ||
98 | 86 | 91 | ||
99 | 87 | def test_no_certificate(self): | 92 | def test_no_certificate(self): |
100 | 88 | self.assertRaises(ValueError, | 93 | self.assertRaises(ValueError, |
102 | 89 | _urllib2_wrappers.match_hostname, {}, "example.com") | 94 | ssl.match_hostname, {}, "example.com") |
103 | 90 | 95 | ||
104 | 91 | def test_wildcards_in_cert(self): | 96 | def test_wildcards_in_cert(self): |
105 | 92 | def ok(cert, hostname): | 97 | def ok(cert, hostname): |
107 | 93 | _urllib2_wrappers.match_hostname(cert, hostname) | 98 | ssl.match_hostname(cert, hostname) |
108 | 99 | |||
109 | 100 | def not_ok(cert, hostname): | ||
110 | 101 | self.assertRaises( | ||
111 | 102 | ssl.CertificateError, | ||
112 | 103 | ssl.match_hostname, cert, hostname) | ||
113 | 94 | 104 | ||
114 | 95 | # Python Issue #17980: avoid denials of service by refusing more than | 105 | # Python Issue #17980: avoid denials of service by refusing more than |
115 | 96 | # one wildcard per fragment. | 106 | # one wildcard per fragment. |
125 | 97 | cert = {'subject': ((('commonName', 'a*b.com'),),)} | 107 | ok({'subject': ((('commonName', 'a*b.com'),),)}, 'axxb.com') |
126 | 98 | ok(cert, 'axxb.com') | 108 | not_ok({'subject': ((('commonName', 'a*b.co*'),),)}, 'axxb.com') |
127 | 99 | cert = {'subject': ((('commonName', 'a*b.co*'),),)} | 109 | not_ok({'subject': ((('commonName', 'a*b*.com'),),)}, 'axxbxxc.com') |
119 | 100 | ok(cert, 'axxb.com') | ||
120 | 101 | cert = {'subject': ((('commonName', 'a*b*.com'),),)} | ||
121 | 102 | try: | ||
122 | 103 | _urllib2_wrappers.match_hostname(cert, 'axxbxxc.com') | ||
123 | 104 | except ValueError as e: | ||
124 | 105 | self.assertIn("too many wildcards", str(e)) | ||
128 | 106 | 110 | ||
129 | 107 | def test_no_valid_attributes(self): | 111 | def test_no_valid_attributes(self): |
131 | 108 | self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname, | 112 | self.assertRaises(ssl.CertificateError, ssl.match_hostname, |
132 | 109 | {"Problem": "Solved"}, "example.com") | 113 | {"Problem": "Solved"}, "example.com") |
133 | 110 | 114 | ||
134 | 111 | def test_common_name(self): | 115 | def test_common_name(self): |
135 | 112 | cert = {'subject': ((('commonName', 'example.com'),),)} | 116 | cert = {'subject': ((('commonName', 'example.com'),),)} |
136 | 113 | self.assertIs(None, | 117 | self.assertIs(None, |
139 | 114 | _urllib2_wrappers.match_hostname(cert, "example.com")) | 118 | ssl.match_hostname(cert, "example.com")) |
140 | 115 | self.assertRaises(CertificateError, _urllib2_wrappers.match_hostname, | 119 | self.assertRaises(ssl.CertificateError, ssl.match_hostname, |
141 | 116 | cert, "example.org") | 120 | cert, "example.org") |
142 | 117 | 121 | ||
143 | === modified file 'bzrlib/transport/http/_urllib2_wrappers.py' | |||
144 | --- bzrlib/transport/http/_urllib2_wrappers.py 2013-05-20 16:38:11 +0000 | |||
145 | +++ bzrlib/transport/http/_urllib2_wrappers.py 2016-01-31 13:08:48 +0000 | |||
146 | @@ -1,4 +1,4 @@ | |||
148 | 1 | # Copyright (C) 2006-2012 Canonical Ltd | 1 | # Copyright (C) 2006-2013, 2016 Canonical Ltd |
149 | 2 | # | 2 | # |
150 | 3 | # This program is free software; you can redistribute it and/or modify | 3 | # This program is free software; you can redistribute it and/or modify |
151 | 4 | # it under the terms of the GNU General Public License as published by | 4 | # it under the terms of the GNU General Public License as published by |
152 | @@ -56,6 +56,7 @@ | |||
153 | 56 | import urllib2 | 56 | import urllib2 |
154 | 57 | import urlparse | 57 | import urlparse |
155 | 58 | import re | 58 | import re |
156 | 59 | import ssl | ||
157 | 59 | import sys | 60 | import sys |
158 | 60 | import time | 61 | import time |
159 | 61 | 62 | ||
160 | @@ -70,23 +71,33 @@ | |||
161 | 70 | transport, | 71 | transport, |
162 | 71 | ui, | 72 | ui, |
163 | 72 | urlutils, | 73 | urlutils, |
168 | 73 | ) | 74 | ) |
169 | 74 | lazy_import.lazy_import(globals(), """ | 75 | |
170 | 75 | import ssl | 76 | try: |
171 | 76 | """) | 77 | _ = (ssl.match_hostname, ssl.CertificateError) |
172 | 78 | except AttributeError: | ||
173 | 79 | # Provide fallbacks for python < 2.7.9 | ||
174 | 80 | def match_hostname(cert, host): | ||
175 | 81 | trace.warning( | ||
176 | 82 | '%s cannot be verified, https certificates verification is only' | ||
177 | 83 | ' available for python versions >= 2.7.9' % (host,)) | ||
178 | 84 | ssl.match_hostname = match_hostname | ||
179 | 85 | ssl.CertificateError = ValueError | ||
180 | 77 | 86 | ||
181 | 78 | 87 | ||
182 | 79 | # Note for packagers: if there is no package providing certs for your platform, | 88 | # Note for packagers: if there is no package providing certs for your platform, |
183 | 80 | # the curl project produces http://curl.haxx.se/ca/cacert.pem weekly. | 89 | # the curl project produces http://curl.haxx.se/ca/cacert.pem weekly. |
184 | 81 | _ssl_ca_certs_known_locations = [ | 90 | _ssl_ca_certs_known_locations = [ |
190 | 82 | u'/etc/ssl/certs/ca-certificates.crt', # Ubuntu/debian/gentoo | 91 | u'/etc/ssl/certs/ca-certificates.crt', # Ubuntu/debian/gentoo |
191 | 83 | u'/etc/pki/tls/certs/ca-bundle.crt', # Fedora/CentOS/RH | 92 | u'/etc/pki/tls/certs/ca-bundle.crt', # Fedora/CentOS/RH |
192 | 84 | u'/etc/ssl/ca-bundle.pem', # OpenSuse | 93 | u'/etc/ssl/ca-bundle.pem', # OpenSuse |
193 | 85 | u'/etc/ssl/cert.pem', # OpenSuse | 94 | u'/etc/ssl/cert.pem', # OpenSuse |
194 | 86 | u"/usr/local/share/certs/ca-root-nss.crt", # FreeBSD | 95 | u"/usr/local/share/certs/ca-root-nss.crt", # FreeBSD |
195 | 87 | # XXX: Needs checking, can't trust the interweb ;) -- vila 2012-01-25 | 96 | # XXX: Needs checking, can't trust the interweb ;) -- vila 2012-01-25 |
198 | 88 | u'/etc/openssl/certs/ca-certificates.crt', # Solaris | 97 | u'/etc/openssl/certs/ca-certificates.crt', # Solaris |
199 | 89 | ] | 98 | ] |
200 | 99 | |||
201 | 100 | |||
202 | 90 | def default_ca_certs(): | 101 | def default_ca_certs(): |
203 | 91 | if sys.platform == 'win32': | 102 | if sys.platform == 'win32': |
204 | 92 | return os.path.join(os.path.dirname(sys.executable), u"cacert.pem") | 103 | return os.path.join(os.path.dirname(sys.executable), u"cacert.pem") |
205 | @@ -115,13 +126,12 @@ | |||
206 | 115 | def cert_reqs_from_store(unicode_str): | 126 | def cert_reqs_from_store(unicode_str): |
207 | 116 | import ssl | 127 | import ssl |
208 | 117 | try: | 128 | try: |
213 | 118 | return { | 129 | return {"required": ssl.CERT_REQUIRED, |
214 | 119 | "required": ssl.CERT_REQUIRED, | 130 | "none": ssl.CERT_NONE}[unicode_str] |
211 | 120 | "none": ssl.CERT_NONE | ||
212 | 121 | }[unicode_str] | ||
215 | 122 | except KeyError: | 131 | except KeyError: |
216 | 123 | raise ValueError("invalid value %s" % unicode_str) | 132 | raise ValueError("invalid value %s" % unicode_str) |
217 | 124 | 133 | ||
218 | 134 | |||
219 | 125 | def default_ca_reqs(): | 135 | def default_ca_reqs(): |
220 | 126 | if sys.platform in ('win32', 'darwin'): | 136 | if sys.platform in ('win32', 'darwin'): |
221 | 127 | # FIXME: Once we get a native access to root certificates there, this | 137 | # FIXME: Once we get a native access to root certificates there, this |
222 | @@ -131,10 +141,10 @@ | |||
223 | 131 | return u'required' | 141 | return u'required' |
224 | 132 | 142 | ||
225 | 133 | opt_ssl_ca_certs = config.Option('ssl.ca_certs', | 143 | opt_ssl_ca_certs = config.Option('ssl.ca_certs', |
230 | 134 | from_unicode=ca_certs_from_store, | 144 | from_unicode=ca_certs_from_store, |
231 | 135 | default=default_ca_certs, | 145 | default=default_ca_certs, |
232 | 136 | invalid='warning', | 146 | invalid='warning', |
233 | 137 | help="""\ | 147 | help="""\ |
234 | 138 | Path to certification authority certificates to trust. | 148 | Path to certification authority certificates to trust. |
235 | 139 | 149 | ||
236 | 140 | This should be a valid path to a bundle containing all root Certificate | 150 | This should be a valid path to a bundle containing all root Certificate |
237 | @@ -144,10 +154,10 @@ | |||
238 | 144 | """) | 154 | """) |
239 | 145 | 155 | ||
240 | 146 | opt_ssl_cert_reqs = config.Option('ssl.cert_reqs', | 156 | opt_ssl_cert_reqs = config.Option('ssl.cert_reqs', |
245 | 147 | default=default_ca_reqs, | 157 | default=default_ca_reqs, |
246 | 148 | from_unicode=cert_reqs_from_store, | 158 | from_unicode=cert_reqs_from_store, |
247 | 149 | invalid='error', | 159 | invalid='error', |
248 | 150 | help="""\ | 160 | help="""\ |
249 | 151 | Whether to require a certificate from the remote side. (default:required) | 161 | Whether to require a certificate from the remote side. (default:required) |
250 | 152 | 162 | ||
251 | 153 | Possible values: | 163 | Possible values: |
252 | @@ -398,68 +408,6 @@ | |||
253 | 398 | self._wrap_socket_for_reporting(self.sock) | 408 | self._wrap_socket_for_reporting(self.sock) |
254 | 399 | 409 | ||
255 | 400 | 410 | ||
256 | 401 | # These two methods were imported from Python 3.2's ssl module | ||
257 | 402 | |||
258 | 403 | def _dnsname_to_pat(dn, max_wildcards=1): | ||
259 | 404 | pats = [] | ||
260 | 405 | for frag in dn.split(r'.'): | ||
261 | 406 | if frag.count('*') > max_wildcards: | ||
262 | 407 | # Python Issue #17980: avoid denials of service by refusing more | ||
263 | 408 | # than one wildcard per fragment. A survery of established | ||
264 | 409 | # policy among SSL implementations showed it to be a | ||
265 | 410 | # reasonable choice. | ||
266 | 411 | raise ValueError( | ||
267 | 412 | "too many wildcards in certificate DNS name: " + repr(dn)) | ||
268 | 413 | if frag == '*': | ||
269 | 414 | # When '*' is a fragment by itself, it matches a non-empty dotless | ||
270 | 415 | # fragment. | ||
271 | 416 | pats.append('[^.]+') | ||
272 | 417 | else: | ||
273 | 418 | # Otherwise, '*' matches any dotless fragment. | ||
274 | 419 | frag = re.escape(frag) | ||
275 | 420 | pats.append(frag.replace(r'\*', '[^.]*')) | ||
276 | 421 | return re.compile(r'\A' + r'\.'.join(pats) + r'\Z', re.IGNORECASE) | ||
277 | 422 | |||
278 | 423 | |||
279 | 424 | def match_hostname(cert, hostname): | ||
280 | 425 | """Verify that *cert* (in decoded format as returned by | ||
281 | 426 | SSLSocket.getpeercert()) matches the *hostname*. RFC 2818 rules | ||
282 | 427 | are mostly followed, but IP addresses are not accepted for *hostname*. | ||
283 | 428 | |||
284 | 429 | CertificateError is raised on failure. On success, the function | ||
285 | 430 | returns nothing. | ||
286 | 431 | """ | ||
287 | 432 | if not cert: | ||
288 | 433 | raise ValueError("empty or no certificate") | ||
289 | 434 | dnsnames = [] | ||
290 | 435 | san = cert.get('subjectAltName', ()) | ||
291 | 436 | for key, value in san: | ||
292 | 437 | if key == 'DNS': | ||
293 | 438 | if _dnsname_to_pat(value).match(hostname): | ||
294 | 439 | return | ||
295 | 440 | dnsnames.append(value) | ||
296 | 441 | if not san: | ||
297 | 442 | # The subject is only checked when subjectAltName is empty | ||
298 | 443 | for sub in cert.get('subject', ()): | ||
299 | 444 | for key, value in sub: | ||
300 | 445 | # XXX according to RFC 2818, the most specific Common Name | ||
301 | 446 | # must be used. | ||
302 | 447 | if key == 'commonName': | ||
303 | 448 | if _dnsname_to_pat(value).match(hostname): | ||
304 | 449 | return | ||
305 | 450 | dnsnames.append(value) | ||
306 | 451 | if len(dnsnames) > 1: | ||
307 | 452 | raise errors.CertificateError( | ||
308 | 453 | "hostname %r doesn't match either of %s" | ||
309 | 454 | % (hostname, ', '.join(map(repr, dnsnames)))) | ||
310 | 455 | elif len(dnsnames) == 1: | ||
311 | 456 | raise errors.CertificateError("hostname %r doesn't match %r" % | ||
312 | 457 | (hostname, dnsnames[0])) | ||
313 | 458 | else: | ||
314 | 459 | raise errors.CertificateError("no appropriate commonName or " | ||
315 | 460 | "subjectAltName fields were found") | ||
316 | 461 | |||
317 | 462 | |||
318 | 463 | class HTTPSConnection(AbstractHTTPConnection, httplib.HTTPSConnection): | 411 | class HTTPSConnection(AbstractHTTPConnection, httplib.HTTPSConnection): |
319 | 464 | 412 | ||
320 | 465 | def __init__(self, host, port=None, key_file=None, cert_file=None, | 413 | def __init__(self, host, port=None, key_file=None, cert_file=None, |
321 | @@ -503,9 +451,10 @@ | |||
322 | 503 | "'bzr help ssl.ca_certs' for more information on setting " | 451 | "'bzr help ssl.ca_certs' for more information on setting " |
323 | 504 | "trusted CAs.") | 452 | "trusted CAs.") |
324 | 505 | try: | 453 | try: |
326 | 506 | ssl_sock = ssl.wrap_socket(self.sock, self.key_file, self.cert_file, | 454 | ssl_sock = ssl.wrap_socket( |
327 | 455 | self.sock, self.key_file, self.cert_file, | ||
328 | 507 | cert_reqs=cert_reqs, ca_certs=ca_certs) | 456 | cert_reqs=cert_reqs, ca_certs=ca_certs) |
330 | 508 | except ssl.SSLError, e: | 457 | except ssl.SSLError: |
331 | 509 | trace.note( | 458 | trace.note( |
332 | 510 | "\n" | 459 | "\n" |
333 | 511 | "See `bzr help ssl.ca_certs` for how to specify trusted CA" | 460 | "See `bzr help ssl.ca_certs` for how to specify trusted CA" |
334 | @@ -515,7 +464,7 @@ | |||
335 | 515 | raise | 464 | raise |
336 | 516 | if cert_reqs == ssl.CERT_REQUIRED: | 465 | if cert_reqs == ssl.CERT_REQUIRED: |
337 | 517 | peer_cert = ssl_sock.getpeercert() | 466 | peer_cert = ssl_sock.getpeercert() |
339 | 518 | match_hostname(peer_cert, host) | 467 | ssl.match_hostname(peer_cert, host) |
340 | 519 | 468 | ||
341 | 520 | # Wrap the ssl socket before anybody use it | 469 | # Wrap the ssl socket before anybody use it |
342 | 521 | self._wrap_socket_for_reporting(ssl_sock) | 470 | self._wrap_socket_for_reporting(ssl_sock) |
343 | @@ -833,7 +782,7 @@ | |||
344 | 833 | % (request, request.connection.sock.getsockname()) | 782 | % (request, request.connection.sock.getsockname()) |
345 | 834 | response = connection.getresponse() | 783 | response = connection.getresponse() |
346 | 835 | convert_to_addinfourl = True | 784 | convert_to_addinfourl = True |
348 | 836 | except (ssl.SSLError, errors.CertificateError): | 785 | except (ssl.SSLError, ssl.CertificateError): |
349 | 837 | # Something is wrong with either the certificate or the hostname, | 786 | # Something is wrong with either the certificate or the hostname, |
350 | 838 | # re-trying won't help | 787 | # re-trying won't help |
351 | 839 | raise | 788 | raise |
352 | 840 | 789 | ||
353 | === modified file 'doc/en/release-notes/bzr-2.7.txt' | |||
354 | --- doc/en/release-notes/bzr-2.7.txt 2016-01-22 08:02:12 +0000 | |||
355 | +++ doc/en/release-notes/bzr-2.7.txt 2016-01-31 13:08:48 +0000 | |||
356 | @@ -70,6 +70,9 @@ | |||
357 | 70 | or UnicodeEncodeError when given unicode strings rather than bytes. | 70 | or UnicodeEncodeError when given unicode strings rather than bytes. |
358 | 71 | (Vincent Ladeuil, #106898) | 71 | (Vincent Ladeuil, #106898) |
359 | 72 | 72 | ||
360 | 73 | * Use ssl.match_hostname from the python ssl module and stop carrying a | ||
361 | 74 | specific version that has become obsolete. (Vincent Ladeuil, #1538480) | ||
362 | 75 | |||
363 | 73 | Changed Behaviour | 76 | Changed Behaviour |
364 | 74 | ***************** | 77 | ***************** |
365 | 75 | 78 |
Vincent, what you've done here is excellent and definitely an improvement over the contributed patch, exempli gratia, adding the not_ok function.
What I'm curious about is this patch requires python >= 2.7.9. Do we plan to bump our python version requirement from 2.6 to 2.7.9?
The other part of the patch checks for match_hostname in the ssl library and, if not found, imports it from backports.
1. Is that a worthwhile exercise in light of python version requirements?
2. Is backports normally available or would we change our dependencies to get it?