Patchwork [V2] sslutil - add tls 1.3 support - done during IETF101 Hackathon

login
register
mail settings
Submitter Codarren Velvindron
Date March 26, 2018, 9:43 a.m.
Message ID <20180326094359.GA2575@hackers.mu>
Download mbox | patch
Permalink /patch/29855/
State Superseded
Headers show

Comments

Codarren Velvindron - March 26, 2018, 9:43 a.m.
# HG changeset patch
# User Codarren Velvindron <codarren@hackers.mu>
# Date 1522053522 -14400
#      Mon Mar 26 12:38:42 2018 +0400
# Node ID e11770b0adde6283965cafff1d6214a048417bfe
# Parent  6715e8035b4ff9379a80f5413a4e9148114ab256
sslutil: add tls 1.3 support
Done during the IETF101 Hackathon.
Yuya Nishihara - March 26, 2018, 1:18 p.m.
On Mon, 26 Mar 2018 13:43:59 +0400, Codarren Velvindron wrote:
> # HG changeset patch
> # User Codarren Velvindron <codarren@hackers.mu>
> # Date 1522053522 -14400
> #      Mon Mar 26 12:38:42 2018 +0400
> # Node ID e11770b0adde6283965cafff1d6214a048417bfe
> # Parent  6715e8035b4ff9379a80f5413a4e9148114ab256
> sslutil: add tls 1.3 support

[...]

> -#if sslcontext tls1.2
> +#if sslcontext tls1.3
>  Start servers running supported TLS versions
>  
>    $ cd test
> @@ -456,6 +457,9 @@
>    $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid --certificate=$PRIV \
>    > --config devel.serverexactprotocol=tls1.2
>    $ cat ../hg2.pid >> $DAEMON_PIDS
> +  $ hg serve -p $HGPORT3 -d --pid-file=../hg3.pid --certificate=$PRIV \
> +  > --config devel.serverexactprotocol=tls1.3
> +  $ cat ../hg3.pid >> $DAEMON_PIDS
>    $ cd ..
>  
>  Clients talking same TLS versions work
> @@ -466,6 +470,8 @@
>    5fed3813f7f5
>    $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT2/
>    5fed3813f7f5
> +  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.3 id https://localhost:$HGPORT3/
> +  5fed3813f7f5

Can you narrow the scope of '#if tls1.3'? It requires very recent versions
of Python.

Other than that, the patch looks good to me. Thanks.
Codarren Velvindron - March 29, 2018, 3:51 p.m.
Hello Yuya,

As usual, thank you for your review.

Could you elaborate on below statement please?

"Can you narrow the scope of '#if tls1.3'? It requires very recent versions
of Python."

Best Regards,
Codarren

On Mon, Mar 26, 2018 at 5:18 PM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Mon, 26 Mar 2018 13:43:59 +0400, Codarren Velvindron wrote:
> > # HG changeset patch
> > # User Codarren Velvindron <codarren@hackers.mu>
> > # Date 1522053522 -14400
> > #      Mon Mar 26 12:38:42 2018 +0400
> > # Node ID e11770b0adde6283965cafff1d6214a048417bfe
> > # Parent  6715e8035b4ff9379a80f5413a4e9148114ab256
> > sslutil: add tls 1.3 support
>
> [...]
>
> > -#if sslcontext tls1.2
> > +#if sslcontext tls1.3
> >  Start servers running supported TLS versions
> >
> >    $ cd test
> > @@ -456,6 +457,9 @@
> >    $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid --certificate=$PRIV \
> >    > --config devel.serverexactprotocol=tls1.2
> >    $ cat ../hg2.pid >> $DAEMON_PIDS
> > +  $ hg serve -p $HGPORT3 -d --pid-file=../hg3.pid --certificate=$PRIV \
> > +  > --config devel.serverexactprotocol=tls1.3
> > +  $ cat ../hg3.pid >> $DAEMON_PIDS
> >    $ cd ..
> >
> >  Clients talking same TLS versions work
> > @@ -466,6 +470,8 @@
> >    5fed3813f7f5
> >    $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id
> https://localhost:$HGPORT2/
> >    5fed3813f7f5
> > +  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.3 id
> https://localhost:$HGPORT3/
> > +  5fed3813f7f5
>
> Can you narrow the scope of '#if tls1.3'? It requires very recent versions
> of Python.
>
> Other than that, the patch looks good to me. Thanks.
>
Matt Harbison - March 30, 2018, 3:12 a.m.
On Thu, 29 Mar 2018 11:51:20 -0400, Codarren Velvindron  
<codarren@hackers.mu> wrote:

> Hello Yuya,
>
> As usual, thank you for your review.
>
> Could you elaborate on below statement please?
>
> "Can you narrow the scope of '#if tls1.3'? It requires very recent  
> versions
> of Python."
>
> Best Regards,
> Codarren
>
> On Mon, Mar 26, 2018 at 5:18 PM, Yuya Nishihara <yuya@tcha.org> wrote:
>
>> On Mon, 26 Mar 2018 13:43:59 +0400, Codarren Velvindron wrote:
>> > # HG changeset patch
>> > # User Codarren Velvindron <codarren@hackers.mu>
>> > # Date 1522053522 -14400
>> > #      Mon Mar 26 12:38:42 2018 +0400
>> > # Node ID e11770b0adde6283965cafff1d6214a048417bfe
>> > # Parent  6715e8035b4ff9379a80f5413a4e9148114ab256
>> > sslutil: add tls 1.3 support
>>
>> [...]
>>
>> > -#if sslcontext tls1.2
>> > +#if sslcontext tls1.3

By changing the conditional here to 1.3, you are requiring support for 1.3  
in order to run the 1.2 tests below.  So instead of changing this line,  
just use #endif after the 1.2 test, and open a 1.3 conditional around the  
1.3 tests.  (Sadly, nested #if isn't supported.)

>> >  Start servers running supported TLS versions
>> >
>> >    $ cd test
>> > @@ -456,6 +457,9 @@
>> >    $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid  
>> --certificate=$PRIV \
>> >    > --config devel.serverexactprotocol=tls1.2
>> >    $ cat ../hg2.pid >> $DAEMON_PIDS
>> > +  $ hg serve -p $HGPORT3 -d --pid-file=../hg3.pid  
>> --certificate=$PRIV \
>> > +  > --config devel.serverexactprotocol=tls1.3
>> > +  $ cat ../hg3.pid >> $DAEMON_PIDS
>> >    $ cd ..
>> >
>> >  Clients talking same TLS versions work
>> > @@ -466,6 +470,8 @@
>> >    5fed3813f7f5
>> >    $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id
>> https://localhost:$HGPORT2/
>> >    5fed3813f7f5
>> > +  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.3 id
>> https://localhost:$HGPORT3/
>> > +  5fed3813f7f5
>>
>> Can you narrow the scope of '#if tls1.3'? It requires very recent  
>> versions
>> of Python.
>>
>> Other than that, the patch looks good to me. Thanks.

Patch

diff -r 6715e8035b4f -r e11770b0adde mercurial/help/config.txt
--- a/mercurial/help/config.txt	Sun Mar 25 11:58:05 2018 +0900
+++ b/mercurial/help/config.txt	Mon Mar 26 12:38:42 2018 +0400
@@ -1163,7 +1163,7 @@ 
     By default, the highest version of TLS supported by both client and server
     is used.
 
-    Allowed values are: ``tls1.0``, ``tls1.1``, ``tls1.2``.
+    Allowed values are: ``tls1.0``, ``tls1.1``, ``tls1.2``. ``tls1.3``.
 
     When running on an old Python version, only ``tls1.0`` is allowed since
     old versions of Python only support up to TLS 1.0.
diff -r 6715e8035b4f -r e11770b0adde mercurial/sslutil.py
--- a/mercurial/sslutil.py	Sun Mar 25 11:58:05 2018 +0900
+++ b/mercurial/sslutil.py	Mon Mar 26 12:38:42 2018 +0400
@@ -38,17 +38,20 @@ 
     'tls1.0',
     'tls1.1',
     'tls1.2',
+    'tls1.3',
 }
 
 hassni = getattr(ssl, 'HAS_SNI', False)
 
-# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
+# TLS 1.1,1.2 and 1.3 may not be supported if the OpenSSL Python is compiled
 # against doesn't support them.
 supportedprotocols = {'tls1.0'}
 if util.safehasattr(ssl, 'PROTOCOL_TLSv1_1'):
     supportedprotocols.add('tls1.1')
 if util.safehasattr(ssl, 'PROTOCOL_TLSv1_2'):
     supportedprotocols.add('tls1.2')
+if util.safehasattr(ssl, 'PROTOCOL_TLSv1_3'):
+    supportedprotocols.add('tls1.3')
 
 try:
     # ssl.SSLContext was added in 2.7.9 and presence indicates modern
@@ -293,7 +296,7 @@ 
     # Despite its name, PROTOCOL_SSLv23 selects the highest protocol
     # that both ends support, including TLS protocols. On legacy stacks,
     # the highest it likely goes is TLS 1.0. On modern stacks, it can
-    # support TLS 1.2.
+    # support TLS 1.2 or 1.3
     #
     # The PROTOCOL_TLSv* constants select a specific TLS version
     # only (as opposed to multiple versions). So the method for
@@ -323,6 +326,8 @@ 
         options |= ssl.OP_NO_TLSv1
     elif protocol == 'tls1.2':
         options |= ssl.OP_NO_TLSv1 | ssl.OP_NO_TLSv1_1
+    elif protocol == 'tls1.3':
+        options |= ssl.OP_NO_TLSv1 | ssl.OP_NO_TLSv1_1 | ssl.OP_NO_TLSv1_2
     else:
         raise error.Abort(_('this should not happen'))
 
@@ -542,6 +547,10 @@ 
         if 'tls1.2' not in supportedprotocols:
             raise error.Abort(_('TLS 1.2 not supported by this Python'))
         protocol = ssl.PROTOCOL_TLSv1_2
+    elif exactprotocol == 'tls1.3':
+        if 'tls1.3' not in supportedprotocols:
+            raise error.Abort(_('TLS 1.3 not supported by this Python'))
+        protocol = ssl.PROTOCOL_TLSv1_3
     elif exactprotocol:
         raise error.Abort(_('invalid value for serverexactprotocol: %s') %
                           exactprotocol)
diff -r 6715e8035b4f -r e11770b0adde tests/hghave.py
--- a/tests/hghave.py	Sun Mar 25 11:58:05 2018 +0900
+++ b/tests/hghave.py	Mon Mar 26 12:38:42 2018 +0400
@@ -523,6 +523,11 @@ 
     from mercurial import sslutil
     return 'tls1.2' in sslutil.supportedprotocols
 
+@check("tls1.3", "TLS 1.3 protocol support")
+def has_tls1_3():
+    from mercurial import sslutil
+    return 'tls1.3' in sslutil.supportedprotocols
+
 @check("windows", "Windows")
 def has_windows():
     return os.name == 'nt'
diff -r 6715e8035b4f -r e11770b0adde tests/test-https.t
--- a/tests/test-https.t	Sun Mar 25 11:58:05 2018 +0900
+++ b/tests/test-https.t	Mon Mar 26 12:38:42 2018 +0400
@@ -442,8 +442,9 @@ 
   $ killdaemons.py hg0.pid
   $ killdaemons.py hg1.pid
   $ killdaemons.py hg2.pid
+  $ killdaemons.py hg3.pid
 
-#if sslcontext tls1.2
+#if sslcontext tls1.3
 Start servers running supported TLS versions
 
   $ cd test
@@ -456,6 +457,9 @@ 
   $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid --certificate=$PRIV \
   > --config devel.serverexactprotocol=tls1.2
   $ cat ../hg2.pid >> $DAEMON_PIDS
+  $ hg serve -p $HGPORT3 -d --pid-file=../hg3.pid --certificate=$PRIV \
+  > --config devel.serverexactprotocol=tls1.3
+  $ cat ../hg3.pid >> $DAEMON_PIDS
   $ cd ..
 
 Clients talking same TLS versions work
@@ -466,6 +470,8 @@ 
   5fed3813f7f5
   $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT2/
   5fed3813f7f5
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.3 id https://localhost:$HGPORT3/
+  5fed3813f7f5
 
 Clients requiring newer TLS version than what server supports fail