[PATCH] https: support tls sni (server name indication) for https urls (issue3090)
Alex Orange
crazycasta at gmail.com
Tue Jan 13 20:41:33 CST 2015
Ok, I've found the problem, wasn't checking ca_certs for noneness. I wasn't
aware that there was a path that it could be None and so wasn't checking
for it. I've checked everything else and everything either can't be None
(sock, ssl_version, cert_reqs), is checked (certfile and ca_certs), or is
ok to be None. keyfile is okay to be None because this indicates that the
private key is in the cert file (I don't believe we actually use this, but
as far as I can tell it's the best consistency with the underlying
behavior). server_hostname can also be None, which I presume means that SNI
is not invoked (I've tested that it doesn't throw anything, but I'm not
sure what effect it has). test-https.t works and I'm running the rest of
the tests, assuming they all pass I'll send a new patch.
On Tue, Jan 13, 2015 at 6:57 PM, Augie Fackler <raf at durin42.com> wrote:
>
> On Jan 13, 2015, at 2:05 PM, Augie Fackler <raf at durin42.com> wrote:
>
> > On Mon, Jan 12, 2015 at 06:11:05PM -0700, Alex Orange wrote:
> >> # HG changeset patch
> >> # User Alex Orange <crazycasta at gmail.com>
> >> # Date 1421110880 25200
> >> # Mon Jan 12 18:01:20 2015 -0700
> >> # Node ID b683de04cfd910f8f9c1c9400c748a11ef853c35
> >> # Parent 678f53865c6860a950392691814766957ee89316
> >> https: support tls sni (server name indication) for https urls
> (issue3090)
> >
> > This looks like a good start. I'll do Antoine's proposal as a
> > followup, as well as a couple of other tweaks I see that'll make doing
> > that less of a headache.
>
> I’ve encountered some test failures with this patch now that we actually
> run test-https.t on 2.7.9 - can you rebase this over mpm’s latest and see
> if you see them?
>
> (I’d have posted a link, but I’m hunting an unrelated slowness problem in
> obsolete markers on my FreeBSD server where I push things.)
>
> >
> > Many thanks! This has been on my todo list since the backport was
> announced.
> >
> >>
> >> SNI is a common way of sharing servers across multiple domains using
> separate
> >> SSL certificates. As of Python 2.7.9 SSLContext has been backported from
> >> Python 3. This patch changes sslutil's ssl_wrap_socket to use
> SSLContext and
> >> take a server hostname as and argument. It also changes the url module
> to make
> >> use of this argument.
> >>
> >> diff -r 678f53865c68 -r b683de04cfd9 mercurial/sslutil.py
> >> --- a/mercurial/sslutil.py Thu Jan 08 00:01:03 2015 +0100
> >> +++ b/mercurial/sslutil.py Mon Jan 12 18:01:20 2015 -0700
> >> @@ -15,16 +15,39 @@
> >> import ssl
> >> CERT_REQUIRED = ssl.CERT_REQUIRED
> >> PROTOCOL_TLSv1 = ssl.PROTOCOL_TLSv1
> >> - def ssl_wrap_socket(sock, keyfile, certfile,
> ssl_version=PROTOCOL_TLSv1,
> >> - cert_reqs=ssl.CERT_NONE, ca_certs=None):
> >> - sslsocket = ssl.wrap_socket(sock, keyfile, certfile,
> >> - cert_reqs=cert_reqs,
> ca_certs=ca_certs,
> >> - ssl_version=ssl_version)
> >> - # check if wrap_socket failed silently because socket had been
> closed
> >> - # - see http://bugs.python.org/issue13721
> >> - if not sslsocket.cipher():
> >> - raise util.Abort(_('ssl connection failed'))
> >> - return sslsocket
> >> + try:
> >> + ssl_context = ssl.SSLContext
> >> +
> >> + def ssl_wrap_socket(sock, keyfile, certfile,
> ssl_version=PROTOCOL_TLSv1,
> >> + cert_reqs=ssl.CERT_NONE, ca_certs=None,
> >> + server_hostname=None):
> >> + sslcontext = ssl.SSLContext(ssl_version)
> >> + if keyfile is not None:
> >> + sslcontext.load_cert_chain(certfile, keyfile)
> >> + sslcontext.verify_mode = cert_reqs
> >> + sslcontext.load_verify_locations(cafile=ca_certs)
> >> +
> >> + sslsocket = sslcontext.wrap_socket(sock,
> >> +
> server_hostname=server_hostname)
> >> + # check if wrap_socket failed silently because socket had
> been
> >> + # closed
> >> + # - see http://bugs.python.org/issue13721
> >> + if not sslsocket.cipher():
> >> + raise util.Abort(_('ssl connection failed'))
> >> + return sslsocket
> >> + except AttributeError:
> >> + def ssl_wrap_socket(sock, keyfile, certfile,
> ssl_version=PROTOCOL_TLSv1,
> >> + cert_reqs=ssl.CERT_NONE, ca_certs=None,
> >> + server_hostname=None):
> >> + sslsocket = ssl.wrap_socket(sock, keyfile, certfile,
> >> + cert_reqs=cert_reqs,
> ca_certs=ca_certs,
> >> + ssl_version=ssl_version)
> >> + # check if wrap_socket failed silently because socket had
> been
> >> + # closed
> >> + # - see http://bugs.python.org/issue13721
> >> + if not sslsocket.cipher():
> >> + raise util.Abort(_('ssl connection failed'))
> >> + return sslsocket
> >> except ImportError:
> >> CERT_REQUIRED = 2
> >>
> >> @@ -33,7 +56,8 @@
> >> import socket, httplib
> >>
> >> def ssl_wrap_socket(sock, keyfile, certfile,
> ssl_version=PROTOCOL_TLSv1,
> >> - cert_reqs=CERT_REQUIRED, ca_certs=None):
> >> + cert_reqs=CERT_REQUIRED, ca_certs=None,
> >> + server_hostname=None):
> >> if not util.safehasattr(socket, 'ssl'):
> >> raise util.Abort(_('Python SSL support not found'))
> >> if ca_certs:
> >> diff -r 678f53865c68 -r b683de04cfd9 mercurial/url.py
> >> --- a/mercurial/url.py Thu Jan 08 00:01:03 2015 +0100
> >> +++ b/mercurial/url.py Mon Jan 12 18:01:20 2015 -0700
> >> @@ -185,7 +185,8 @@
> >> self.sock.connect((self.host, self.port))
> >> if _generic_proxytunnel(self):
> >> # we do not support client X.509 certificates
> >> - self.sock = sslutil.ssl_wrap_socket(self.sock, None,
> None)
> >> + self.sock = sslutil.ssl_wrap_socket(self.sock, None,
> None,
> >> +
> server_hostname=self.host)
> >> else:
> >> keepalive.HTTPConnection.connect(self)
> >>
> >> @@ -341,7 +342,7 @@
> >> _generic_proxytunnel(self)
> >> host = self.realhostport.rsplit(':', 1)[0]
> >> self.sock = sslutil.ssl_wrap_socket(
> >> - self.sock, self.key_file, self.cert_file,
> >> + self.sock, self.key_file, self.cert_file,
> server_hostname=host,
> >> **sslutil.sslkwargs(self.ui, host))
> >> sslutil.validator(self.ui, host)(self.sock)
> >>
> >> _______________________________________________
> >> Mercurial-devel mailing list
> >> Mercurial-devel at selenic.com
> >> http://selenic.com/mailman/listinfo/mercurial-devel
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150113/46777523/attachment.html>
More information about the Mercurial-devel
mailing list