# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 2017-07-11 04:09:46
# Node ID 3bdbbadddecc2fa105cbdfdf28fa1f61073f41b0
# Parent  892d255ec2a1e78ea6726f0109588506140c9e3e

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