Mercurial Mercurial > mercurial / changeset
summary | shortlog | changelog | graph | tags | bookmarks | branches | files | changeset | raw | help
Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
sslutil: move and change warning when cert verification is disabled
authorGregory Szorc <gregory.szorc@gmail.com>
Mon, 30 May 2016 13:15:53 -0700
changeset 29289 3536673a25ae
parent 29288 7dee15dee53c
child 29290 01248c37a68e
sslutil: move and change warning when cert verification is disabled A short time ago, validatesocket() didn't know the reasons why cert verification was disabled. Multiple code paths could lead to cert verification being disabled. e.g. --insecure and lack of loaded CAs. With the recent refactorings to sslutil.py, we now know the reasons behind security settings. This means we can recognize when the user requested security be disabled (as opposed to being unable to provide certificate verification due to lack of CAs). This patch moves the check for certificate verification being disabled and changes the wording to distinguish it from other states. The warning message is purposefully more dangerous sounding in order to help discourage people from disabling security outright. We may want to add a URL or hint to this message. I'm going to wait until additional changes to security defaults before committing to something.
mercurial/sslutil.py file | annotate | diff | comparison | revisions
tests/test-https.t file | annotate | diff | comparison | revisions 
--- a/mercurial/sslutil.py	Wed Jun 01 19:57:20 2016 -0700
+++ b/mercurial/sslutil.py	Mon May 30 13:15:53 2016 -0700
@@ -354,6 +354,18 @@
         raise error.Abort(_('%s certificate error: '
                            'no certificate received') % host)
 
+    if settings['disablecertverification']:
+        # We don't print the certificate fingerprint because it shouldn't
+        # be necessary: if the user requested certificate verification be
+        # disabled, they presumably already saw a message about the inability
+        # to verify the certificate and this message would have printed the
+        # fingerprint. So printing the fingerprint here adds little to no
+        # value.
+        ui.warn(_('warning: connection security to %s is disabled per current '
+                  'settings; communication is susceptible to eavesdropping '
+                  'and tampering\n') % host)
+        return
+
     # If a certificate fingerprint is pinned, use it and only it to
     # validate the remote cert.
     peerfingerprints = {
@@ -383,19 +395,6 @@
                  (host, nicefingerprint))
         return
 
-    # If insecure connections were explicitly requested, print a warning
-    # and do no verification.
-    #
-    # It may seem odd that this is checked *after* host fingerprint pinning.
-    # This is for backwards compatibility (for now). The message is also
-    # the same as below for BC.
-    if settings['disablecertverification']:
-        ui.warn(_('warning: %s certificate with fingerprint %s not '
-                  'verified (check %s or web.cacerts '
-                  'config setting)\n') %
-                (host, nicefingerprint, section))
-        return
-
     if not sock._hgstate['caloaded']:
         ui.warn(_('warning: %s certificate with fingerprint %s '
                   'not verified (check %s or web.cacerts config '
--- a/tests/test-https.t	Wed Jun 01 19:57:20 2016 -0700
+++ b/tests/test-https.t	Mon May 30 13:15:53 2016 -0700
@@ -235,7 +235,7 @@
   no changes found
   $ P=`pwd` hg -R copy-pull pull --insecure
   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)
+  warning: connection security to localhost is disabled per current settings; communication is susceptible to eavesdropping and tampering
   searching for changes
   no changes found
 
@@ -248,7 +248,7 @@
   [255]
   $ hg -R copy-pull pull --config web.cacerts=pub.pem https://127.0.0.1:$HGPORT/ --insecure
   pulling from https://127.0.0.1:$HGPORT/
-  warning: 127.0.0.1 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)
+  warning: connection security to 127.0.0.1 is disabled per current settings; communication is susceptible to eavesdropping and tampering
   searching for changes
   no changes found
   $ hg -R copy-pull pull --config web.cacerts=pub-other.pem
@@ -257,7 +257,7 @@
   [255]
   $ hg -R copy-pull pull --config web.cacerts=pub-other.pem --insecure
   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)
+  warning: connection security to localhost is disabled per current settings; communication is susceptible to eavesdropping and tampering
   searching for changes
   no changes found
 
@@ -347,7 +347,7 @@
 
   $ http_proxy=http://localhost:$HGPORT1/ hg -R copy-pull pull --insecure --traceback
   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)
+  warning: connection security to localhost is disabled per current settings; communication is susceptible to eavesdropping and tampering
   searching for changes
   no changes found
mercurial