[PATCH 2 of 2] url: check server certificates when connecting through proxy (issue2407)

Henrik Stuart hg at hstuart.dk
Wed Nov 3 13:31:46 CDT 2010


On 01-11-2010 12:01, Mads Kiilerich wrote:
> On 11/01/2010 11:12 AM, Thomas Arendsen Hein wrote:
>> * Mads Kiilerich<mads at kiilerich.com>  [20101101 02:16]:
>>> # HG changeset patch
>>> # User Mads Kiilerich<mads at kiilerich.com>
>>> # Date 1288573886 -3600
>>> # Branch stable
>>> # Node ID 391543217c1a222deb236d1fcc426e264921cd82
>>> # Parent  191f4f62a3ae7d9f27b3b16267c4f1400fd2451f
>>> url: check server certificates when connecting through proxy (issue2407)
>>
>> Does not help for me:
>>
>>> diff --git a/mercurial/url.py b/mercurial/url.py
>>> --- a/mercurial/url.py
>>> +++ b/mercurial/url.py
>>> @@ -540,8 +540,25 @@
>>>                   self.sock = socket.socket(socket.AF_INET,
>>> socket.SOCK_STREAM)
>>>                   self.sock.connect((self.host, self.port))
>>>                   if _generic_proxytunnel(self):
>>> -                    self.sock = _ssl_wrap_socket(self.sock,
>>> self.key_file,
>>> -                            self.cert_file)
>>> +                    if hasattr(self, 'ui'):
>>> +                        cacerts = self.ui.config('web', 'cacerts')
>>> +                    else:
>>> +                        cacerts = None
>>> +
>>> +                    if cacerts:
>>> +                        self.sock = _ssl_wrap_socket(self.sock,
>>> self.key_file,
>>> +                                self.cert_file,
>>> cert_reqs=CERT_REQUIRED,
>>> +                                ca_certs=cacerts)
>>> +                        realhost = self.realhostport.rsplit(':', 1)[0]
>>> +                        msg = _verifycert(self.sock.getpeercert(),
>>> realhost)
>>> +                        if msg:
>>> +                            raise util.Abort(_('%s certificate
>>> error: %s') %
>>> +                                             (realhost, msg))
>>> +                        self.ui.debug('%s certificate successfully
>>> verified\n' %
>>> +                                      realhost)
>>> +                    else:
>>> +                        self.sock = _ssl_wrap_socket(self.sock,
>>> self.key_file,
>>> +                                self.cert_file)
>>>               else:
>>>                   BetterHTTPS.connect(self)
>>
>> This code path is not executed for me, but httpconnection.connect()
>> (url.py, line 140), which has no handling of cacerts.
> 
> Line 338?
> 
> Can you give a test case? Are you connecting to a https server through
> proxy but using "http://"?
> 
> FWIW I don't understand why httpconnection.connect unconditionally wraps
> in SSL if SSL is available and we are using proxy and we can CONNECT.
> Shouldn't that only be done for https connections - which won't end up
> in that code anyway? And what's the story behind the comment that we
> don't support client x509 certificates?

The Python guys broke functionality between 2.5 and 2.6, as you probably
have discovered by now (actually they tried to introduce the CONNECT
support in 2.6, but I'll leave interested people to look at their
implementation and why we still have our own).

In earlier versions of Python, all proxy handling, also for
HTTPS-connections, went through httpconnection and not httpsconnection
as could be presumed. They figured out this was a bad idea and changed
it in 2.6, completely breaking our proxy support (this is why we have
support for it both in httpsconnection and httpconnection).

Also, in earlier versions, client certificates weren't propagated
properly to the remote side. Client certificates should work just fine
from 2.6 and onward; the comment could probably do with being revised.

-- 
Kind regards,
  Henrik Stuart


More information about the Mercurial-devel mailing list