Remove old style NewSessionTicket from TLSv1.3

TLSv1.3 has a NewSessionTicket message, but it is *completely* different to
the TLSv1.2 one and may as well have been called something else. This commit
removes the old style NewSessionTicket from TLSv1.3. We will have to add the
new style one back in later.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index f89d317ce796050506eae4278235b3cc40774743..0429e43ee751a2db8cebff12f804a174d95efa4d 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -136,12 +136,7 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
 
     case TLS_ST_CR_SRVR_HELLO:
         if (s->hit) {
-            if (s->tlsext_ticket_expected) {
-                if (mt == SSL3_MT_NEWSESSION_TICKET) {
-                    st->hand_state = TLS_ST_CR_SESSION_TICKET;
-                    return 1;
-                }
-            } else if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
+            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
                 st->hand_state = TLS_ST_CR_CHANGE;
                 return 1;
             }
@@ -182,18 +177,6 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_CW_FINISHED:
-        if (s->tlsext_ticket_expected) {
-            if (mt == SSL3_MT_NEWSESSION_TICKET) {
-                st->hand_state = TLS_ST_CR_SESSION_TICKET;
-                return 1;
-            }
-        } else if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_CR_CHANGE;
-            return 1;
-        }
-        break;
-
-    case TLS_ST_CR_SESSION_TICKET:
         if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
             st->hand_state = TLS_ST_CR_CHANGE;
             return 1;

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 97ecbcd178cf5159fee15d39098284b42e99b30f..c8c31a3a67da3015381d57c88b3e4c5174cc594f 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -419,8 +419,7 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
 
     case TLS_ST_SW_SRVR_HELLO:
         if (s->hit)
-            st->hand_state = s->tlsext_ticket_expected
-                                ? TLS_ST_SW_SESSION_TICKET : TLS_ST_SW_CHANGE;
+            st->hand_state = TLS_ST_SW_CHANGE;
         else
             st->hand_state = TLS_ST_SW_CERT;
 
@@ -453,14 +452,10 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
             ossl_statem_set_in_init(s, 0);
             return WRITE_TRAN_CONTINUE;
         }
+        st->hand_state = TLS_ST_SW_CHANGE;
 
-        st->hand_state = s->tlsext_ticket_expected ? TLS_ST_SW_SESSION_TICKET
-                                                   : TLS_ST_SW_CHANGE;
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_SESSION_TICKET:
-        st->hand_state = TLS_ST_SW_CHANGE;
-        return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_CHANGE:
         st->hand_state = TLS_ST_SW_FINISHED;

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 74022eeb2e6fa31e0cb097cee8989447dd0cd83d..19853cae4a20651d9a62ac486c7470ffff53a217 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -943,7 +943,7 @@ int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op)
 
 static int tls_use_ticket(SSL *s)
 {
-    if (s->options & SSL_OP_NO_TICKET)
+    if (s->options & SSL_OP_NO_TICKET || SSL_IS_TLS13(s))
         return 0;
     return ssl_security(s, SSL_SECOP_TICKET, 0, 0, NULL);
 }
@@ -2287,7 +2287,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello, int *al)
             }
         }
 #endif                          /* OPENSSL_NO_EC */
-        else if (currext->type == TLSEXT_TYPE_session_ticket) {
+        else if (currext->type == TLSEXT_TYPE_session_ticket
+                && !SSL_IS_TLS13(s)) {
             if (s->tls_session_ticket_ext_cb &&
                 !s->tls_session_ticket_ext_cb(s,
                     PACKET_data(&currext->data),
@@ -3176,7 +3177,8 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
     s->tlsext_ticket_expected = 0;
 
     /*
-     * If tickets disabled behave as if no ticket present to permit stateful
+     * If tickets disabled or not supported by the protocol version
+     * (e.g. TLSv1.3) behave as if no ticket present to permit stateful
      * resumption.
      */
     if (s->version <= SSL3_VERSION || !tls_use_ticket(s))

diff --git a/test/clienthellotest.c b/test/clienthellotest.c
index b8157f23c657b27a1ce947b0a456dfca2cc094f5..4219598846c0cafdbbab67b4f38cfa45817ec19d 100644 (file)
--- a/test/clienthellotest.c
+++ b/test/clienthellotest.c
@@ -56,10 +56,26 @@ int main(int argc, char *argv[])
     for (; currtest < TOTAL_NUM_TESTS; currtest++) {
         testresult = 0;
         ctx = SSL_CTX_new(TLS_method());
+
+        /*
+         * This test is testing session tickets for <= TLS1.2. It isn't relevant
+         * for TLS1.3
+         */
+        if (ctx == NULL || !SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))
+            goto end;
+
         con = SSL_new(ctx);
+        if (con == NULL)
+            goto end;
 
         rbio = BIO_new(BIO_s_mem());
         wbio = BIO_new(BIO_s_mem());
+        if (rbio == NULL || wbio == NULL) {
+            BIO_free(rbio);
+            BIO_free(wbio);
+            goto end;
+        }
+
         SSL_set_bio(con, rbio, wbio);
         SSL_set_connect_state(con);
 

diff --git a/test/recipes/70-test_sslsessiontick.t b/test/recipes/70-test_sslsessiontick.t
index 0c29ec7ce0dd894ffa59e04c46f77ec33c92c1d1..8b7b20c95e13aef190bd2c9e8ec563ea388a466b 100755 (executable)
--- a/test/recipes/70-test_sslsessiontick.t
+++ b/test/recipes/70-test_sslsessiontick.t
@@ -24,8 +24,8 @@ plan skip_all => "$test_name needs the dynamic engine feature enabled"
 plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
-plan skip_all => "$test_name needs TLS enabled"
-    if alldisabled(available_protocols("tls"));
+plan skip_all => "$test_name needs SSLv3, TLSv1, TLSv1.1 or TLSv1.2 enabled"
+    if alldisabled(("ssl3", "tls1", "tls1_1", "tls1_2"));
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 
@@ -48,6 +48,7 @@ my $proxy = TLSProxy::Proxy->new(
 #Test 1: By default with no existing session we should get a session ticket
 #Expected result: ClientHello extension seen; ServerHello extension seen
 #                 NewSessionTicket message seen; Full handshake
+$proxy->clientflags("-no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 10;
 checkmessages(1, "Default session ticket test", 1, 1, 1, 1);
@@ -57,6 +58,7 @@ checkmessages(1, "Default session ticket test", 1, 1, 1, 1);
 #Expected result: ClientHello extension seen; ServerHello extension not seen
 #                 NewSessionTicket message not seen; Full handshake
 clearall();
+$proxy->clientflags("-no_tls1_3");
 $proxy->serverflags("-no_ticket");
 $proxy->start();
 checkmessages(2, "No server support session ticket test", 1, 0, 0, 1);
@@ -66,7 +68,7 @@ checkmessages(2, "No server support session ticket test", 1, 0, 0, 1);
 #Expected result: ClientHello extension not seen; ServerHello extension not seen
 #                 NewSessionTicket message not seen; Full handshake
 clearall();
-$proxy->clientflags("-no_ticket");
+$proxy->clientflags("-no_tls1_3 -no_ticket");
 $proxy->start();
 checkmessages(3, "No client support session ticket test", 0, 0, 0, 1);
 
@@ -76,10 +78,10 @@ checkmessages(3, "No client support session ticket test", 0, 0, 0, 1);
 clearall();
 (undef, my $session) = tempfile();
 $proxy->serverconnects(2);
-$proxy->clientflags("-sess_out ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_out ".$session);
 $proxy->start();
 $proxy->clearClient();
-$proxy->clientflags("-sess_in ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_in ".$session);
 $proxy->clientstart();
 checkmessages(4, "Session resumption session ticket test", 1, 0, 0, 0);
 unlink $session;
@@ -90,10 +92,10 @@ unlink $session;
 clearall();
 (undef, $session) = tempfile();
 $proxy->serverconnects(2);
-$proxy->clientflags("-sess_out ".$session." -no_ticket");
+$proxy->clientflags("-no_tls1_3 -sess_out ".$session." -no_ticket");
 $proxy->start();
 $proxy->clearClient();
-$proxy->clientflags("-sess_in ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_in ".$session);
 $proxy->clientstart();
 checkmessages(5, "Session resumption with ticket capable client without a "
                  ."ticket", 1, 1, 1, 0);
@@ -104,6 +106,7 @@ unlink $session;
 #                 NewSessionTicket message seen; Full handshake.
 clearall();
 $proxy->filter(\&ticket_filter);
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 checkmessages(6, "Empty ticket test",  1, 1, 1, 1);
 
@@ -112,17 +115,17 @@ clearall();
 (undef, $session) = tempfile();
 $proxy->serverconnects(3);
 $proxy->filter(undef);
-$proxy->clientflags("-sess_out ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_out ".$session);
 $proxy->start();
 $proxy->clearClient();
-$proxy->clientflags("-sess_in ".$session." -sess_out ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_in ".$session." -sess_out ".$session);
 $proxy->filter(\&inject_empty_ticket_filter);
 $proxy->clientstart();
 #Expected result: ClientHello extension seen; ServerHello extension seen;
 #                 NewSessionTicket message seen; Abbreviated handshake.
 checkmessages(7, "Empty ticket resumption test",  1, 1, 1, 0);
 clearclient();
-$proxy->clientflags("-sess_in ".$session);
+$proxy->clientflags("-no_tls1_3 -sess_in ".$session);
 $proxy->filter(undef);
 $proxy->clientstart();
 #Expected result: ClientHello extension seen; ServerHello extension not seen;
@@ -134,6 +137,7 @@ unlink $session;
 #NewSessionTicket
 #Expected result: Connection failure
 clearall();
+$proxy->clientflags("-no_tls1_3");
 $proxy->serverflags("-no_ticket");
 $proxy->filter(\&inject_ticket_extension_filter);
 $proxy->start();
@@ -143,6 +147,7 @@ ok(TLSProxy::Message->fail, "Server sends ticket extension but no ticket test");
 #NewSessionTicket
 #Expected result: Connection failure
 clearall();
+$proxy->clientflags("-no_tls1_3");
 $proxy->serverflags("-no_ticket");
 $proxy->filter(\&inject_empty_ticket_filter);
 $proxy->start();

diff --git a/test/ssl-tests/06-sni-ticket.conf b/test/ssl-tests/06-sni-ticket.conf
index 9620e015a1a9e343e09204afaf11cb20728f2e72..ce0f63bc83baa7ee6e41665b00ee466ab8a1b39b 100644 (file)
--- a/test/ssl-tests/06-sni-ticket.conf
+++ b/test/ssl-tests/06-sni-ticket.conf
@@ -43,6 +43,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [0-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -84,6 +85,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [1-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -126,6 +128,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [2-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -168,6 +171,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [3-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -210,6 +214,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [4-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -252,6 +257,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [5-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -294,6 +300,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [6-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -336,6 +343,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [7-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -378,6 +386,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [8-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -420,6 +429,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [9-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -462,6 +472,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [10-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -504,6 +515,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [11-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -546,6 +558,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [12-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -588,6 +601,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [13-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -630,6 +644,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [14-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -672,6 +687,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [15-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -714,6 +730,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [16-sni-session-ticket-client]
 CipherString = DEFAULT
+MaxProtocol = TLSv1.2
 Options = -SessionTicket
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer

diff --git a/test/ssl-tests/06-sni-ticket.conf.in b/test/ssl-tests/06-sni-ticket.conf.in
index ccb9cbdb7b072967105a9d83ae5063303c4d8a5e..9c5266f268d06110132429396fb5cf6f8a193bcb 100644 (file)
--- a/test/ssl-tests/06-sni-ticket.conf.in
+++ b/test/ssl-tests/06-sni-ticket.conf.in
@@ -7,7 +7,7 @@
 # https://www.openssl.org/source/license.html
 
 
-## Test version negotiation
+## Test SNI/Session tickets
 
 use strict;
 use warnings;
@@ -17,12 +17,15 @@ package ssltests;
 
 our @tests = ();
 
+#Note: MaxProtocol is set to TLSv1.2 as session tickets work differently in
+#TLSv1.3.
+#TODO(TLS1.3): Implement TLSv1.3 style session tickets
 sub generate_tests() {
     foreach my $c ("SessionTicket", "-SessionTicket") {
-       foreach my $s1 ("SessionTicket", "-SessionTicket") {
-           foreach my $s2 ("SessionTicket", "-SessionTicket") {
-               foreach my $n ("server1", "server2") {
-                   my $result = expected_result($c, $s1, $s2, $n);
+        foreach my $s1 ("SessionTicket", "-SessionTicket") {
+            foreach my $s2 ("SessionTicket", "-SessionTicket") {
+                foreach my $n ("server1", "server2") {
+                    my $result = expected_result($c, $s1, $s2, $n);
                     push @tests, {
                         "name" => "sni-session-ticket",
                         "client" => {
@@ -30,6 +33,7 @@ sub generate_tests() {
                             "extra" => {
                                 "ServerName" => $n,
                             },
+                            "MaxProtocol" => "TLSv1.2"
                         },
                         "server" => {
                             "Options" => $s1,
@@ -38,13 +42,13 @@ sub generate_tests() {
                                 "ServerNameCallback" => "IgnoreMismatch",
                             },
                         },
-                       "server2" => {
-                           "Options" => $s2,
-                       },
+                        "server2" => {
+                            "Options" => $s2,
+                        },
                         "test" => {
                             "ExpectedServerName" => $n,
                             "ExpectedResult" => "Success",
-                           "SessionTicketExpected" => $result,
+                            "SessionTicketExpected" => $result,
                         }
                     };
                 }
@@ -72,23 +76,24 @@ sub expected_result {
 push @tests, {
     "name" => "sni-session-ticket",
     "client" => {
-       "Options" => "SessionTicket",
+        "MaxProtocol" => "TLSv1.2",
+        "Options" => "SessionTicket",
         "extra" => {
             "ServerName" => "server1",
         }
     },
     "server" => {
-       "Options" => "SessionTicket",
+        "Options" => "SessionTicket",
         "extra" => {
               "BrokenSessionTicket" => "Yes",
         },
     },
     "server2" => {
-       "Options" => "SessionTicket",
+        "Options" => "SessionTicket",
     },
     "test" => {
-       "ExpectedResult" => "Success",
-       "SessionTicketExpected" => "No",
+        "ExpectedResult" => "Success",
+        "SessionTicketExpected" => "No",
     }
 };