# HG changeset patch # User Mads Kiilerich # Date 1285886819 -7200 # Node ID f2937d6492c539cf02beef2b2071f6a93fd140ad # Parent 1c9bb7e00f7182198b648d8126406ece67c25899 url: verify correctness of https server certificates (issue2407) Pythons SSL module verifies that certificates received for HTTPS are valid according to the specified cacerts, but it doesn't verify that the certificate is for the host we connect to. We now explicitly verify that the commonName in the received certificate matches the requested hostname and is valid for the time being. This is a minimal patch where we try to fail to the safe side, but we do still rely on Python's SSL functionality and do not try to implement the standards fully and correctly. CRLs and subjectAltName are not handled and proxies haven't been considered. This change might break connections to some sites if cacerts is specified and the certificates (by our definition) isn't correct. The workaround is to disable cacerts which in most cases isn't much worse than it was before with cacerts. diff -r 1c9bb7e00f71 -r f2937d6492c5 mercurial/url.py --- a/mercurial/url.py Tue Sep 28 00:41:08 2010 +0200 +++ b/mercurial/url.py Fri Oct 01 00:46:59 2010 +0200 @@ -7,7 +7,7 @@ # This software may be used and distributed according to the terms of the # GNU General Public License version 2 or any later version. -import urllib, urllib2, urlparse, httplib, os, re, socket, cStringIO +import urllib, urllib2, urlparse, httplib, os, re, socket, cStringIO, time from i18n import _ import keepalive, util @@ -469,6 +469,31 @@ _generic_start_transaction(self, h, req) return keepalive.HTTPHandler._start_transaction(self, h, req) +def _verifycert(cert, hostname): + '''Verify that cert (in socket.getpeercert() format) matches hostname and is + valid at this time. CRLs and subjectAltName are not handled. + + Returns error message if any problems are found and None on success. + ''' + if not cert: + return _('no certificate received') + notafter = cert.get('notAfter') + if notafter and time.time() > ssl.cert_time_to_seconds(notafter): + return _('certificate expired %s') % notafter + notbefore = cert.get('notBefore') + if notbefore and time.time() < ssl.cert_time_to_seconds(notbefore): + return _('certificate not valid before %s') % notbefore + dnsname = hostname.lower() + for s in cert.get('subject', []): + key, value = s[0] + if key == 'commonName': + certname = value.lower() + if (certname == dnsname or + '.' in dnsname and certname == '*.' + dnsname.split('.', 1)[1]): + return None + return _('certificate is for %s') % certname + return _('no commonName found in certificate') + if has_https: class BetterHTTPS(httplib.HTTPSConnection): send = keepalive.safesend @@ -484,7 +509,11 @@ self.sock = _ssl_wrap_socket(sock, self.key_file, self.cert_file, cert_reqs=CERT_REQUIRED, ca_certs=cacerts) - self.ui.debug(_('server identity verification succeeded\n')) + msg = _verifycert(self.sock.getpeercert(), self.host) + if msg: + raise util.Abort('%s certificate error: %s' % (self.host, msg)) + self.ui.debug(_('%s certificate successfully verified\n') % + self.host) else: httplib.HTTPSConnection.connect(self) diff -r 1c9bb7e00f71 -r f2937d6492c5 tests/test-url.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/test-url.py Fri Oct 01 00:46:59 2010 +0200 @@ -0,0 +1,41 @@ +#!/usr/bin/env python + +def check(a, b): + if a != b: + print (a, b) + +from mercurial.url import _verifycert + +# Test non-wildcard certificates +check(_verifycert({'subject': ((('commonName', 'example.com'),),)}, 'example.com'), + None) +check(_verifycert({'subject': ((('commonName', 'example.com'),),)}, 'www.example.com'), + 'certificate is for example.com') +check(_verifycert({'subject': ((('commonName', 'www.example.com'),),)}, 'example.com'), + 'certificate is for www.example.com') + +# Test wildcard certificates +check(_verifycert({'subject': ((('commonName', '*.example.com'),),)}, 'www.example.com'), + None) +check(_verifycert({'subject': ((('commonName', '*.example.com'),),)}, 'example.com'), + 'certificate is for *.example.com') +check(_verifycert({'subject': ((('commonName', '*.example.com'),),)}, 'w.w.example.com'), + 'certificate is for *.example.com') + +# Avoid some pitfalls +check(_verifycert({'subject': ((('commonName', '*.foo'),),)}, 'foo'), + 'certificate is for *.foo') +check(_verifycert({'subject': ((('commonName', '*o'),),)}, 'foo'), + 'certificate is for *o') + +import time +lastyear = time.gmtime().tm_year - 1 +nextyear = time.gmtime().tm_year + 1 +check(_verifycert({'notAfter': 'May 9 00:00:00 %s GMT' % lastyear}, 'example.com'), + 'certificate expired May 9 00:00:00 %s GMT' % lastyear) +check(_verifycert({'notBefore': 'May 9 00:00:00 %s GMT' % nextyear}, 'example.com'), + 'certificate not valid before May 9 00:00:00 %s GMT' % nextyear) +check(_verifycert({'notAfter': 'Sep 29 15:29:48 %s GMT' % nextyear, 'subject': ()}, 'example.com'), + 'no commonName found in certificate') +check(_verifycert(None, 'example.com'), + 'no certificate received')