Patchwork [3,of,4,V2] ssl: set explicit symbol "!" to web.cacerts to disable SSL verification (BC)

login
register
mail settings
Submitter Yuya Nishihara
Date March 12, 2015, 3:41 p.m.
Message ID <aa314d9b7beae9d6bdc9.1426174916@mimosa>
Download mbox | patch
Permalink /patch/8023/
State Accepted
Commit b76d8c641746fd19caf5762ca73c4c24a124ea3e
Headers show

Comments

Yuya Nishihara - March 12, 2015, 3:41 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1425479224 -32400
#      Wed Mar 04 23:27:04 2015 +0900
# Node ID aa314d9b7beae9d6bdc9f939615edf5a18c17144
# Parent  e86c238e627c36dde5ed32a34442d5eacc63a839
ssl: set explicit symbol "!" to web.cacerts to disable SSL verification (BC)

The next patch will enable verification by using the system's CA store if
possible, which means we would have to distinguish None (=use default) from
'' (=--insecure). This smells bug-prone and provides no way to override
web.cacerts to forcibly use the system's store by --config argument.

This patch changes the meaning of web.cacerts as follows:

  value   behavior
  ------- ---------------------------------------
  None/'' use default
  '!'     never use CA certs (set by --insecure)
  <path>  verify by the specified CA certificates

Values other than <path> are for internal use and therefore undocumented.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -826,7 +826,7 @@  def _dispatch(req):
 
     if cmdoptions.get('insecure', False):
         for ui_ in uis:
-            ui_.setconfig('web', 'cacerts', '', '--insecure')
+            ui_.setconfig('web', 'cacerts', '!', '--insecure')
 
     if options['version']:
         return commands.version_(ui)
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -672,7 +672,9 @@  def remoteui(src, opts):
         for key, val in src.configitems(sect):
             dst.setconfig(sect, key, val, 'copied')
     v = src.config('web', 'cacerts')
-    if v:
+    if v == '!':
+        dst.setconfig('web', 'cacerts', v, 'copied')
+    elif v:
         dst.setconfig('web', 'cacerts', util.expandpath(v), 'copied')
 
     return dst
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -134,7 +134,7 @@  def _defaultcacerts():
         dummycert = os.path.join(os.path.dirname(__file__), 'dummycert.pem')
         if os.path.exists(dummycert):
             return dummycert
-    return None
+    return '!'
 
 def sslkwargs(ui, host):
     kws = {}
@@ -142,17 +142,18 @@  def sslkwargs(ui, host):
     if hostfingerprint:
         return kws
     cacerts = ui.config('web', 'cacerts')
-    if cacerts:
+    if cacerts == '!':
+        pass
+    elif cacerts:
         cacerts = util.expandpath(cacerts)
         if not os.path.exists(cacerts):
             raise util.Abort(_('could not find web.cacerts: %s') % cacerts)
-    elif cacerts is None:
-        dummycert = _defaultcacerts()
-        if dummycert:
-            ui.debug('using %s to enable OS X system CA\n' % dummycert)
-            ui.setconfig('web', 'cacerts', dummycert, 'dummy')
-            cacerts = dummycert
-    if cacerts:
+    else:
+        cacerts = _defaultcacerts()
+        if cacerts and cacerts != '!':
+            ui.debug('using %s to enable OS X system CA\n' % cacerts)
+        ui.setconfig('web', 'cacerts', cacerts, 'defaultcacerts')
+    if cacerts != '!':
         kws.update({'ca_certs': cacerts,
                     'cert_reqs': CERT_REQUIRED,
                     })
@@ -201,7 +202,7 @@  class validator(object):
                                  hint=_('check hostfingerprint configuration'))
             self.ui.debug('%s certificate matched fingerprint %s\n' %
                           (host, nicefingerprint))
-        elif cacerts:
+        elif cacerts != '!':
             msg = _verifycert(peercert2, host)
             if msg:
                 raise util.Abort(_('%s certificate error: %s') % (host, msg),
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -323,7 +323,7 @@  def has_ssl():
 @check("defaultcacerts", "can verify SSL certs by system's CA certs store")
 def has_defaultcacerts():
     from mercurial import sslutil
-    return sslutil._defaultcacerts()
+    return sslutil._defaultcacerts() != '!'
 
 @check("windows", "Windows")
 def has_windows():
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -124,7 +124,7 @@  Apple's OpenSSL. This trick do not work 
   abort: error: *certificate verify failed* (glob)
   [255]
 
-  $ DISABLEOSXDUMMYCERT="--config=web.cacerts="
+  $ DISABLEOSXDUMMYCERT="--config=web.cacerts=!"
 #endif
 
 clone via pull
@@ -240,7 +240,7 @@  Fingerprints
   $ echo "127.0.0.1 = 914f1aff87249c09b6859b88b1906d30756491ca" >> copy-pull/.hg/hgrc
 
 - works without cacerts
-  $ hg -R copy-pull id https://localhost:$HGPORT/ --config web.cacerts=
+  $ hg -R copy-pull id https://localhost:$HGPORT/ --config web.cacerts=!
   5fed3813f7f5
 
 - fails when cert doesn't match hostname (port is ignored)