[PATCH 1 of 1 RFC] url: debug print ssl certificate info if verify failed
Yuya Nishihara
yuya at tcha.org
Sun Jan 9 12:37:13 CST 2011
Mads Kiilerich wrote:
> [ starting with reply to other sub-thread: ]
>
> Yuya Nishihara wrote, On 01/09/2011 06:52 AM:
> > Mads Kiilerich wrote:
> >> With this patch it is possible to show much of the server certificate
> >> info. Cool. But in which cases can that be used? Can you describe some
> >> use cases? What do the user see and how do he understand it and what
> >> will he then do?
> > We could ask a user to attach --debug output on bug report.
> > Currently it needs extra effort to get to know why Mercurial complains
> > about certificate.
>
> But how will this help debugging what the problem is? I don't think the
> readable information python/openssl makes available is of any use. I
> would like to see a case where this information helps.
Hmm, I agree that the provided information is cheap.
> I think the best way for most users to debug certificate issues is to
>
> 1. browse the site with a browser and make sure it works there
> 2. investigate the certificate in the browser and verify the root/CA
> certificate is in cacerts
> (http://mercurial.selenic.com/wiki/CACertificates#Self-signed_certificates )
> 3. ?
> 4. success
>
> BUT I think it in some cases could be helpful to show the PEM encoded
> certificate we receive from the https server. Users with skills and
> motivation could then decode and analyze it with stronger dedicated
> tools. I think it would be better to avoid doing too much extra work and
> extra network connections in the except clause, so why not get the
> server certificate and show it before we make the "real" connection
> using cacerts? I am not sure if it should require --debug or if
> --verbose should be enough.
>
> >> (If the subject is the same as the issuer then the certificate is
> >> self-signed and the certificate from get_server_certificate can thus
> >> probably be used in cacerts. That case can however also just be detected
> >> with two consecutive calls to get_server_certificate.)
> > like this? This seems more solid than poking internal _ssl module.
> >
> > f.write(ssl.get_server_certificate(addr))
> > f.close()
> > ssl.get_server_certificate(ssl.get_server_certificate(addr),
> > ca_certs=f.name)
>
> Almost. More like:
>
> open(tmpfn, 'w').write(ssl.get_server_certificate(addr))
> try:
> cert = ssl.get_server_certificate(addr, ca_certs=tmpfn)
> print '%s cert is self-signed and could perhaps be used in cacerts:'
> print cert
> except:
> print 'bad luck'
>
> That will however only work in some cases. A general solution would be
> far better. I am not sure how much we should try to handle special
> cases. That might seem inconsistent and unpredictable.
>
>
> Yuya Nishihara wrote, On 01/09/2011 04:12 PM:
> > # HG changeset patch
> > # User Yuya Nishihara<yuya at tcha.org>
> > # Date 1294583184 -32400
> > # Node ID 94b670827fba25c056643015f8c7b3c1f52df0f5
> > # Parent 0c554a73798b08106def4328d68a2053769b5c3e
> > url: debug print ssl certificate info if verify failed
> >
> > If a server provides certificate that can't be verified by the configured
> > cacerts, ssl.wrap_socket raises exception like
> > "routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed".
> >
> > This patch tries to show data about unverified certificate, so that a user
> > can know detailed circumstances.
> >
> > As of Python 2.6 or 2.7, it doesn't officially provide a way to decode
> > PEM-encoded certificate to human-readable text.
> > This patch uses _ssl._test_decode_cert(), which is available at least on
> > CPython 2.6.6 or 2.7.1.
> >
> > diff --git a/mercurial/url.py b/mercurial/url.py
> > --- a/mercurial/url.py
> > +++ b/mercurial/url.py
> > @@ -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, tempfile
> > import __builtin__
> > from i18n import _
> > import keepalive, util
> > @@ -308,8 +308,11 @@ if has_https:
> > import ssl
> > _ssl_wrap_socket = ssl.wrap_socket
> > CERT_REQUIRED = ssl.CERT_REQUIRED
> > + SSLError = ssl.SSLError
> > except ImportError:
> > CERT_REQUIRED = 2
> > + class SSLError(Exception):
> > + pass
> >
> > def _ssl_wrap_socket(sock, key_file, cert_file,
> > cert_reqs=CERT_REQUIRED, ca_certs=None):
> > @@ -527,6 +530,71 @@ def _verifycert(cert, hostname):
> > return _('certificate is for %s') % certname
> > return _('no commonName found in certificate')
> >
> > +def _formatcert(cert):
> > + """Return formatted text for the given certificate info
> > +
> > +>>> cert = {'issuer': ((('commonName', u'localhost'),),
> > + ... (('emailAddress', u'hg at localhost'),)),
> > + ... 'notAfter': 'Aug 31 12:50:48 2035 GMT',
> > + ... 'notBefore': 'Jan 9 12:50:48 2011 GMT',
> > + ... 'subject': ((('commonName', u'localhost'),),
> > + ... (('emailAddress', u'hg at localhost'),))}
>
> Add a subjectAltName too.
>
> > +>>> _formatcert(cert) # doctest: +NORMALIZE_WHITESPACE
> > + 'ssl certificate:\\n
> > + notBefore: Jan 9 12:50:48 2011 GMT\\n
> > + notAfter: Aug 31 12:50:48 2035 GMT\\n
> > + issuer.commonName: localhost\\n
> > + issuer.emailAddress: hg at localhost\\n
> > + subject.commonName: localhost\\n
> > + subject.emailAddress: hg at localhost\\n'
> > + """
> > + l = []
> > + l.append('ssl certificate:')
>
> Isn't it more like 'ssl certificate information received from
> hostname:port:'?
>
> And remember localization markup of that line.
Does it need for a debug output?
> But perhaps this function just should format the body and leave the
> headline to the caller.
Yes, that's better.
> > + for k in ('notBefore', 'notAfter'):
> > + if k in cert:
> > + l.append(' %s: %s' % (k, cert[k]))
> > + for k in ('issuer', 'subject'):
> > + for s in cert.get(k, []):
> > + key, value = s[0]
> > + l.append(' %s.%s: %s'
> > + % (k, key, value.encode('ascii', 'replace')))
> > + for key, value in cert.get('subjectAltName', []):
> > + l.append(' subjectAltName.%s: %s' % (key, value))
> > + return '\n'.join(l) + '\n'
> > +
> > +def _decodecert(pem):
> > + """Decode PEM-encoded string to a dict like a result of getpeercert()"""
> > + try: # needs CPython 2.6 or 2.7
> > + import _ssl
> > + decode_certfile = _ssl._test_decode_cert
> > + except (ImportError, AttributeError):
> > + return
> > + if not pem:
> > + return
> > + fh, fn = tempfile.mkstemp(prefix='hg-servercert-')
> > + try:
> > + f = os.fdopen(fh, 'w')
> > + f.write(pem)
> > + f.close()
> > + return decode_certfile(fn)
> > + finally:
> > + os.unlink(fn)
> > +
> > +def _debugservercert(ui, addr):
> > + if not ui.debugflag:
> > + return
> > + try:
> > + import ssl
> > + except ImportError:
> > + return
> > + try:
> > + pem = ssl.get_server_certificate(addr)
>
> Watch out for demandimport side effects. It will delay the ssl import
> (and failure) to this point.
Oops, I didn't know it. Thanks.
> > + except SSLError:
> > + return
> > + cert = _decodecert(pem)
> > + if cert:
> > + ui.debug(_formatcert(cert))
> > +
> > if has_https:
> > class BetterHTTPS(httplib.HTTPSConnection):
> > send = keepalive.safesend
> > @@ -541,9 +609,13 @@ if has_https:
> >
> > if cacerts:
> > sock = _create_connection((self.host, self.port))
> > - self.sock = _ssl_wrap_socket(sock, self.key_file,
> > - self.cert_file, cert_reqs=CERT_REQUIRED,
> > - ca_certs=cacerts)
> > + try:
> > + self.sock = _ssl_wrap_socket(sock, self.key_file,
> > + self.cert_file, cert_reqs=CERT_REQUIRED,
> > + ca_certs=cacerts)
> > + except SSLError:
> > + _debugservercert(self.ui, (self.host, self.port))
> > + raise
> > msg = _verifycert(self.sock.getpeercert(), self.host)
> > if msg:
> > raise util.Abort(_('%s certificate error: %s') %
> > diff --git a/tests/test-https.t b/tests/test-https.t
> > --- a/tests/test-https.t
> > +++ b/tests/test-https.t
> > @@ -188,3 +188,18 @@ Test server cert which no longer is vali
> > $ hg -R copy-pull pull --config web.cacerts=pub-expired.pem https://localhost:$HGPORT2/
> > abort: error: *:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed (glob)
> > [255]
> > +
> > +debug output on certificate verify failed
> > +
> > + $ hg -R copy-pull --debug pull --config web.cacerts=pub-other.pem
> > + using https://localhost:$HGPORT/
> > + sending between command
> > + ssl certificate:
> > + notBefore: Oct 14 20:30:14 2010 GMT
> > + notAfter: Jun 5 20:30:14 2035 GMT
> > + issuer.commonName: localhost
> > + issuer.emailAddress: hg at localhost
> > + subject.commonName: localhost
> > + subject.emailAddress: hg at localhost
> > + abort: error: *:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed (glob)
> > + [255]
>
> Thanks for the example.
>
> As mentioned in the other mail I doubt this information will help much.
> I don't think this is a good idea, considering the size of the code and
> how ugly it has to be.
Okay, it isn't useful worth maintaining such ugly code.
Thanks for reviewing it.
Yuya,
More information about the Mercurial-devel
mailing list