[PATCH 4 of 9 V2] sslutil: restore behavior of web.cacerts=! to disable CA loading

Gregory Szorc gregory.szorc at gmail.com
Tue May 31 22:22:00 EDT 2016


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1464639083 25200
#      Mon May 30 13:11:23 2016 -0700
# Node ID 317bb3df2da17ffd258ca80072a6fbb0e36b84a1
# Parent  1b9bbb794da9681fd6639f321a0156c75ece764f
sslutil: restore behavior of web.cacerts=! to disable CA loading

This commit logically reverts ef316c653b7f.

There are various tests for behavior when CA certs aren't loaded.
Previously, we would pass --insecure to disable loading of CA
certs. This has worked up to this point because the error message
for --insecure and no CAs loaded is the same. Upcoming commits will
change the error message for --insecure and will change behavior
when CAs aren't loaded.

This commit restores the ability to disable loading of CA certs
by setting web.cacerts to "!". Unlike the previous implementation
which treated web.cacerts=! and --insecure as equivalent, this version
explicitly disables loading of CA certs when web.cacerts=!, even if
system/default CA certs are available.

Tests have been updated to use the new method. The variable used
to disable CA certs has been renamed because the method is not
OS X specific.

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -107,16 +107,19 @@ except AttributeError:
             return ssl.wrap_socket(socket, **args)
 
 def _hostsettings(ui, hostname):
     """Obtain security settings for a hostname.
 
     Returns a dict of settings relevant to that hostname.
     """
     s = {
+        # Whether we should attempt to load default/available CA certs
+        # if an explicit ``cafile`` is not defined.
+        'allowloaddefaultcerts': True,
         # List of 2-tuple of (hash algorithm, hash).
         'certfingerprints': [],
         # Path to file containing concatenated CA certs. Used by
         # SSLContext.load_verify_locations().
         'cafile': None,
         # Whether certificate verification should be disabled.
         'disablecertverification': False,
         # Whether the legacy [hostfingerprints] section has data for this host.
@@ -157,31 +160,37 @@ def _hostsettings(ui, hostname):
         s['verifymode'] = ssl.CERT_NONE
 
     # Try to hook up CA certificate validation unless something above
     # makes it not necessary.
     if s['verifymode'] is None:
         # Find global certificates file in config.
         cafile = ui.config('web', 'cacerts')
 
-        if cafile:
+        # "!" means to disable loading of CA certificates. This is
+        # intentionally undocumented, as users should not do this:
+        # they should prefer per-host security settings instead.
+        if cafile == '!':
+            cafile = None
+            s['allowloaddefaultcerts'] = False
+        elif cafile:
             cafile = util.expandpath(cafile)
             if not os.path.exists(cafile):
                 raise error.Abort(_('could not find web.cacerts: %s') % cafile)
         else:
             # No global CA certs. See if we can load defaults.
             cafile = _defaultcacerts()
             if cafile:
                 ui.debug('using %s to enable OS X system CA\n' % cafile)
 
         s['cafile'] = cafile
 
         # Require certificate validation if CA certs are being loaded and
         # verification hasn't been disabled above.
-        if cafile or _canloaddefaultcerts:
+        if cafile or (_canloaddefaultcerts and s['allowloaddefaultcerts']):
             s['verifymode'] = ssl.CERT_REQUIRED
         else:
             # At this point we don't have a fingerprint, aren't being
             # explicitly insecure, and can't load CA certs. Connecting
             # at this point is insecure. But we do it for BC reasons.
             # TODO abort here to make secure by default.
             s['verifymode'] = ssl.CERT_NONE
 
@@ -238,20 +247,22 @@ def wrapsocket(sock, keyfile, certfile, 
         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'])
         caloaded = True
-    else:
+    elif settings['allowloaddefaultcerts']:
         # This is a no-op on old Python.
         sslcontext.load_default_certs()
-        caloaded = _canloaddefaultcerts
+        caloaded = True
+    else:
+        caloaded = False
 
     sslsocket = sslcontext.wrap_socket(sock, server_hostname=serverhostname)
     # check if wrap_socket failed silently because socket had been
     # closed
     # - see http://bugs.python.org/issue13721
     if not sslsocket.cipher():
         raise error.Abort(_('ssl connection failed'))
 
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -157,31 +157,30 @@ Test server address cannot be reused
   [255]
 #else
   $ hg serve -p $HGPORT --certificate=$PRIV 2>&1
   abort: cannot start server at ':$HGPORT': Address already in use
   [255]
 #endif
   $ cd ..
 
-OS X has a dummy CA cert that enables use of the system CA store when using
-Apple's OpenSSL. This trick do not work with plain OpenSSL.
+Our test cert is not signed by a trusted CA. It should fail to verify if
+we are able to load CA certs.
 
-  $ DISABLEOSXDUMMYCERT=
 #if defaultcacerts
   $ hg clone https://localhost:$HGPORT/ copy-pull
   abort: error: *certificate verify failed* (glob)
   [255]
+#endif
 
-  $ DISABLEOSXDUMMYCERT="--insecure"
-#endif
+  $ DISABLECACERTS="--config web.cacerts=!"
 
 clone via pull
 
-  $ hg clone https://localhost:$HGPORT/ copy-pull $DISABLEOSXDUMMYCERT
+  $ hg clone https://localhost:$HGPORT/ copy-pull $DISABLECACERTS
   warning: localhost certificate with fingerprint 91:4f:1a:ff:87:24:9c:09:b6:85:9b:88:b1:90:6d:30:75:64:91:ca not verified (check hostsecurity or web.cacerts config setting)
   requesting all changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 4 changes to 4 files
   updating to branch default
   4 files updated, 0 files merged, 0 files removed, 0 files unresolved
@@ -197,17 +196,17 @@ clone via pull
   adding bar
   $ cd ..
 
 pull without cacert
 
   $ cd copy-pull
   $ echo '[hooks]' >> .hg/hgrc
   $ echo "changegroup = printenv.py changegroup" >> .hg/hgrc
-  $ hg pull $DISABLEOSXDUMMYCERT
+  $ hg pull $DISABLECACERTS
   pulling from https://localhost:$HGPORT/
   warning: localhost certificate with fingerprint 91:4f:1a:ff:87:24:9c:09:b6:85:9b:88:b1:90:6d:30:75:64:91:ca not verified (check hostsecurity or web.cacerts config setting)
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
   changegroup hook: HG_NODE=5fed3813f7f5e1824344fdc9cf8f63bb662c292d HG_NODE_LAST=5fed3813f7f5e1824344fdc9cf8f63bb662c292d HG_SOURCE=pull HG_TXNID=TXN:* HG_URL=https://localhost:$HGPORT/ (glob)


More information about the Mercurial-devel mailing list