Patchwork [05,of,11,V4] sslutil: implement wrapserversocket()

login
register
mail settings
Submitter Gregory Szorc
Date July 15, 2016, 4:09 a.m.
Message ID <e0c8977678cca5a1ceb8.1468555746@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15869/
State Superseded
Headers show

Comments

Gregory Szorc - July 15, 2016, 4:09 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468552459 25200
#      Thu Jul 14 20:14:19 2016 -0700
# Node ID e0c8977678cca5a1ceb8174ecd347810482f6656
# Parent  99d9188b9d45039a06c65fa7dda36a12d08369c2
sslutil: implement wrapserversocket()

wrapsocket() is heavily tailored towards client use. In preparation
for converting the built-in server to use sslutil (as opposed to
the ssl module directly), we add wrapserversocket() for wrapping
a socket to be used on servers.

Again, we can't use ssl.create_default_context() because of CA control
concerns. So we basically implement it inline.
Yuya Nishihara - July 15, 2016, 1:09 p.m.
On Thu, 14 Jul 2016 21:09:06 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1468552459 25200
> #      Thu Jul 14 20:14:19 2016 -0700
> # Node ID e0c8977678cca5a1ceb8174ecd347810482f6656
> # Parent  99d9188b9d45039a06c65fa7dda36a12d08369c2
> sslutil: implement wrapserversocket()
> 
> wrapsocket() is heavily tailored towards client use. In preparation
> for converting the built-in server to use sslutil (as opposed to
> the ssl module directly), we add wrapserversocket() for wrapping
> a socket to be used on servers.
> 
> Again, we can't use ssl.create_default_context() because of CA control
> concerns. So we basically implement it inline.

I think create_default_context() can be used here because the purpose is
CLIENT_AUTH, which doesn't set CERT_REQUIRED, and the system CA certs are
not loaded automatically.
Gregory Szorc - July 15, 2016, 5:01 p.m.
On Fri, Jul 15, 2016 at 6:09 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Thu, 14 Jul 2016 21:09:06 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1468552459 25200
> > #      Thu Jul 14 20:14:19 2016 -0700
> > # Node ID e0c8977678cca5a1ceb8174ecd347810482f6656
> > # Parent  99d9188b9d45039a06c65fa7dda36a12d08369c2
> > sslutil: implement wrapserversocket()
> >
> > wrapsocket() is heavily tailored towards client use. In preparation
> > for converting the built-in server to use sslutil (as opposed to
> > the ssl module directly), we add wrapserversocket() for wrapping
> > a socket to be used on servers.
> >
> > Again, we can't use ssl.create_default_context() because of CA control
> > concerns. So we basically implement it inline.
>
> I think create_default_context() can be used here because the purpose is
> CLIENT_AUTH, which doesn't set CERT_REQUIRED, and the system CA certs are
> not loaded automatically.
>

You are correct: create_default_context() is usable for server sockets.
I'll send a V5 series.
Gregory Szorc - July 15, 2016, 5:07 p.m.
On Fri, Jul 15, 2016 at 10:01 AM, Gregory Szorc <gregory.szorc@gmail.com>
wrote:

> On Fri, Jul 15, 2016 at 6:09 AM, Yuya Nishihara <yuya@tcha.org> wrote:
>
>> On Thu, 14 Jul 2016 21:09:06 -0700, Gregory Szorc wrote:
>> > # HG changeset patch
>> > # User Gregory Szorc <gregory.szorc@gmail.com>
>> > # Date 1468552459 25200
>> > #      Thu Jul 14 20:14:19 2016 -0700
>> > # Node ID e0c8977678cca5a1ceb8174ecd347810482f6656
>> > # Parent  99d9188b9d45039a06c65fa7dda36a12d08369c2
>> > sslutil: implement wrapserversocket()
>> >
>> > wrapsocket() is heavily tailored towards client use. In preparation
>> > for converting the built-in server to use sslutil (as opposed to
>> > the ssl module directly), we add wrapserversocket() for wrapping
>> > a socket to be used on servers.
>> >
>> > Again, we can't use ssl.create_default_context() because of CA control
>> > concerns. So we basically implement it inline.
>>
>> I think create_default_context() can be used here because the purpose is
>> CLIENT_AUTH, which doesn't set CERT_REQUIRED, and the system CA certs are
>> not loaded automatically.
>>
>
> You are correct: create_default_context() is usable for server sockets.
> I'll send a V5 series.
>

... except in a subsequent patch where we need to configure an explicit
protocol version we can't use create_default_context(). At that point we
have 3 code paths for creating the context - 2 for modern ssl and 1 for
legacy. I'm tempted to leave this implementation for now and follow-up to
use create_default_context(). But I can tweak the comment. So I'll still
send a V5.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -330,16 +330,63 @@  def wrapsocket(sock, keyfile, certfile, 
         'caloaded': caloaded,
         'hostname': serverhostname,
         'settings': settings,
         'ui': ui,
     }
 
     return sslsocket
 
+def wrapserversocket(sock, ui, certfile=None, keyfile=None, cafile=None,
+                     requireclientcert=False):
+    """Wrap a socket for use by servers.
+
+    ``certfile`` and ``keyfile`` specify the files containing the certificate's
+    public and private keys, respectively. Both keys can be defined in the same
+    file via ``certfile`` (the private key must come first in the file).
+
+    ``cafile`` defines the path to certificate authorities.
+
+    ``requireclientcert`` specifies whether to require client certificates.
+
+    Typically ``cafile`` is only defined if ``requireclientcert`` is true.
+    """
+    if modernssl:
+        # We can't use ssl.create_default_context() for reasons described
+        # above. Do most of what it does inline.
+        sslcontext = SSLContext(ssl.PROTOCOL_SSLv23)
+        # SSLv2 and SSLv3 are broken. Ban them outright.
+        sslcontext.options |= OP_NO_SSLv2 | OP_NO_SSLv3
+        # Prevent CRIME
+        sslcontext.options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
+        # Improve forward secrecy.
+        sslcontext.options |= getattr(ssl, 'OP_SINGLE_DH_USE', 0)
+        sslcontext.options |= getattr(ssl, 'OP_SINGLE_ECDH_USE', 0)
+
+        # The the list of more secure ciphers if found in the ssl module.
+        if util.safehasattr(ssl, '_RESTRICTED_SERVER_CIPHERS'):
+            sslcontext.options |= getattr(ssl, 'OP_CIPHER_SERVER_PREFERENCE', 0)
+            sslcontext.set_ciphers(ssl._RESTRICTED_SERVER_CIPHERS)
+    else:
+        sslcontext = SSLContext(ssl.PROTOCOL_TLSv1)
+        # Legacy ssl module can't do much :/
+
+    if requireclientcert:
+        sslcontext.verify_mode = ssl.CERT_REQUIRED
+    else:
+        sslcontext.verify_mode = ssl.CERT_NONE
+
+    if certfile or keyfile:
+        sslcontext.load_cert_chain(certfile=certfile, keyfile=keyfile)
+
+    if cafile:
+        sslcontext.load_verify_locations(cafile=cafile)
+
+    return sslcontext.wrap_socket(sock, server_side=True)
+
 class wildcarderror(Exception):
     """Represents an error parsing wildcards in DNS name."""
 
 def _dnsnamematch(dn, hostname, maxwildcards=1):
     """Match DNS names according RFC 6125 section 6.4.3.
 
     This code is effectively copied from CPython's ssl._dnsname_match.