Split ServerHello extensions
authorMatt Caswell <matt@openssl.org>
Mon, 28 Nov 2016 16:45:52 +0000 (16:45 +0000)
committerMatt Caswell <matt@openssl.org>
Thu, 8 Dec 2016 17:19:11 +0000 (17:19 +0000)
In TLS1.3 some ServerHello extensions remain in the ServerHello, while
others move to the EncryptedExtensions message. This commit performs that
move.

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>
ssl/statem/statem_clnt.c
ssl/statem/statem_srvr.c
test/recipes/70-test_sslcertstatus.t

index aa1e6e1..c47c9aa 100644 (file)
@@ -1064,6 +1064,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     int i, al = SSL_AD_INTERNAL_ERROR;
     unsigned int compression;
     unsigned int sversion;
+    unsigned int context;
     int protverr;
     RAW_EXTENSION *extensions = NULL;
 #ifndef OPENSSL_NO_COMP
@@ -1306,24 +1307,10 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
-    /*
-     * TODO(TLS1.3): We give multiple contexts for now until we're ready to
-     * give something more specific
-     */
-
-    if (!tls_collect_extensions(s, &extpkt, EXT_TLS1_2_SERVER_HELLO
-                                            | EXT_TLS1_3_SERVER_HELLO
-                                            | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
-                                            | EXT_TLS1_3_CERTIFICATE,
-                                &extensions, &al))
-        goto f_err;
-
-
-    if (!tls_parse_all_extensions(s, EXT_TLS1_2_SERVER_HELLO
-                                     | EXT_TLS1_3_SERVER_HELLO
-                                     | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
-                                     | EXT_TLS1_3_CERTIFICATE,
-                                  extensions, &al))
+    context = SSL_IS_TLS13(s) ? EXT_TLS1_3_SERVER_HELLO
+                              : EXT_TLS1_2_SERVER_HELLO;
+    if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al)
+            || !tls_parse_all_extensions(s, context, extensions, &al))
         goto f_err;
 
 #ifndef OPENSSL_NO_SCTP
@@ -3108,14 +3095,28 @@ static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt)
 {
     int al = SSL_AD_INTERNAL_ERROR;
     PACKET extensions;
+    RAW_EXTENSION *rawexts = NULL;
 
-    /* TODO(TLS1.3): We need to process these extensions. For now ignore them */
     if (!PACKET_as_length_prefixed_2(pkt, &extensions)) {
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_ENCRYPTED_EXTENSIONS, SSL_R_LENGTH_MISMATCH);
         goto err;
     }
 
+    /*
+     * TODO(TLS1.3): For now we are processing Encrypted Extensions and
+     * Certificate extensions as part of this one message. Later we need to
+     * split out the Certificate extensions into the Certificate message
+     */
+    if (!tls_collect_extensions(s, &extensions,
+                                EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+                                | EXT_TLS1_3_CERTIFICATE, &rawexts, &al)
+            || !tls_parse_all_extensions(s,
+                                         EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+                                         | EXT_TLS1_3_CERTIFICATE,
+                                         rawexts, &al))
+        goto err;
+
     return MSG_PROCESS_CONTINUE_READING;
 
  err:
index 5aa395f..d40141d 100644 (file)
@@ -1910,15 +1910,10 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
             || (!SSL_IS_TLS13(s)
                 && !WPACKET_put_bytes_u8(pkt, compm))
-               /*
-                * TODO(TLS1.3): For now we add all 1.2 and 1.3 extensions. Later
-                * we will do this based on the actual protocol
-                */
             || !tls_construct_extensions(s, pkt,
-                                         EXT_TLS1_2_SERVER_HELLO
-                                         | EXT_TLS1_3_SERVER_HELLO
-                                         | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
-                                         | EXT_TLS1_3_CERTIFICATE, &al)) {
+                                         SSL_IS_TLS13(s)
+                                         ? EXT_TLS1_3_SERVER_HELLO
+                                         : EXT_TLS1_2_SERVER_HELLO, &al)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -3495,10 +3490,18 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
 
 static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt)
 {
-    /* TODO(TLS1.3): Zero length encrypted extensions message for now */
-    if (!WPACKET_put_bytes_u16(pkt, 0)) {
+    int al;
+
+    /*
+     * TODO(TLS1.3): For now we send certificate extensions in with the
+     * encrypted extensions. Later we need to move these to the certificate
+     * message.
+     */
+    if (!tls_construct_extensions(s, pkt, EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+                                          | EXT_TLS1_3_CERTIFICATE, &al)) {
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
         SSLerr(SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS, ERR_R_INTERNAL_ERROR);
-        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
         return 0;
     }
 
index f700f92..ed01855 100755 (executable)
@@ -39,7 +39,9 @@ my $proxy = TLSProxy::Proxy->new(
 
 #Test 1: Sending a status_request extension in both ClientHello and
 #ServerHello but then omitting the CertificateStatus message is valid
-$proxy->clientflags("-status");
+#TODO(TLS1.3): Temporarily disabling this test in TLS1.3 until we've completed
+#the move the status request extension to the Certificate message.
+$proxy->clientflags("-status -no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 1;
 ok(TLSProxy::Message->success, "Missing CertificateStatus message");