Update code for the final RFC version of TLSv1.3 (RFC8446)

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6741)

diff --git a/CHANGES b/CHANGES
index 8d07a23709d2170531f7796132edb0a8d74249cd..760160fa1e67644859e1273ab1977780edd94f11 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -225,16 +225,7 @@
   *) Support for TLSv1.3 added. Note that users upgrading from an earlier
      version of OpenSSL should review their configuration settings to ensure
      that they are still appropriate for TLSv1.3. For further information see:
-     https://www.openssl.org/blog/blog/2018/02/08/tlsv1.3/
-
-     NOTE: In this pre-release of OpenSSL a draft version of the
-     TLSv1.3 standard has been implemented. Implementations of different draft
-     versions of the standard do not inter-operate, and this version will not
-     inter-operate with an implementation of the final standard when it is
-     eventually published. Different pre-release versions may implement
-     different versions of the draft. The final version of OpenSSL 1.1.1 will
-     implement the final version of the standard.
-     TODO(TLS1.3): Remove the above note before final release
+     https://wiki.openssl.org/index.php/TLS1.3
      [Matt Caswell]
 
   *) Grand redesign of the OpenSSL random generator

diff --git a/doc/man3/SSL_export_keying_material.pod b/doc/man3/SSL_export_keying_material.pod
index 0090097d4b27cce99101096bbde348a0da8cdf72..abebf911fc327ce2b2d71568e2e56f4641ff1f69 100644 (file)
--- a/doc/man3/SSL_export_keying_material.pod
+++ b/doc/man3/SSL_export_keying_material.pod
@@ -26,8 +26,7 @@ During the creation of a TLS or DTLS connection shared keying material is
 established between the two endpoints. The functions
 SSL_export_keying_material() and SSL_export_keying_material_early() enable an
 application to use some of this keying material for its own purposes in
-accordance with RFC5705 (for TLSv1.2 and below) or RFCXXXX (for TLSv1.3).
-TODO(TLS1.3): Update the RFC number when the RFC is published.
+accordance with RFC5705 (for TLSv1.2 and below) or RFC8446 (for TLSv1.3).
 
 SSL_export_keying_material() derives keying material using
 the F<exporter_master_secret> established in the handshake.

diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 2f19ccf2298fbed0fc81845432e5266b7d761d53..2e46cf80d34205f5939d24dab3a9507e25902164 100644 (file)
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -30,14 +30,6 @@ extern "C" {
 # define TLS1_3_VERSION                  0x0304
 # define TLS_MAX_VERSION                 TLS1_3_VERSION
 
-/* TODO(TLS1.3) REMOVE ME: Version indicators for draft version */
-# define TLS1_3_VERSION_DRAFT_26         0x7f1a
-# define TLS1_3_VERSION_DRAFT_27         0x7f1b
-# define TLS1_3_VERSION_DRAFT            0x7f1c
-# define TLS1_3_VERSION_DRAFT_TXT_26     "TLS 1.3 (draft 26)"
-# define TLS1_3_VERSION_DRAFT_TXT_27     "TLS 1.3 (draft 27)"
-# define TLS1_3_VERSION_DRAFT_TXT        "TLS 1.3 (draft 28)"
-
 /* Special value for method supporting multiple versions */
 # define TLS_ANY_VERSION                 0x10000
 

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index a1a880c05c496c72a12c571103785aa70fc3783e..6d6404ba3d5bd16021c0f4535e1abc2f59c834af 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1071,8 +1071,6 @@ struct ssl_st {
      * DTLS1_VERSION)
      */
     int version;
-    /* TODO(TLS1.3): Remove this before release */
-    int version_draft;
     /* SSLv3 */
     const SSL_METHOD *method;
     /*

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index cc4563b357622aa84342e955b6464544d355805c..86d6189ea1eac0bf4aabd5dc0e850864f24ac918 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -530,23 +530,8 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
-    /*
-     * TODO(TLS1.3): There is some discussion on the TLS list as to whether
-     * we should include versions <TLS1.2. For the moment we do. To be
-     * reviewed later.
-     */
     for (currv = max_version; currv >= min_version; currv--) {
-        /* TODO(TLS1.3): Remove this first if clause prior to release!! */
-        if (currv == TLS1_3_VERSION) {
-            if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)
-                    || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT_27)
-                    || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT_26)) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                         SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS,
-                         ERR_R_INTERNAL_ERROR);
-                return EXT_RETURN_FAIL;
-            }
-        } else if (!WPACKET_put_bytes_u16(pkt, currv)) {
+        if (!WPACKET_put_bytes_u16(pkt, currv)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS,
                      ERR_R_INTERNAL_ERROR);
@@ -1790,12 +1775,6 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
         return 0;
     }
 
-    /* TODO(TLS1.3): Remove this before release */
-    if (version == TLS1_3_VERSION_DRAFT
-            || version == TLS1_3_VERSION_DRAFT_27
-            || version == TLS1_3_VERSION_DRAFT_26)
-        version = TLS1_3_VERSION;
-
     /*
      * The only protocol version we support which is valid in this extension in
      * a ServerHello is TLSv1.3 therefore we shouldn't be getting anything else.

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 00c0ec9c09773d4695f960dd93746cbb5994870b..295d3e7ee555b50c9f4d01188666793b21be7eb8 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -897,8 +897,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     }
     if (!WPACKET_put_bytes_u16(&hrrpkt, TLSEXT_TYPE_supported_versions)
             || !WPACKET_start_sub_packet_u16(&hrrpkt)
-               /* TODO(TLS1.3): Fix this before release */
-            || !WPACKET_put_bytes_u16(&hrrpkt, s->version_draft)
+            || !WPACKET_put_bytes_u16(&hrrpkt, s->version)
             || !WPACKET_close(&hrrpkt)) {
         WPACKET_cleanup(&hrrpkt);
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_COOKIE,
@@ -1651,8 +1650,7 @@ EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt,
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
             || !WPACKET_start_sub_packet_u16(pkt)
-                /* TODO(TLS1.3): Update to remove the TLSv1.3 draft indicator */
-            || !WPACKET_put_bytes_u16(pkt, s->version_draft)
+            || !WPACKET_put_bytes_u16(pkt, s->version)
             || !WPACKET_close(pkt)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                  SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS,

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index d60284641601c626bfd00a27191a8f1656715535..d04f8773debc188e0d8c1235148d75a4638f4dcd 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1742,8 +1742,6 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
         unsigned int best_vers = 0;
         const SSL_METHOD *best_method = NULL;
         PACKET versionslist;
-        /* TODO(TLS1.3): Remove this before release */
-        unsigned int orig_candidate = 0;
 
         suppversions->parsed = 1;
 
@@ -1765,24 +1763,6 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
             return SSL_R_BAD_LEGACY_VERSION;
 
         while (PACKET_get_net_2(&versionslist, &candidate_vers)) {
-            /* TODO(TLS1.3): Remove this before release */
-            if (candidate_vers == TLS1_3_VERSION_DRAFT
-                    || candidate_vers == TLS1_3_VERSION_DRAFT_27
-                    || candidate_vers == TLS1_3_VERSION_DRAFT_26) {
-                if (best_vers == TLS1_3_VERSION
-                        && orig_candidate > candidate_vers)
-                    continue;
-                orig_candidate = candidate_vers;
-                candidate_vers = TLS1_3_VERSION;
-            } else if (candidate_vers == TLS1_3_VERSION) {
-                /* Don't actually accept real TLSv1.3 */
-                continue;
-            }
-            /*
-             * TODO(TLS1.3): There is some discussion on the TLS list about
-             * whether to ignore versions <TLS1.2 in supported_versions. At the
-             * moment we honour them if present. To be reviewed later
-             */
             if (version_cmp(s, candidate_vers, best_vers) <= 0)
                 continue;
             if (ssl_version_supported(s, candidate_vers, &best_method))
@@ -1805,9 +1785,6 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
             }
             check_for_downgrade(s, best_vers, dgrd);
             s->version = best_vers;
-            /* TODO(TLS1.3): Remove this before release */
-            if (best_vers == TLS1_3_VERSION)
-                s->version_draft = orig_candidate;
             s->method = best_method;
             return 0;
         }

diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c
index 4d052d0705a5f6d4dedf9b28518abe3b2488b84f..b79c776f2dba259cf04783f74c2b5c3e262f2b68 100644 (file)
--- a/ssl/t1_trce.c
+++ b/ssl/t1_trce.c
@@ -65,10 +65,6 @@ static const ssl_trace_tbl ssl_version_tbl[] = {
     {TLS1_1_VERSION, "TLS 1.1"},
     {TLS1_2_VERSION, "TLS 1.2"},
     {TLS1_3_VERSION, "TLS 1.3"},
-    /* TODO(TLS1.3): Remove these lines before release */
-    {TLS1_3_VERSION_DRAFT_26, TLS1_3_VERSION_DRAFT_TXT_26},
-    {TLS1_3_VERSION_DRAFT_27, TLS1_3_VERSION_DRAFT_TXT_27},
-    {TLS1_3_VERSION_DRAFT, TLS1_3_VERSION_DRAFT_TXT},
     {DTLS1_VERSION, "DTLS 1.0"},
     {DTLS1_2_VERSION, "DTLS 1.2"},
     {DTLS1_BAD_VER, "DTLS 1.0 (bad)"}
@@ -642,18 +638,8 @@ static int ssl_print_version(BIO *bio, int indent, const char *name,
     if (*pmsglen < 2)
         return 0;
     vers = ((*pmsg)[0] << 8) | (*pmsg)[1];
-    if (version != NULL) {
-        /* TODO(TLS1.3): Remove the draft conditional here before release */
-        switch(vers) {
-        case TLS1_3_VERSION_DRAFT_26:
-        case TLS1_3_VERSION_DRAFT_27:
-        case TLS1_3_VERSION_DRAFT:
-            *version = TLS1_3_VERSION;
-            break;
-        default:
-            *version = vers;
-        }
-    }
+    if (version != NULL)
+        *version = vers;
     BIO_indent(bio, indent, 80);
     BIO_printf(bio, "%s=0x%x (%s)\n",
                name, vers, ssl_trace_str(vers, ssl_version_tbl));

diff --git a/test/asynciotest.c b/test/asynciotest.c
index 73e415fdd328f1d2ee63309e20e58fc0794884e0..5e85cbb044a2e0175312e049b66e7c5b9021bf52 100644 (file)
--- a/test/asynciotest.c
+++ b/test/asynciotest.c
@@ -227,11 +227,9 @@ static int async_write(BIO *bio, const char *in, int inl)
                 /*
                  * We can't fragment anything after the ServerHello (or CCS <=
                  * TLS1.2), otherwise we get a bad record MAC
-                 * TODO(TLS1.3): Change TLS1_3_VERSION_DRAFT to TLS1_3_VERSION
-                 * before release
                  */
                 if (contenttype == SSL3_RT_CHANGE_CIPHER_SPEC
-                        || (negversion == TLS1_3_VERSION_DRAFT
+                        || (negversion == TLS1_3_VERSION
                             && msgtype == SSL3_MT_SERVER_HELLO)) {
                     fragment = 0;
                     break;

diff --git a/test/recipes/70-test_sslcertstatus.t b/test/recipes/70-test_sslcertstatus.t
index f66d3435784cf4ee2264923e8d6ad8d206a87d81..6fd89feaf75ea3c1fe1305471518a114a339b832 100644 (file)
--- a/test/recipes/70-test_sslcertstatus.t
+++ b/test/recipes/70-test_sslcertstatus.t
@@ -40,8 +40,6 @@ my $proxy = TLSProxy::Proxy->new(
 
 #Test 1: Sending a status_request extension in both ClientHello and
 #ServerHello but then omitting the CertificateStatus message is valid
-#TODO(TLS1.3): Temporarily disabling this test in TLS1.3 until we've completed
-#the move the status request extension to the Certificate message.
 $proxy->clientflags("-status -no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 1;

diff --git a/test/recipes/70-test_sslversions.t b/test/recipes/70-test_sslversions.t
index 8ef85af7a9c6acb8e7d667739eb8aa1fa21d8c97..b0342740d30c0c0bd1f42f01122fda90a8dd4047 100644 (file)
--- a/test/recipes/70-test_sslversions.t
+++ b/test/recipes/70-test_sslversions.t
@@ -145,8 +145,7 @@ sub modify_supported_versions_filter
                 $ext = pack "C5",
                     0x04, # Length
                     0x03, 0x03, #TLSv1.2
-                    #TODO(TLS1.3): Fix before release
-                    0x7f, 0x1c; #TLSv1.3 (draft 28)
+                    0x03, 0x04; #TLSv1.3
             } elsif ($testtype == UNRECOGNISED_VERSIONS) {
                 $ext = pack "C5",
                     0x04, # Length
@@ -160,8 +159,7 @@ sub modify_supported_versions_filter
             } elsif ($testtype == WITH_TLS1_4) {
                     $ext = pack "C5",
                         0x04, # Length
-                        #TODO(TLS1.3): Fix before release
-                        0x7f, 0x1c; #TLSv1.3 (draft 28)
+                        0x03, 0x04; #TLSv1.3
             }
             if ($testtype == REVERSE_ORDER_VERSIONS
                     || $testtype == UNRECOGNISED_VERSIONS

diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm
index dae6daa696456f34884943332a5bd35fdbfd3867..16ed0120669b884c95112137f395f670c04ebad5 100644 (file)
--- a/util/perl/TLSProxy/Message.pm
+++ b/util/perl/TLSProxy/Message.pm
@@ -95,9 +95,8 @@ use constant {
     EXT_FORCE_LAST => 0xffff
 };
 
-# SignatureScheme of TLS 1.3, from
-# https://tools.ietf.org/html/draft-ietf-tls-tls13-20#appendix-B.3.1.3
-# TODO(TLS1.3) update link to IANA registry after publication
+# SignatureScheme of TLS 1.3 from:
+# https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-signaturescheme
 # We have to manually grab the SHA224 equivalents from the old registry
 use constant {
     SIG_ALG_RSA_PKCS1_SHA256 => 0x0401,

diff --git a/util/perl/TLSProxy/Record.pm b/util/perl/TLSProxy/Record.pm
index 8db50d0bffea4e93922faa29d7badaee78099413..0a280cb269961bf750798ba89753d9d252368544 100644 (file)
--- a/util/perl/TLSProxy/Record.pm
+++ b/util/perl/TLSProxy/Record.pm
@@ -36,7 +36,6 @@ my %record_type = (
 
 use constant {
     VERS_TLS_1_4 => 0x0305,
-    VERS_TLS_1_3_DRAFT => 0x7f1c,
     VERS_TLS_1_3 => 0x0304,
     VERS_TLS_1_2 => 0x0303,
     VERS_TLS_1_1 => 0x0302,

diff --git a/util/perl/TLSProxy/ServerHello.pm b/util/perl/TLSProxy/ServerHello.pm
index 934eaf4dea328d88c6ab2d31787c07b4ee3a47ab..232c778b3413bbb90fd89dd1e0c5e4f50a08b41a 100644 (file)
--- a/util/perl/TLSProxy/ServerHello.pm
+++ b/util/perl/TLSProxy/ServerHello.pm
@@ -101,9 +101,7 @@ sub parse
 
     if ($random eq $hrrrandom) {
         TLSProxy::Proxy->is_tls13(1);
-        # TODO(TLS1.3): Replace this reference to draft version before release
-    } elsif ($neg_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
-        $neg_version = TLSProxy::Record::VERS_TLS_1_3;
+    } elsif ($neg_version == TLSProxy::Record::VERS_TLS_1_3) {
         TLSProxy::Proxy->is_tls13(1);
 
         TLSProxy::Record->server_encrypting(1);