Never send a session id in TLS1.3
[openssl.git] / ssl / statem / statem_clnt.c
index 80ae480b12103e12b4f8010110ad6b7762c78957..d5d622c1c5fb4d85e47cf33b205b7872813081e7 100644 (file)
@@ -181,6 +181,13 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
             return 1;
         }
         break;
+
+    case TLS_ST_OK:
+        if (mt == SSL3_MT_NEWSESSION_TICKET) {
+            st->hand_state = TLS_ST_CR_SESSION_TICKET;
+            return 1;
+        }
+        break;
     }
 
     /* No valid transition found */
@@ -351,6 +358,13 @@ int ossl_statem_client_read_transition(SSL *s, int mt)
             return 1;
         }
         break;
+
+    case TLS_ST_OK:
+        if (mt == SSL3_MT_HELLO_REQUEST) {
+            st->hand_state = TLS_ST_CR_HELLO_REQ;
+            return 1;
+        }
+        break;
     }
 
  err:
@@ -399,10 +413,15 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
         st->hand_state = TLS_ST_CW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
+    case TLS_ST_CR_SESSION_TICKET:
     case TLS_ST_CW_FINISHED:
         st->hand_state = TLS_ST_OK;
         ossl_statem_set_in_init(s, 0);
         return WRITE_TRAN_CONTINUE;
+
+    case TLS_ST_OK:
+        /* Just go straight to trying to read from the server */
+        return WRITE_TRAN_FINISHED;
     }
 }
 
@@ -428,6 +447,13 @@ WRITE_TRAN ossl_statem_client_write_transition(SSL *s)
         return WRITE_TRAN_ERROR;
 
     case TLS_ST_OK:
+        if (!s->renegotiate) {
+            /*
+             * We haven't requested a renegotiation ourselves so we must have
+             * received a message from the server. Better read it.
+             */
+            return WRITE_TRAN_FINISHED;
+        }
         /* Renegotiation - fall through */
     case TLS_ST_BEFORE:
         st->hand_state = TLS_ST_CW_CLNT_HELLO;
@@ -515,6 +541,23 @@ WRITE_TRAN ossl_statem_client_write_transition(SSL *s)
             ossl_statem_set_in_init(s, 0);
             return WRITE_TRAN_CONTINUE;
         }
+
+    case TLS_ST_CR_HELLO_REQ:
+        /*
+         * If we can renegotiate now then do so, otherwise wait for a more
+         * convenient time.
+         */
+        if (ssl3_renegotiate_check(s, 1)) {
+            if (!tls_setup_handshake(s)) {
+                ossl_statem_set_error(s);
+                return WRITE_TRAN_ERROR;
+            }
+            st->hand_state = TLS_ST_CW_CLNT_HELLO;
+            return WRITE_TRAN_CONTINUE;
+        }
+        st->hand_state = TLS_ST_OK;
+        ossl_statem_set_in_init(s, 0);
+        return WRITE_TRAN_CONTINUE;
     }
 }
 
@@ -559,7 +602,7 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst)
         break;
 
     case TLS_ST_OK:
-        return tls_finish_handshake(s, wst);
+        return tls_finish_handshake(s, wst, 1);
     }
 
     return WORK_FINISHED_CONTINUE;
@@ -819,6 +862,9 @@ MSG_PROCESS_RETURN ossl_statem_client_process_message(SSL *s, PACKET *pkt)
     case TLS_ST_CR_FINISHED:
         return tls_process_finished(s, pkt);
 
+    case TLS_ST_CR_HELLO_REQ:
+        return tls_process_hello_req(s, pkt);
+
     case TLS_ST_CR_ENCRYPTED_EXTENSIONS:
         return tls_process_encrypted_extensions(s, pkt);
     }
@@ -893,6 +939,9 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
     }
     /* else use the pre-loaded session */
 
+    /* This is a real handshake so make sure we clean it up at the end */
+    s->statem.cleanuphand = 1;
+
     p = s->s3->client_random;
 
     /*
@@ -954,7 +1003,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
     }
 
     /* Session ID */
-    if (s->new_session)
+    if (s->new_session || s->session->ssl_version == TLS1_3_VERSION)
         sess_id_len = 0;
     else
         sess_id_len = s->session->session_id_length;
@@ -1425,8 +1474,11 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt)
             if (!tls_collect_extensions(s, &extensions, EXT_TLS1_3_CERTIFICATE,
                                         &rawexts, &al)
                     || !tls_parse_all_extensions(s, EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al))
+                                                 rawexts, x, chainidx, &al)) {
+                OPENSSL_free(rawexts);
                 goto f_err;
+            }
+            OPENSSL_free(rawexts);
         }
 
         if (!sk_X509_push(sk, x)) {
@@ -1637,6 +1689,8 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al)
     DH *dh = NULL;
     BIGNUM *p = NULL, *g = NULL, *bnpub_key = NULL;
 
+    int check_bits = 0;
+
     if (!PACKET_get_length_prefixed_2(pkt, &prime)
         || !PACKET_get_length_prefixed_2(pkt, &generator)
         || !PACKET_get_length_prefixed_2(pkt, &pub_key)) {
@@ -1666,7 +1720,8 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al)
         goto err;
     }
 
-    if (BN_is_zero(p) || BN_is_zero(g) || BN_is_zero(bnpub_key)) {
+    /* test non-zero pupkey */
+    if (BN_is_zero(bnpub_key)) {
         *al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE);
         goto err;
@@ -1679,6 +1734,12 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al)
     }
     p = g = NULL;
 
+    if (DH_check_params(dh, &check_bits) == 0 || check_bits != 0) {
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE);
+        goto err;
+    }
+
     if (!DH_set0_key(dh, bnpub_key, NULL)) {
         *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, ERR_R_BN_LIB);
@@ -2134,21 +2195,30 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
 {
     int al;
     unsigned int ticklen;
-    unsigned long ticket_lifetime_hint;
+    unsigned long ticket_lifetime_hint, add_age;
     unsigned int sess_len;
+    RAW_EXTENSION *exts = NULL;
 
     if (!PACKET_get_net_4(pkt, &ticket_lifetime_hint)
+        || (SSL_IS_TLS13(s) && !PACKET_get_net_4(pkt, &add_age))
         || !PACKET_get_net_2(pkt, &ticklen)
-        || PACKET_remaining(pkt) != ticklen) {
+        || (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) != ticklen)
+        || (SSL_IS_TLS13(s) && (ticklen == 0
+                                || PACKET_remaining(pkt) < ticklen))) {
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, SSL_R_LENGTH_MISMATCH);
         goto f_err;
     }
 
-    /* Server is allowed to change its mind and send an empty ticket. */
+    /*
+     * Server is allowed to change its mind (in <=TLSv1.2) and send an empty
+     * ticket. We already checked this TLSv1.3 case above, so it should never
+     * be 0 here in that instance
+     */
     if (ticklen == 0)
         return MSG_PROCESS_CONTINUE_READING;
 
+    /* TODO(TLS1.3): Is this a suitable test for TLS1.3? */
     if (s->session->session_id_length > 0) {
         int i = s->session_ctx->session_cache_mode;
         SSL_SESSION *new_sess;
@@ -2190,6 +2260,21 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
 
     s->session->ext.tick_lifetime_hint = ticket_lifetime_hint;
     s->session->ext.ticklen = ticklen;
+
+    if (SSL_IS_TLS13(s)) {
+        PACKET extpkt;
+
+        if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
+                || !tls_collect_extensions(s, &extpkt,
+                                           EXT_TLS1_3_NEW_SESSION_TICKET,
+                                           &exts, &al)
+                || !tls_parse_all_extensions(s, EXT_TLS1_3_NEW_SESSION_TICKET,
+                                             exts, NULL, 0, &al)) {
+            SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, SSL_R_BAD_EXTENSION);
+            goto f_err;
+        }
+    }
+
     /*
      * There are two ways to detect a resumed ticket session. One is to set
      * an appropriate session ID and then the server must return a match in
@@ -2212,6 +2297,13 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
         goto err;
     }
     s->session->session_id_length = sess_len;
+
+    /* This is a standalone message in TLSv1.3, so there is no more to read */
+    if (SSL_IS_TLS13(s)) {
+        ssl_update_cache(s, SSL_SESS_CACHE_CLIENT);
+        return MSG_PROCESS_FINISHED_READING;
+    }
+
     return MSG_PROCESS_CONTINUE_READING;
  f_err:
     ssl3_send_alert(s, SSL3_AL_FATAL, al);
@@ -3100,6 +3192,30 @@ int tls_construct_next_proto(SSL *s, WPACKET *pkt)
 }
 #endif
 
+MSG_PROCESS_RETURN tls_process_hello_req(SSL *s, PACKET *pkt)
+{
+    if (PACKET_remaining(pkt) > 0) {
+        /* should contain no data */
+        SSLerr(SSL_F_TLS_PROCESS_HELLO_REQ, SSL_R_LENGTH_MISMATCH);
+        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+        ossl_statem_set_error(s);
+        return MSG_PROCESS_ERROR;
+    }
+
+    /*
+     * This is a historical discrepancy maintained for compatibility
+     * reasons. If a TLS client receives a HelloRequest it will attempt
+     * an abbreviated handshake. However if a DTLS client receives a
+     * HelloRequest it will do a full handshake.
+     */
+    if (SSL_IS_DTLS(s))
+        SSL_renegotiate(s);
+    else
+        SSL_renegotiate_abbreviated(s);
+
+    return MSG_PROCESS_FINISHED_READING;
+}
+
 static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt)
 {
     int al = SSL_AD_INTERNAL_ERROR;