changeset 33381:3bdbbadddecc

sslutil: check for missing certificate and key files (issue5598)

Currently, sslutil._hostsettings() performs validation that web.cacerts
exists. However, client certificates are passed in to the function
and not all callers may validate them. This includes
httpconnection.readauthforuri(), which loads the [auth] section.

If a missing file is specified, the ssl module will raise a generic
IOException. And, it doesn't even give us the courtesy of telling
us which file is missing! Mercurial then prints a generic
"abort: No such file or directory" (or similar) error, leaving users
to scratch their head as to what file is missing.

This commit introduces explicit validation of all paths passed as
arguments to wrapsocket() and wrapserversocket(). Any missing file
is alerted about explicitly.

We should probably catch missing files earlier - as part of loading
the [auth] section. However, I think the sslutil functions should
check for file presence regardless of what callers do because that's
the only way to be sure that missing files are always detected.

--- a/mercurial/sslutil.py	Fri Jul 07 08:55:12 2017 -0700
+++ b/mercurial/sslutil.py	Mon Jul 10 21:09:46 2017 -0700
@@ -343,6 +343,13 @@
     if not serverhostname:
         raise error.Abort(_('serverhostname argument is required'))
 
+    for f in (keyfile, certfile):
+        if f and not os.path.exists(f):
+            raise error.Abort(_('certificate file (%s) does not exist; '
+                                'cannot connect to %s') % (f, serverhostname),
+                              hint=_('restore missing file or fix references '
+                                     'in Mercurial config'))
+
     settings = _hostsettings(ui, serverhostname)
 
     # We can't use ssl.create_default_context() because it calls
@@ -499,6 +506,13 @@
 
     Typically ``cafile`` is only defined if ``requireclientcert`` is true.
     """
+    # This function is not used much by core Mercurial, so the error messaging
+    # doesn't have to be as detailed as for wrapsocket().
+    for f in (certfile, keyfile, cafile):
+        if f and not os.path.exists(f):
+            raise error.Abort(_('referenced certificate file (%s) does not '
+                                'exist') % f)
+
     protocol, options, _protocolui = protocolsettings('tls1.0')
 
     # This config option is intended for use in tests only. It is a giant

--- a/tests/test-https.t	Fri Jul 07 08:55:12 2017 -0700
+++ b/tests/test-https.t	Mon Jul 10 21:09:46 2017 -0700
@@ -592,9 +592,22 @@
 
 #if sslcontext
 
+  $ cd test
+
+Missing certificate file(s) are detected
+
+  $ hg serve -p $HGPORT --certificate=/missing/certificate \
+  > --config devel.servercafile=$PRIV --config devel.serverrequirecert=true
+  abort: referenced certificate file (/missing/certificate) does not exist
+  [255]
+
+  $ hg serve -p $HGPORT --certificate=$PRIV \
+  > --config devel.servercafile=/missing/cafile --config devel.serverrequirecert=true
+  abort: referenced certificate file (/missing/cafile) does not exist
+  [255]
+
 Start hgweb that requires client certificates:
 
-  $ cd test
   $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
   > --config devel.servercafile=$PRIV --config devel.serverrequirecert=true
   $ cat ../hg0.pid >> $DAEMON_PIDS
@@ -631,4 +644,16 @@
   abort: error: * (glob)
   [255]
 
+Missing certficate and key files result in error
+
+  $ hg id https://localhost:$HGPORT/ --config auth.l.cert=/missing/cert
+  abort: certificate file (/missing/cert) does not exist; cannot connect to localhost
+  (restore missing file or fix references in Mercurial config)
+  [255]
+
+  $ hg id https://localhost:$HGPORT/ --config auth.l.key=/missing/key
+  abort: certificate file (/missing/key) does not exist; cannot connect to localhost
+  (restore missing file or fix references in Mercurial config)
+  [255]
+
 #endif