Patchwork [2,of,5] sslutil: display a better error message when CA file loading fails

login
register
mail settings
Submitter Gregory Szorc
Date June 30, 2016, 2:51 a.m.
Message ID <2c6829fa01fda373d282.1467255080@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15660/
State Accepted
Headers show

Comments

Gregory Szorc - June 30, 2016, 2:51 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1467254258 25200
#      Wed Jun 29 19:37:38 2016 -0700
# Node ID 2c6829fa01fda373d2829c50b9ceebaa62bbb396
# Parent  a5af4aa0c5f18af545f6a7873fb0f7e0e863ec30
sslutil: display a better error message when CA file loading fails

Before, sslcontext.load_verify_locations() would raise a
ssl.SSLError which would be caught further up the stack and converted
to a urlerror. By that time, we lost track of what actually errored.

Trapping the error here gives users a slightly more actionable error
message.

The behavior between Python <2.7.9 and Python 2.7.9+ differs. This
is because our fake SSLContext class installed on <2.7.9 doesn't
actually do anything during load_verify_locations: it defers actions
until wrap_socket() time. Unfortunately, a number of errors can occur
at wrap_socket() time and we're unable to ascertain what the root
cause is. But that shouldn't stop us from providing better error
messages to people running a modern and secure Python version.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -263,17 +263,22 @@  def wrapsocket(sock, keyfile, certfile, 
 
     if certfile is not None:
         def password():
             f = keyfile or certfile
             return ui.getpass(_('passphrase for %s: ') % f, '')
         sslcontext.load_cert_chain(certfile, keyfile, password)
 
     if settings['cafile'] is not None:
-        sslcontext.load_verify_locations(cafile=settings['cafile'])
+        try:
+            sslcontext.load_verify_locations(cafile=settings['cafile'])
+        except ssl.SSLError as e:
+            raise error.Abort(_('error loading CA file %s: %s') % (
+                              settings['cafile'], e.args[1]),
+                              hint=_('file is empty or malformed?'))
         caloaded = True
     elif settings['allowloaddefaultcerts']:
         # This is a no-op on old Python.
         sslcontext.load_default_certs()
         caloaded = True
     else:
         caloaded = False
 
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -57,19 +57,26 @@  Specifying a per-host certificate file t
 
   $ hg --config hostsecurity.localhost:verifycertsfile=/does/not/exist clone https://localhost:$HGPORT/
   abort: path specified by hostsecurity.localhost:verifycertsfile does not exist: /does/not/exist
   [255]
 
 A malformed per-host certificate file will raise an error
 
   $ echo baddata > badca.pem
+#if sslcontext
+  $ hg --config hostsecurity.localhost:verifycertsfile=badca.pem clone https://localhost:$HGPORT/
+  abort: error loading CA file badca.pem: * (glob)
+  (file is empty or malformed?)
+  [255]
+#else
   $ hg --config hostsecurity.localhost:verifycertsfile=badca.pem clone https://localhost:$HGPORT/
   abort: error: * (glob)
   [255]
+#endif
 
 A per-host certificate mismatching the server will fail verification
 
   $ hg --config hostsecurity.localhost:verifycertsfile="$CERTSDIR/client-cert.pem" clone https://localhost:$HGPORT/
   abort: error: *certificate verify failed* (glob)
   [255]
 
 A per-host certificate matching the server's cert will be accepted
@@ -178,20 +185,29 @@  variables in the filename
   pulling from https://localhost:$HGPORT/
   warning: connection security to localhost is disabled per current settings; communication is susceptible to eavesdropping and tampering
   searching for changes
   no changes found
 
 empty cacert file
 
   $ touch emptycafile
+
+#if sslcontext
+  $ hg --config web.cacerts=emptycafile -R copy-pull pull
+  pulling from https://localhost:$HGPORT/
+  abort: error loading CA file emptycafile: * (glob)
+  (file is empty or malformed?)
+  [255]
+#else
   $ hg --config web.cacerts=emptycafile -R copy-pull pull
   pulling from https://localhost:$HGPORT/
   abort: error: * (glob)
   [255]
+#endif
 
 cacert mismatch
 
   $ hg -R copy-pull pull --config web.cacerts="$CERTSDIR/pub.pem" \
   > https://127.0.0.1:$HGPORT/
   pulling from https://127.0.0.1:$HGPORT/
   abort: 127.0.0.1 certificate error: certificate is for localhost
   (set hostsecurity.127.0.0.1:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30 config setting or use --insecure to connect insecurely)