Patchwork [3,of,5] sslutil: don't load default certificates when they aren't relevant

login
register
mail settings
Submitter Gregory Szorc
Date June 30, 2016, 2:51 a.m.
Message ID <a86fc51ac3a21db8126e.1467255081@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15661/
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 1467254304 25200
#      Wed Jun 29 19:38:24 2016 -0700
# Node ID a86fc51ac3a21db8126e1922cb8c29e62d3de7ac
# Parent  2c6829fa01fda373d2829c50b9ceebaa62bbb396
sslutil: don't load default certificates when they aren't relevant

Before, we would call SSLContext.load_default_certs() when
certificate verification wasn't being used. Since
SSLContext.verify_mode == ssl.CERT_NONE, this would ideally
no-op. However, there is a slim chance the loading of system
certs could cause a failure. Furthermore, this behavior
interfered with a future patch that aims to provide a more
helpful error message when we're unable to load CAs.

The lack of test fallout is hopefully a sign that our
security code and tests are in a relatively good state.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -149,21 +149,23 @@  def _hostsettings(ui, hostname):
         fingerprint = fingerprint.replace(':', '').lower()
         s['certfingerprints'].append(('sha1', fingerprint))
         s['legacyfingerprint'] = True
 
     # If a host cert fingerprint is defined, it is the only thing that
     # matters. No need to validate CA certs.
     if s['certfingerprints']:
         s['verifymode'] = ssl.CERT_NONE
+        s['allowloaddefaultcerts'] = False
 
     # If --insecure is used, don't take CAs into consideration.
     elif ui.insecureconnections:
         s['disablecertverification'] = True
         s['verifymode'] = ssl.CERT_NONE
+        s['allowloaddefaultcerts'] = False
 
     if ui.configbool('devel', 'disableloaddefaultcerts'):
         s['allowloaddefaultcerts'] = False
 
     # If both fingerprints and a per-host ca file are specified, issue a warning
     # because users should not be surprised about what security is or isn't
     # being performed.
     cafile = ui.config('hostsecurity', '%s:verifycertsfile' % hostname)