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

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1499746186 25200
#      Mon Jul 10 21:09:46 2017 -0700
# Node ID b9e2be8b5a75cbd1adef810111a469a10c9846c5
# Parent  4672db164c986da4442bd864cd044512d975c3f2
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.

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -343,6 +343,13 @@ def wrapsocket(sock, keyfile, certfile, 
     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 @@ def wrapserversocket(sock, ui, certfile=
 
     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
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -592,9 +592,22 @@ Test https with cert problems through pr
 
 #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 @@ with client certificate:
   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