Update state machine to be closer to TLS1.3

This is a major overhaul of the TLSv1.3 state machine. Currently it still
looks like TLSv1.2. This commit changes things around so that it starts
to look a bit less like TLSv1.2 and bit more like TLSv1.3.

After this commit we have:

ClientHello
+ key_share          ---->
                           ServerHello
                           +key_share
                           {CertificateRequest*}
                           {Certificate*}
                           {CertificateStatus*}
                     <---- {Finished}
{Certificate*}
{CertificateVerify*}
{Finished}           ---->
[ApplicationData]    <---> [Application Data]

Key differences between this intermediate position and the final TLSv1.3
position are:
- No EncryptedExtensions message yet
- No server side CertificateVerify message yet
- CertificateStatus still exists as a separate message
- A number of the messages are still in the TLSv1.2 format
- Still running on the TLSv1.2 record layer
Reviewed-by: NRich Salz <rsalz@openssl.org>

include/openssl/ssl.h

@@ -2242,7 +2242,8 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_SSL_WRITE_EX                               433
 # define SSL_F_STATE_MACHINE                              353
 # define SSL_F_TLS12_CHECK_PEER_SIGALG                    333
-# define SSL_F_TLS13_CHANGE_CIPHER_STATE                  435
+# define SSL_F_TLS13_CHANGE_CIPHER_STATE                  440
+# define SSL_F_TLS13_SETUP_KEY_BLOCK                      441
 # define SSL_F_TLS1_CHANGE_CIPHER_STATE                   209
 # define SSL_F_TLS1_CHECK_DUPLICATE_EXTENSIONS            341
 # define SSL_F_TLS1_ENC                                   401
@@ -2336,6 +2337,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_BIO_NOT_SET                                128
 # define SSL_R_BLOCK_CIPHER_PAD_IS_WRONG                  129
 # define SSL_R_BN_LIB                                     130
+# define SSL_R_CANNOT_CHANGE_CIPHER                       109
 # define SSL_R_CA_DN_LENGTH_MISMATCH                      131
 # define SSL_R_CA_KEY_TOO_SMALL                           397
 # define SSL_R_CA_MD_TOO_WEAK                             398

ssl/s3_lib.c

@@ -4067,8 +4067,8 @@ EVP_PKEY *ssl_generate_pkey_curve(int id)
 }
 #endif
 
-/* Derive premaster or master secret for ECDH/DH */
-int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int genmaster)
+/* Derive secrets for ECDH/DH */
+int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int gensecret)
 {
     int rv = 0;
     unsigned char *pms = NULL;
@@ -4093,9 +4093,20 @@ int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int genmaster)
     if (EVP_PKEY_derive(pctx, pms, &pmslen) <= 0)
         goto err;
 
-    if (genmaster) {
-        /* Generate master secret and discard premaster */
-        rv = ssl_generate_master_secret(s, pms, pmslen, 1);
+    if (gensecret) {
+        if (SSL_IS_TLS13(s)) {
+            /*
+             * TODO(TLS1.3): For now we just use the default early_secret, this
+             * will need to change later when other early_secrets will be
+             * possible.
+             */
+            rv = tls13_generate_early_secret(s, NULL, 0)
+                 && tls13_generate_handshake_secret(s, pms, pmslen);
+            OPENSSL_free(pms);
+        } else {
+            /* Generate master secret and discard premaster */
+            rv = ssl_generate_master_secret(s, pms, pmslen, 1);
+        }
         pms = NULL;
     } else {
         /* Save premaster secret */

ssl/ssl_err.c

@@ -239,6 +239,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_STATE_MACHINE), "state_machine"},
     {ERR_FUNC(SSL_F_TLS12_CHECK_PEER_SIGALG), "tls12_check_peer_sigalg"},
     {ERR_FUNC(SSL_F_TLS13_CHANGE_CIPHER_STATE), "tls13_change_cipher_state"},
+    {ERR_FUNC(SSL_F_TLS13_SETUP_KEY_BLOCK), "tls13_setup_key_block"},
     {ERR_FUNC(SSL_F_TLS1_CHANGE_CIPHER_STATE), "tls1_change_cipher_state"},
     {ERR_FUNC(SSL_F_TLS1_CHECK_DUPLICATE_EXTENSIONS),
      "tls1_check_duplicate_extensions"},
@@ -368,6 +369,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_REASON(SSL_R_BLOCK_CIPHER_PAD_IS_WRONG),
      "block cipher pad is wrong"},
     {ERR_REASON(SSL_R_BN_LIB), "bn lib"},
+    {ERR_REASON(SSL_R_CANNOT_CHANGE_CIPHER), "cannot change cipher"},
     {ERR_REASON(SSL_R_CA_DN_LENGTH_MISMATCH), "ca dn length mismatch"},
     {ERR_REASON(SSL_R_CA_KEY_TOO_SMALL), "ca key too small"},
     {ERR_REASON(SSL_R_CA_MD_TOO_WEAK), "ca md too weak"},

ssl/ssl_locl.h

@@ -2003,6 +2003,9 @@ __owur size_t tls1_final_finish_mac(SSL *s, const char *str, size_t slen,
 __owur int tls1_generate_master_secret(SSL *s, unsigned char *out,
                                        unsigned char *p, size_t len,
                                        size_t *secret_size);
+__owur int tls13_setup_key_block(SSL *s);
+__owur size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen,
+                                     unsigned char *p);
 __owur int tls13_change_cipher_state(SSL *s, int which);
 __owur int tls13_derive_secret(SSL *s, const unsigned char *insecret,
                                const unsigned char *label, size_t labellen,

ssl/statem/statem_clnt.c

@@ -136,18 +136,28 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
 
     case TLS_ST_CR_SRVR_HELLO:
         if (s->hit) {
-            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-                st->hand_state = TLS_ST_CR_CHANGE;
+            if (mt == SSL3_MT_FINISHED) {
+                st->hand_state = TLS_ST_CR_FINISHED;
                 return 1;
             }
         } else {
-            if (mt == SSL3_MT_CERTIFICATE) {
+            if (mt == SSL3_MT_CERTIFICATE_REQUEST) {
+                st->hand_state = TLS_ST_CR_CERT_REQ;
+                return 1;
+            } else if (mt == SSL3_MT_CERTIFICATE) {
                 st->hand_state = TLS_ST_CR_CERT;
                 return 1;
             }
         }
         break;
 
+    case TLS_ST_CR_CERT_REQ:
+        if (mt == SSL3_MT_CERTIFICATE) {
+            st->hand_state = TLS_ST_CR_CERT;
+            return 1;
+        }
+        break;
+
     case TLS_ST_CR_CERT:
         /*
          * The CertificateStatus message is optional even if
@@ -160,38 +170,14 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
         /* Fall through */
 
     case TLS_ST_CR_CERT_STATUS:
-        if (mt == SSL3_MT_CERTIFICATE_REQUEST) {
-            if (cert_req_allowed(s)) {
-                st->hand_state = TLS_ST_CR_CERT_REQ;
-                return 1;
-            }
-            goto err;
-        }
-        /* Fall through */
-
-    case TLS_ST_CR_CERT_REQ:
-        if (mt == SSL3_MT_SERVER_DONE) {
-            st->hand_state = TLS_ST_CR_SRVR_DONE;
-            return 1;
-        }
-        break;
-
-    case TLS_ST_CW_FINISHED:
-        if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_CR_CHANGE;
-            return 1;
-        }
-        break;
-
-    case TLS_ST_CR_CHANGE:
         if (mt == SSL3_MT_FINISHED) {
             st->hand_state = TLS_ST_CR_FINISHED;
             return 1;
         }
         break;
+
     }
 
- err:
     /* No valid transition found */
     ssl3_send_alert(s, SSL3_AL_FATAL, SSL3_AD_UNEXPECTED_MESSAGE);
     SSLerr(SSL_F_OSSL_STATEM_CLIENT13_READ_TRANSITION,
@@ -393,38 +379,22 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
         /* Shouldn't happen */
         return WRITE_TRAN_ERROR;
 
-    case TLS_ST_CR_SRVR_DONE:
+    case TLS_ST_CR_FINISHED:
         st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT
-                                                    : TLS_ST_CW_CHANGE;
+                                                    : TLS_ST_CW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_CW_CERT:
         /* If a non-empty Certificate we also send CertificateVerify */
         st->hand_state = (s->s3->tmp.cert_req == 1) ? TLS_ST_CW_CERT_VRFY
-                                                    : TLS_ST_CW_CHANGE;
+                                                    : TLS_ST_CW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_CW_CERT_VRFY:
-        st->hand_state = TLS_ST_CW_CHANGE;
-        return WRITE_TRAN_CONTINUE;
-
-    case TLS_ST_CW_CHANGE:
         st->hand_state = TLS_ST_CW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_CW_FINISHED:
-        if (s->hit) {
-            st->hand_state = TLS_ST_OK;
-            ossl_statem_set_in_init(s, 0);
-            return WRITE_TRAN_CONTINUE;
-        }
-        return WRITE_TRAN_FINISHED;
-
-    case TLS_ST_CR_FINISHED:
-        if (s->hit) {
-            st->hand_state = TLS_ST_CW_CHANGE;
-            return WRITE_TRAN_CONTINUE;
-        }
         st->hand_state = TLS_ST_OK;
         ossl_statem_set_in_init(s, 0);
         return WRITE_TRAN_CONTINUE;
@@ -666,6 +636,12 @@ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst)
 #endif
         if (statem_flush(s) != 1)
             return WORK_MORE_B;
+
+        if (SSL_IS_TLS13(s)) {
+            if (!s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_WRITE))
+            return WORK_ERROR;
+        }
         break;
     }
 
@@ -1343,6 +1319,21 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 #endif
 
+    /*
+     * In TLSv1.3 we have some post-processing to change cipher state, otherwise
+     * we're done with this message
+     */
+    if (SSL_IS_TLS13(s)
+            && (!s->method->ssl3_enc->setup_key_block(s)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                    SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_WRITE)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                    SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_READ))) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_CANNOT_CHANGE_CIPHER);
+        goto f_err;
+    }
+
     return MSG_PROCESS_CONTINUE_READING;
  f_err:
     ssl3_send_alert(s, SSL3_AL_FATAL, al);

ssl/statem/statem_lib.c

@@ -330,7 +330,7 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
     size_t md_len;
 
     /* If this occurs, we have missed a message */
-    if (!s->s3->change_cipher_spec) {
+    if (!SSL_IS_TLS13(s) && !s->s3->change_cipher_spec) {
         al = SSL_AD_UNEXPECTED_MESSAGE;
         SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_GOT_A_FIN_BEFORE_A_CCS);
         goto f_err;
@@ -367,6 +367,32 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
         s->s3->previous_server_finished_len = md_len;
     }
 
+    /* In TLS1.3 we also have to change cipher state */
+    if (SSL_IS_TLS13(s)) {
+        if (s->server) {
+            if (!s->method->ssl3_enc->change_cipher_state(s,
+                    SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_READ)) {
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
+                goto f_err;
+            }
+        } else {
+            if (!s->method->ssl3_enc->generate_master_secret(s,
+                    s->session->master_key, s->handshake_secret, 0,
+                    &s->session->master_key_length)) {
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
+                goto f_err;
+            }
+            if (!s->method->ssl3_enc->change_cipher_state(s,
+                    SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
+                goto f_err;
+            }
+        }
+    }
+
     return MSG_PROCESS_FINISHED_READING;
  f_err:
     ssl3_send_alert(s, SSL3_AL_FATAL, al);

ssl/statem/statem_srvr.c

@@ -94,15 +94,15 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
     default:
         break;
 
-    case TLS_ST_SW_SRVR_DONE:
+    case TLS_ST_SW_FINISHED:
         if (s->s3->tmp.cert_request) {
             if (mt == SSL3_MT_CERTIFICATE) {
                 st->hand_state = TLS_ST_SR_CERT;
                 return 1;
             }
         } else {
-            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-                st->hand_state = TLS_ST_SR_CHANGE;
+            if (mt == SSL3_MT_FINISHED) {
+                st->hand_state = TLS_ST_SR_FINISHED;
                 return 1;
             }
         }
@@ -110,8 +110,8 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
 
     case TLS_ST_SR_CERT:
         if (s->session->peer == NULL) {
-            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-                st->hand_state = TLS_ST_SR_CHANGE;
+            if (mt == SSL3_MT_FINISHED) {
+                st->hand_state = TLS_ST_SR_FINISHED;
                 return 1;
             }
         } else {
@@ -123,25 +123,11 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_SR_CERT_VRFY:
-        if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_SR_CHANGE;
-            return 1;
-        }
-        break;
-
-    case TLS_ST_SR_CHANGE:
         if (mt == SSL3_MT_FINISHED) {
             st->hand_state = TLS_ST_SR_FINISHED;
             return 1;
         }
         break;
-
-    case TLS_ST_SW_FINISHED:
-        if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_SR_CHANGE;
-            return 1;
-        }
-        break;
     }
 
     /* No valid transition found */
@@ -419,52 +405,33 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
 
     case TLS_ST_SW_SRVR_HELLO:
         if (s->hit)
-            st->hand_state = TLS_ST_SW_CHANGE;
+            st->hand_state = TLS_ST_SW_FINISHED;
+        else if (send_certificate_request(s))
+            st->hand_state = TLS_ST_SW_CERT_REQ;
         else
             st->hand_state = TLS_ST_SW_CERT;
 
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_CERT:
-        if (s->tlsext_status_expected) {
-            st->hand_state = TLS_ST_SW_CERT_STATUS;
-            return WRITE_TRAN_CONTINUE;
-        }
-        /* Fall through */
-
-    case TLS_ST_SW_CERT_STATUS:
-        if (send_certificate_request(s)) {
-            st->hand_state = TLS_ST_SW_CERT_REQ;
-            return WRITE_TRAN_CONTINUE;
-        }
-        /* Fall through */
-
     case TLS_ST_SW_CERT_REQ:
-        st->hand_state = TLS_ST_SW_SRVR_DONE;
+        st->hand_state = TLS_ST_SW_CERT;
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_SRVR_DONE:
-        return WRITE_TRAN_FINISHED;
-
-    case TLS_ST_SR_FINISHED:
-        if (s->hit) {
-            st->hand_state = TLS_ST_OK;
-            ossl_statem_set_in_init(s, 0);
-            return WRITE_TRAN_CONTINUE;
-        }
-        st->hand_state = TLS_ST_SW_CHANGE;
-
+    case TLS_ST_SW_CERT:
+        if (s->tlsext_status_expected)
+            st->hand_state = TLS_ST_SW_CERT_STATUS;
+        else
+            st->hand_state = TLS_ST_SW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
-
-    case TLS_ST_SW_CHANGE:
+    case TLS_ST_SW_CERT_STATUS:
         st->hand_state = TLS_ST_SW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_FINISHED:
-        if (s->hit)
-            return WRITE_TRAN_FINISHED;
+        return WRITE_TRAN_FINISHED;
 
+    case TLS_ST_SR_FINISHED:
         st->hand_state = TLS_ST_OK;
         ossl_statem_set_in_init(s, 0);
         return WRITE_TRAN_CONTINUE;
@@ -740,6 +707,20 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                      sizeof(sctpauthkey), sctpauthkey);
         }
 #endif
+        /*
+         * TODO(TLS1.3): This actually causes a problem. We don't yet know
+         * whether the next record we are going to receive is an unencrypted
+         * alert, or an encrypted handshake message. We're going to need
+         * something clever in the record layer for this.
+         */
+        if (SSL_IS_TLS13(s)) {
+            if (!s->method->ssl3_enc->setup_key_block(s)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_SERVER_WRITE)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_HANDSHAKE |SSL3_CHANGE_CIPHER_SERVER_READ))
+            return WORK_ERROR;
+        }
         break;
 
     case TLS_ST_SW_CHANGE:
@@ -782,6 +763,14 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                      0, NULL);
         }
 #endif
+        if (SSL_IS_TLS13(s)) {
+            if (!s->method->ssl3_enc->generate_master_secret(s,
+                        s->session->master_key, s->handshake_secret, 0,
+                        &s->session->master_key_length)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_WRITE))
+            return WORK_ERROR;
+        }
         break;
     }
 
@@ -1001,7 +990,7 @@ WORK_STATE ossl_statem_server_post_process_message(SSL *s, WORK_STATE wst)
 #endif
         return WORK_FINISHED_CONTINUE;
     }
-
+    return WORK_FINISHED_CONTINUE;
 }
 
 #ifndef OPENSSL_NO_SRP

ssl/t1_lib.c

@@ -81,10 +81,10 @@ SSL3_ENC_METHOD const TLSv1_2_enc_data = {
 SSL3_ENC_METHOD const TLSv1_3_enc_data = {
     tls1_enc,
     tls1_mac,
-    tls1_setup_key_block,
-    tls1_generate_master_secret,
-    tls1_change_cipher_state,
-    tls1_final_finish_mac,
+    tls13_setup_key_block,
+    tls13_generate_master_secret,
+    tls13_change_cipher_state,
+    tls13_final_finish_mac,
     TLS_MD_CLIENT_FINISH_CONST, TLS_MD_CLIENT_FINISH_CONST_SIZE,
     TLS_MD_SERVER_FINISH_CONST, TLS_MD_SERVER_FINISH_CONST_SIZE,
     tls1_alert_code,

ssl/tls13_enc.c

@@ -214,6 +214,53 @@ int tls13_generate_master_secret(SSL *s, unsigned char *out,
     return tls13_generate_secret(s, prev, NULL, 0, out);
 }
 
+/*
+ * Generates the mac for the Finished message.
+ *
+ * Returns the length of the MAC or 0 on error.
+ */
+size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen,
+                             unsigned char *out)
+{
+    size_t hashlen;
+    const EVP_MD *md;
+
+    /*
+     * TODO(TLS1.3): This is a dummy implementation for now. We need to come
+     * back and fill this in.
+     */
+    md = ssl_handshake_md(s);
+    hashlen = EVP_MD_size(md);
+    memset(out, 0, hashlen);
+
+    return hashlen;
+}
+
+/*
+ * There isn't really a key block in TLSv1.3, but we still need this function
+ * for initialising the cipher and hash.
+ *
+ * Returns 1 on success or 0 on failure.
+ */
+int tls13_setup_key_block(SSL *s)
+{
+    const EVP_CIPHER *c;
+    const EVP_MD *hash;
+    int mac_type = NID_undef;
+
+    s->session->cipher = s->s3->tmp.new_cipher;
+    if (!ssl_cipher_get_evp
+        (s->session, &c, &hash, &mac_type, NULL, NULL, 0)) {
+        SSLerr(SSL_F_TLS13_SETUP_KEY_BLOCK, SSL_R_CIPHER_OR_HASH_UNAVAILABLE);
+        return 0;
+    }
+
+    s->s3->tmp.new_sym_enc = c;
+    s->s3->tmp.new_hash = hash;
+
+    return 1;
+}
+
 const unsigned char client_handshake_traffic[] =
     "client handshake traffic secret";
 const unsigned char client_application_traffic[] =

test/tls13secretstest.c

@@ -170,6 +170,14 @@ void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl)
 {
 }
 
+int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc,
+                       const EVP_MD **md, int *mac_pkey_type,
+                       size_t *mac_secret_size, SSL_COMP **comp, int use_etm)
+
+{
+    return 0;
+}
+
 /* End of mocked out code */
 
 static int test_secret(SSL *s, unsigned char *prk,