[PATCH 1 of 2] url: debug print ssl certificate if verify failed

Mads Kiilerich mads at kiilerich.com
Sat Jan 8 21:26:58 CST 2011


Yuya Nishihara wrote, On 01/08/2011 02:08 PM:
> # HG changeset patch
> # User Yuya Nishihara<yuya at tcha.org>
> # Date 1294491293 -32400
> # Branch stable
> # Node ID 160f24a7970a402d0f7df1912b92419d7acbd8f3
> # Parent  74ed7f84498b066d6d90c97a3b15240b499365c1
> url: debug print ssl certificate if verify failed

Thanks, but ...

SSLSocket.getpeercert do not return the certificate. It just returns 
some information from it, some information in its own format which just 
shows which server the verified certificate applies to and when it expires.

One way to really show the certificate is

   $ python -c 'import ssl; print 
ssl.get_server_certificate(("example.com", 443))' | openssl x509 -text

This patch will not show the certificate but something I would call 
something else.

This patch is currently helpful because it cares about subjectAltName 
and our _verifycert doesn't. That's a bug that should be fixed.

But if _verifycert fails a certificate it should (and do already) tell 
why it fails it. I thus don't think this patch will show relevant 
information in any relevant cases.

Can you describe some uses cases and include some sample output?

> diff --git a/mercurial/url.py b/mercurial/url.py
> --- a/mercurial/url.py
> +++ b/mercurial/url.py
> @@ -509,6 +509,18 @@ def _verifycert(cert, hostname):
>               return _('certificate is for %s') % certname
>       return _('no commonName found in certificate')
>
> +def _printcert(cert, write):

I think I would prefer to either just pass ui as first parameter or to 
let the function return a multi-line string.

> +    write('ssl certificate:\n')
> +    for k in ('version', 'notBefore', 'notAfter'):

AFAIK we will (with this patch) never see anything but notAfter - and we 
will only get this far if that isn't a problem.

> +        if k in cert:
> +            write(' %s: %s\n' % (k, cert[k]))
> +    for k in ('issuer', 'subject'):

AFAIK we will never see issuer here.

> +        for s in cert.get(k, []):
> +            key, value = s[0]
> +            write(' %s.%s: %s\n' % (k, key, value.encode('ascii', 'replace')))
> +    for key, value in cert.get('subjectAltName', []):
> +        write(' subjectAltName.%s: %s\n' % (key, value))
> +
>   if has_https:
>       class BetterHTTPS(httplib.HTTPSConnection):
>           send = keepalive.safesend
> @@ -528,6 +540,8 @@ if has_https:
>                           ca_certs=cacerts)
>                   msg = _verifycert(self.sock.getpeercert(), self.host)
>                   if msg:
> +                    if self.ui.debugflag:
> +                        _printcert(self.sock.getpeercert(), self.ui.debug)
>                       raise util.Abort(_('%s certificate error: %s') %
>                                        (self.host, msg))
>                   self.ui.debug('%s certificate successfully verified\n' %

Finally: This should be tested in test-https.t.

/Mads



More information about the Mercurial-devel mailing list