Patchwork [1,of,6,V6] sslutil: update comment about create_default_context()

login
register
mail settings
Submitter Gregory Szorc
Date July 16, 2016, 5:18 a.m.
Message ID <8b135fc9edb73748fbc1.1468646324@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15899/
State Superseded
Headers show

Comments

Gregory Szorc - July 16, 2016, 5:18 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468551399 25200
#      Thu Jul 14 19:56:39 2016 -0700
# Node ID 8b135fc9edb73748fbc1329ba63f95a60b8e07a5
# Parent  1b8b6adb2365b4f8674b7c4964be0c2d234cd6a5
sslutil: update comment about create_default_context()

While ssl.create_default_context() creates a SSLContext with
reasonable default options, we can't use it because it conflicts with
our CA loading controls. So replace the comment with reality.

(FWIW the comment was written before the existing CA loading code
was in place.)

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -259,17 +259,23 @@  def wrapsocket(sock, keyfile, certfile, 
       server (and client) support SNI, this tells the server which certificate
       to use.
     """
     if not serverhostname:
         raise error.Abort(_('serverhostname argument is required'))
 
     settings = _hostsettings(ui, serverhostname)
 
-    # TODO use ssl.create_default_context() on modernssl.
+    # We can't use ssl.create_default_context() because it calls
+    # load_default_certs() unless CA arguments are passed to it. We want to
+    # have explicit control over CA loading because implicitly loading
+    # CAs may undermine the user's intent. For example, a user may define a CA
+    # bundle with a specific CA cert removed. If the system/default CA bundle
+    # is loaded and contains that removed CA, you've just undone the user's
+    # choice.
     sslcontext = SSLContext(settings['protocol'])
 
     # This is a no-op unless using modern ssl.
     sslcontext.options |= settings['ctxoptions']
 
     # This still works on our fake SSLContext.
     sslcontext.verify_mode = settings['verifymode']