Make CertificateVerify TLS1.3 aware

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)

3 files changed, 129 insertions, 12 deletions

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index d0d384c14b..ff57e9217a 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -169,12 +169,18 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_CR_CERT:
+        if (mt == SSL3_MT_CERTIFICATE_VERIFY) {
+            st->hand_state = TLS_ST_CR_CERT_VRFY;
+            return 1;
+        }
+        break;
+
+    case TLS_ST_CR_CERT_VRFY:
         if (mt == SSL3_MT_FINISHED) {
             st->hand_state = TLS_ST_CR_FINISHED;
             return 1;
         }
         break;
-
     }
 
     /* No valid transition found */
@@ -562,6 +568,8 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst)
 /*
  * Perform any work that needs to be done after sending a message from the
  * client to the server.
+    case TLS_ST_SR_CERT_VRFY:
+        return SSL3_RT_MAX_PLAIN_LENGTH;
  */
 WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst)
 {
@@ -730,6 +738,9 @@ size_t ossl_statem_client_max_message_size(SSL *s)
     case TLS_ST_CR_CERT:
         return s->max_cert_list;
 
+    case TLS_ST_CR_CERT_VRFY:
+        return SSL3_RT_MAX_PLAIN_LENGTH;
+
     case TLS_ST_CR_CERT_STATUS:
         return SSL3_RT_MAX_PLAIN_LENGTH;
 
@@ -784,6 +795,9 @@ MSG_PROCESS_RETURN ossl_statem_client_process_message(SSL *s, PACKET *pkt)
     case TLS_ST_CR_CERT:
         return tls_process_server_certificate(s, pkt);
 
+    case TLS_ST_CR_CERT_VRFY:
+        return tls_process_cert_verify(s, pkt);
+
     case TLS_ST_CR_CERT_STATUS:
         return tls_process_cert_status(s, pkt);
 
@@ -1497,8 +1511,18 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt)
     X509_up_ref(x);
     s->session->peer = x;
     s->session->verify_result = s->verify_result;
-
     x = NULL;
+
+    /* Save the current hash state for when we receive the CertificateVerify */
+    if (SSL_IS_TLS13(s)
+            && !ssl_handshake_hash(s, s->cert_verify_hash,
+                                   sizeof(s->cert_verify_hash),
+                                   &s->cert_verify_hash_len)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_SERVER_CERTIFICATE, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
+
     ret = MSG_PROCESS_CONTINUE_READING;
     goto done;
 
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 0395e9f002..1a6c1fccf8 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -72,30 +72,99 @@ int tls_close_construct_packet(SSL *s, WPACKET *pkt, int htype)
     return 1;
 }
 
+/*
+ * Size of the to-be-signed TLS13 data, without the hash size itself:
+ * 64 bytes of value 32, 33 context bytes, 1 byte separator
+ */
+#define TLS13_TBS_START_SIZE            64
+#define TLS13_TBS_PREAMBLE_SIZE         (TLS13_TBS_START_SIZE + 33 + 1)
+
+static int get_cert_verify_tbs_data(SSL *s, unsigned char *tls13tbs,
+                                    void **hdata, size_t *hdatalen)
+{
+    static const char *servercontext = "TLS 1.3, server CertificateVerify";
+    static const char *clientcontext = "TLS 1.3, client CertificateVerify";
+
+    if (SSL_IS_TLS13(s)) {
+        size_t hashlen;
+
+        /* Set the first 64 bytes of to-be-signed data to octet 32 */
+        memset(tls13tbs, 32, TLS13_TBS_START_SIZE);
+        /* This copies the 33 bytes of context plus the 0 separator byte */
+        if (s->statem.hand_state == TLS_ST_CR_CERT_VRFY
+                 || s->statem.hand_state == TLS_ST_SW_CERT_VRFY)
+            strcpy((char *)tls13tbs + TLS13_TBS_START_SIZE, servercontext);
+        else
+            strcpy((char *)tls13tbs + TLS13_TBS_START_SIZE, clientcontext);
+
+        /*
+         * If we're currently reading then we need to use the saved handshake
+         * hash value. We can't use the current handshake hash state because
+         * that includes the CertVerify itself.
+         */
+        if (s->statem.hand_state == TLS_ST_CR_CERT_VRFY
+                || s->statem.hand_state == TLS_ST_SR_CERT_VRFY) {
+            memcpy(tls13tbs + TLS13_TBS_PREAMBLE_SIZE, s->cert_verify_hash,
+                   s->cert_verify_hash_len);
+            hashlen = s->cert_verify_hash_len;
+        } else if (!ssl_handshake_hash(s, tls13tbs + TLS13_TBS_PREAMBLE_SIZE,
+                                       EVP_MAX_MD_SIZE, &hashlen)) {
+            return 0;
+        }
+
+        *hdata = tls13tbs;
+        *hdatalen = TLS13_TBS_PREAMBLE_SIZE + hashlen;
+    } else {
+        size_t retlen;
+
+        retlen = BIO_get_mem_data(s->s3->handshake_buffer, hdata);
+        if (retlen <= 0)
+            return 0;
+        *hdatalen = retlen;
+    }
+
+    return 1;
+}
+
 int tls_construct_cert_verify(SSL *s, WPACKET *pkt)
 {
     EVP_PKEY *pkey;
-    const EVP_MD *md = s->s3->tmp.md[s->cert->key - s->cert->pkeys];
+    const EVP_MD *md;
     EVP_MD_CTX *mctx = NULL;
     unsigned u = 0;
-    long hdatalen = 0;
+    size_t hdatalen = 0;
     void *hdata;
     unsigned char *sig = NULL;
+    unsigned char tls13tbs[TLS13_TBS_PREAMBLE_SIZE + EVP_MAX_MD_SIZE];
+
+    if (s->server) {
+        /* Only happens in TLSv1.3 */
+        /*
+         * TODO(TLS1.3): This needs to change. We should not get this from the
+         * cipher. However, for now, we have not done the work to separate the
+         * certificate type from the ciphersuite
+         */
+        pkey = ssl_get_sign_pkey(s, s->s3->tmp.new_cipher, &md);
+        if (pkey == NULL)
+            goto err;
+    } else {
+        md = s->s3->tmp.md[s->cert->key - s->cert->pkeys];
+        pkey = s->cert->key->privatekey;
+    }
 
     mctx = EVP_MD_CTX_new();
     if (mctx == NULL) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_MALLOC_FAILURE);
         goto err;
     }
-    pkey = s->cert->key->privatekey;
 
-    hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
-    if (hdatalen <= 0) {
+    /* Get the data to be signed */
+    if (!get_cert_verify_tbs_data(s, tls13tbs, &hdata, &hdatalen)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR);
         goto err;
     }
 
-    if (SSL_USE_SIGALGS(s)&& !tls12_get_sigandhash(pkt, pkey, md)) {
+    if (SSL_USE_SIGALGS(s) && !tls12_get_sigandhash(pkt, pkey, md)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -158,8 +227,9 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
     unsigned int len;
     X509 *peer;
     const EVP_MD *md = NULL;
-    long hdatalen = 0;
+    size_t hdatalen = 0;
     void *hdata;
+    unsigned char tls13tbs[TLS13_TBS_PREAMBLE_SIZE + EVP_MAX_MD_SIZE];
 
     EVP_MD_CTX *mctx = EVP_MD_CTX_new();
 
@@ -240,8 +310,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
-    hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
-    if (hdatalen <= 0) {
+    if (!get_cert_verify_tbs_data(s, tls13tbs, &hdata, &hdatalen)) {
         SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_INTERNAL_ERROR);
         al = SSL_AD_INTERNAL_ERROR;
         goto f_err;
@@ -288,7 +357,10 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
-    ret = MSG_PROCESS_CONTINUE_PROCESSING;
+    if (SSL_IS_TLS13(s))
+        ret = MSG_PROCESS_CONTINUE_READING;
+    else
+        ret = MSG_PROCESS_CONTINUE_PROCESSING;
     if (0) {
  f_err:
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index f29f0f3412..d08ed6f77b 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -427,6 +427,10 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_CERT:
+        st->hand_state = TLS_ST_SW_CERT_VRFY;
+        return WRITE_TRAN_CONTINUE;
+
+    case TLS_ST_SW_CERT_VRFY:
         st->hand_state = TLS_ST_SW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
@@ -826,6 +830,12 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt,
         *mt = SSL3_MT_CERTIFICATE;
         break;
 
+    case TLS_ST_SW_CERT_VRFY:
+        *confunc = tls_construct_cert_verify;
+        *mt = SSL3_MT_CERTIFICATE_VERIFY;
+        break;
+
+
     case TLS_ST_SW_KEY_EXCH:
         *confunc = tls_construct_server_key_exchange;
         *mt = SSL3_MT_SERVER_KEY_EXCHANGE;
@@ -3109,6 +3119,17 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
      * certificate, while we do include it in statem_clnt.c
      */
     sk = NULL;
+
+    /* Save the current hash state for when we receive the CertificateVerify */
+    if (SSL_IS_TLS13(s)
+            && !ssl_handshake_hash(s, s->cert_verify_hash,
+                                   sizeof(s->cert_verify_hash),
+                                   &s->cert_verify_hash_len)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
+
     ret = MSG_PROCESS_CONTINUE_READING;
     goto done;