Send and Receive a TLSv1.3 format ServerHello

There are some minor differences in the format of a ServerHello in TLSv1.3.

Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz

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

3 files changed, 52 insertions, 31 deletions

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 287d8ab8a6..bd657aabef 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1089,17 +1089,22 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     s->hit = 0;
 
     /* Get the session-id. */
-    if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
-        goto f_err;
-    }
-    session_id_len = PACKET_remaining(&session_id);
-    if (session_id_len > sizeof s->session->session_id
-        || session_id_len > SSL3_SESSION_ID_SIZE) {
-        al = SSL_AD_ILLEGAL_PARAMETER;
-        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_SSL3_SESSION_ID_TOO_LONG);
-        goto f_err;
+    if (!SSL_IS_TLS13(s)) {
+        if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
+            goto f_err;
+        }
+        session_id_len = PACKET_remaining(&session_id);
+        if (session_id_len > sizeof s->session->session_id
+            || session_id_len > SSL3_SESSION_ID_SIZE) {
+            al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO,
+                   SSL_R_SSL3_SESSION_ID_TOO_LONG);
+            goto f_err;
+        }
+    } else {
+        session_id_len = 0;
     }
 
     if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
@@ -1120,8 +1125,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
      * we can resume, and later peek at the next handshake message to see if the
      * server wants to resume.
      */
-    if (s->version >= TLS1_VERSION && s->tls_session_secret_cb &&
-        s->session->tlsext_tick) {
+    if (s->version >= TLS1_VERSION && !SSL_IS_TLS13(s)
+            && s->tls_session_secret_cb && s->session->tlsext_tick) {
         const SSL_CIPHER *pref_cipher = NULL;
         /*
          * s->session->master_key_length is a size_t, but this is an int for
@@ -1235,11 +1240,16 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     s->s3->tmp.new_cipher = c;
     /* lets get the compression algorithm */
     /* COMPRESSION */
-    if (!PACKET_get_1(pkt, &compression)) {
-        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
-        al = SSL_AD_DECODE_ERROR;
-        goto f_err;
+    if (!SSL_IS_TLS13(s)) {
+        if (!PACKET_get_1(pkt, &compression)) {
+            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
+            al = SSL_AD_DECODE_ERROR;
+            goto f_err;
+        }
+    } else {
+        compression = 0;
     }
+
 #ifdef OPENSSL_NO_COMP
     if (compression != 0) {
         al = SSL_AD_ILLEGAL_PARAMETER;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 33808ed9f4..fa56af1764 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1773,9 +1773,11 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         compm = s->s3->tmp.new_compression->id;
 #endif
 
-    if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl)
+    if ((!SSL_IS_TLS13(s)
+                && !WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl))
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
-            || !WPACKET_put_bytes_u8(pkt, compm)
+            || (!SSL_IS_TLS13(s)
+                && !WPACKET_put_bytes_u8(pkt, compm))
             || !ssl_prepare_serverhello_tlsext(s)
             || !ssl_add_serverhello_tlsext(s, pkt, &al)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c
index 421d90d289..ee08d0eb87 100644
--- a/ssl/t1_trce.c
+++ b/ssl/t1_trce.c
@@ -588,12 +588,17 @@ static int ssl_print_hexbuf(BIO *bio, int indent,
 }
 
 static int ssl_print_version(BIO *bio, int indent, const char *name,
-                             const unsigned char **pmsg, size_t *pmsglen)
+                             const unsigned char **pmsg, size_t *pmsglen,
+                             unsigned int *version)
 {
     int vers;
     if (*pmsglen < 2)
         return 0;
     vers = ((*pmsg)[0] << 8) | (*pmsg)[1];
+    if (version != NULL) {
+        /* TODO(TLS1.3): Remove the draft conditional here before release */
+        *version = (vers == TLS1_3_VERSION_DRAFT) ? TLS1_3_VERSION : vers;
+    }
     BIO_indent(bio, indent, 80);
     BIO_printf(bio, "%s=0x%x (%s)\n",
                name, vers, ssl_trace_str(vers, ssl_version_tbl));
@@ -796,7 +801,7 @@ static int ssl_print_client_hello(BIO *bio, SSL *ssl, int indent,
 {
     size_t len;
     unsigned int cs;
-    if (!ssl_print_version(bio, indent, "client_version", &msg, &msglen))
+    if (!ssl_print_version(bio, indent, "client_version", &msg, &msglen, NULL))
         return 0;
     if (!ssl_print_random(bio, indent, &msg, &msglen))
         return 0;
@@ -849,7 +854,7 @@ static int ssl_print_client_hello(BIO *bio, SSL *ssl, int indent,
 static int dtls_print_hello_vfyrequest(BIO *bio, int indent,
                                        const unsigned char *msg, size_t msglen)
 {
-    if (!ssl_print_version(bio, indent, "server_version", &msg, &msglen))
+    if (!ssl_print_version(bio, indent, "server_version", &msg, &msglen, NULL))
         return 0;
     if (!ssl_print_hexbuf(bio, indent, "cookie", 1, &msg, &msglen))
         return 0;
@@ -860,11 +865,13 @@ static int ssl_print_server_hello(BIO *bio, int indent,
                                   const unsigned char *msg, size_t msglen)
 {
     unsigned int cs;
-    if (!ssl_print_version(bio, indent, "server_version", &msg, &msglen))
+    unsigned int vers;
+    if (!ssl_print_version(bio, indent, "server_version", &msg, &msglen, &vers))
         return 0;
     if (!ssl_print_random(bio, indent, &msg, &msglen))
         return 0;
-    if (!ssl_print_hexbuf(bio, indent, "session_id", 1, &msg, &msglen))
+    if (vers != TLS1_3_VERSION
+            && !ssl_print_hexbuf(bio, indent, "session_id", 1, &msg, &msglen))
         return 0;
     if (msglen < 2)
         return 0;
@@ -874,13 +881,15 @@ static int ssl_print_server_hello(BIO *bio, int indent,
                msg[0], msg[1], ssl_trace_str(cs, ssl_ciphers_tbl));
     msg += 2;
     msglen -= 2;
-    if (msglen < 1)
-        return 0;
-    BIO_indent(bio, indent, 80);
-    BIO_printf(bio, "compression_method: %s (0x%02X)\n",
-               ssl_trace_str(msg[0], ssl_comp_tbl), msg[0]);
-    msg++;
-    msglen--;
+    if (vers != TLS1_3_VERSION) {
+        if (msglen < 1)
+            return 0;
+        BIO_indent(bio, indent, 80);
+        BIO_printf(bio, "compression_method: %s (0x%02X)\n",
+                   ssl_trace_str(msg[0], ssl_comp_tbl), msg[0]);
+        msg++;
+        msglen--;
+    }
     if (!ssl_print_extensions(bio, indent, 1, msg, msglen))
         return 0;
     return 1;