[openssl-commits] [openssl] master update

Dr. Stephen Henson steve at openssl.org
Fri Mar 17 18:43:02 UTC 2017


The branch master has been updated
       via  45615c5fac0aba7bd41be270c4bcf194bf1049f4 (commit)
       via  32f661079d6a4bd5aa22921ed9cdbc3369ec40e8 (commit)
       via  51c7d3e824612a9c71bd987862a00140eb4b0711 (commit)
       via  5d6cca05b0c17bbed1e24e17dfddb9a309c0516b (commit)
      from  fa013b65241dfed9b7d9e10e0adfedc9869c797e (commit)


- Log -----------------------------------------------------------------
commit 45615c5fac0aba7bd41be270c4bcf194bf1049f4
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Fri Mar 10 16:31:20 2017 +0000

    Implement certificate_authorities extension
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2918)

commit 32f661079d6a4bd5aa22921ed9cdbc3369ec40e8
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Mon Mar 13 13:29:34 2017 +0000

    Support draft-19 TLS certificate request format
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2918)

commit 51c7d3e824612a9c71bd987862a00140eb4b0711
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Mon Mar 13 13:27:18 2017 +0000

    Allow signature algorithms in TLS 1.3 certificate request extensions.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2918)

commit 5d6cca05b0c17bbed1e24e17dfddb9a309c0516b
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Wed Mar 8 18:17:17 2017 +0000

    Move parsing and construction of CA names to separate functions
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2918)

-----------------------------------------------------------------------

Summary of changes:
 include/openssl/ssl.h    |   2 +
 include/openssl/tls1.h   |   1 +
 ssl/ssl_err.c            |   3 +
 ssl/ssl_locl.h           |   1 +
 ssl/statem/extensions.c  |  61 +++++++++++++++++++-
 ssl/statem/statem_clnt.c | 143 +++++++++++++++++------------------------------
 ssl/statem/statem_lib.c  |  95 +++++++++++++++++++++++++++++++
 ssl/statem/statem_locl.h |   4 ++
 ssl/statem/statem_srvr.c |  57 +++++--------------
 9 files changed, 230 insertions(+), 137 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 8003959..1041e3c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2174,6 +2174,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_OSSL_STATEM_SERVER13_READ_TRANSITION       437
 # define SSL_F_OSSL_STATEM_SERVER_CONSTRUCT_MESSAGE       431
 # define SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION         418
+# define SSL_F_PARSE_CA_NAMES                             541
 # define SSL_F_PROCESS_KEY_SHARE_EXT                      439
 # define SSL_F_READ_STATE_MACHINE                         352
 # define SSL_F_SET_CLIENT_CIPHERSUITE                     540
@@ -2335,6 +2336,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_CHOOSE_SIGALG                          513
 # define SSL_F_TLS_CLIENT_KEY_EXCHANGE_POST_WORK          354
 # define SSL_F_TLS_COLLECT_EXTENSIONS                     435
+# define SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES      542
 # define SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST          372
 # define SSL_F_TLS_CONSTRUCT_CERT_STATUS                  429
 # define SSL_F_TLS_CONSTRUCT_CERT_STATUS_BODY             494
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index f2af3ab..38ad74c 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -184,6 +184,7 @@ extern "C" {
 # define TLSEXT_TYPE_supported_versions          43
 # define TLSEXT_TYPE_cookie                      44
 # define TLSEXT_TYPE_psk_kex_modes               45
+# define TLSEXT_TYPE_certificate_authorities     47
 
 /* Temporary extension type */
 # define TLSEXT_TYPE_renegotiate                 0xff01
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index f7ee171..c7e407f 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -74,6 +74,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
      "ossl_statem_server_construct_message"},
     {ERR_FUNC(SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION),
      "ossl_statem_server_read_transition"},
+    {ERR_FUNC(SSL_F_PARSE_CA_NAMES), "parse_ca_names"},
     {ERR_FUNC(SSL_F_PROCESS_KEY_SHARE_EXT), "process_key_share_ext"},
     {ERR_FUNC(SSL_F_READ_STATE_MACHINE), "read_state_machine"},
     {ERR_FUNC(SSL_F_SET_CLIENT_CIPHERSUITE), "set_client_ciphersuite"},
@@ -281,6 +282,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_CLIENT_KEY_EXCHANGE_POST_WORK),
      "tls_client_key_exchange_post_work"},
     {ERR_FUNC(SSL_F_TLS_COLLECT_EXTENSIONS), "tls_collect_extensions"},
+    {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES),
+     "tls_construct_certificate_authorities"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST),
      "tls_construct_certificate_request"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERT_STATUS), "tls_construct_cert_status"},
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 9913548..f6eb03f 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1808,6 +1808,7 @@ typedef enum tlsext_index_en {
     TLSEXT_IDX_cookie,
     TLSEXT_IDX_cryptopro_bug,
     TLSEXT_IDX_early_data,
+    TLSEXT_IDX_certificate_authorities,
     TLSEXT_IDX_padding,
     TLSEXT_IDX_psk
 } TLSEXT_INDEX;
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index d62c5af..d0b15d5 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -30,6 +30,13 @@ static int init_npn(SSL *s, unsigned int context);
 static int init_alpn(SSL *s, unsigned int context);
 static int final_alpn(SSL *s, unsigned int context, int sent, int *al);
 static int init_sig_algs(SSL *s, unsigned int context);
+static int init_certificate_authorities(SSL *s, unsigned int context);
+static int tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
+                                                 unsigned int context, X509 *x,
+                                                 size_t chainidx, int *al);
+static int tls_parse_certificate_authorities(SSL *s, PACKET *pkt,
+                                             unsigned int context, X509 *x,
+                                             size_t chainidx, int *al);
 #ifndef OPENSSL_NO_SRP
 static int init_srp(SSL *s, unsigned int context);
 #endif
@@ -159,8 +166,9 @@ static const EXTENSION_DEFINITION ext_defs[] = {
     },
     {
         TLSEXT_TYPE_signature_algorithms,
-        EXT_CLIENT_HELLO,
-        init_sig_algs, tls_parse_ctos_sig_algs, NULL, NULL,
+        EXT_CLIENT_HELLO | EXT_TLS1_3_CERTIFICATE_REQUEST,
+        init_sig_algs, tls_parse_ctos_sig_algs,
+        tls_parse_ctos_sig_algs, tls_construct_ctos_sig_algs,
         tls_construct_ctos_sig_algs, final_sig_algs
     },
 #ifndef OPENSSL_NO_OCSP
@@ -288,6 +296,14 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         final_early_data
     },
     {
+        TLSEXT_TYPE_certificate_authorities,
+        EXT_CLIENT_HELLO | EXT_TLS1_3_CERTIFICATE_REQUEST | EXT_TLS1_3_ONLY,
+        init_certificate_authorities,
+        tls_parse_certificate_authorities, tls_parse_certificate_authorities,
+        tls_construct_certificate_authorities,
+        tls_construct_certificate_authorities, NULL,
+    },
+    {
         /* Must be immediately before pre_shared_key */
         TLSEXT_TYPE_padding,
         EXT_CLIENT_HELLO,
@@ -965,6 +981,47 @@ static int final_ems(SSL *s, unsigned int context, int sent, int *al)
     return 1;
 }
 
+static int init_certificate_authorities(SSL *s, unsigned int context)
+{
+    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
+    s->s3->tmp.ca_names = NULL;
+    return 1;
+}
+
+static int tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
+                                                 unsigned int context, X509 *x,
+                                                 size_t chainidx, int *al)
+{
+    STACK_OF(X509_NAME) *ca_sk = SSL_get_client_CA_list(s);
+
+    if (ca_sk == NULL || sk_X509_NAME_num(ca_sk) == 0)
+        return 1;
+
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_certificate_authorities)
+        || !WPACKET_start_sub_packet_u16(pkt)
+        || !construct_ca_names(s, pkt)
+        || !WPACKET_close(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES,
+               ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
+}
+
+static int tls_parse_certificate_authorities(SSL *s, PACKET *pkt,
+                                             unsigned int context, X509 *x,
+                                             size_t chainidx, int *al)
+{
+    if (!parse_ca_names(s, pkt, al))
+        return 0;
+    if (PACKET_remaining(pkt) != 0) {
+        *al = SSL_AD_DECODE_ERROR;
+        return 0;
+    }
+    return 1;
+}
+
 #ifndef OPENSSL_NO_SRTP
 static int init_srtp(SSL *s, unsigned int context)
 {
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index d153afe..d584bd7 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -65,7 +65,6 @@ static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt);
 
 static ossl_inline int cert_req_allowed(SSL *s);
 static int key_exchange_expected(SSL *s);
-static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b);
 static int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk,
                                     WPACKET *pkt);
 
@@ -2328,149 +2327,107 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
 MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
 {
     int ret = MSG_PROCESS_ERROR;
-    unsigned int i, name_len;
-    X509_NAME *xn = NULL;
-    const unsigned char *namestart, *namebytes;
-    STACK_OF(X509_NAME) *ca_sk = NULL;
-    PACKET cadns;
-
-    if ((ca_sk = sk_X509_NAME_new(ca_dn_cmp)) == NULL) {
-        SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
+    int al = SSL_AD_DECODE_ERROR;
+    size_t i;
+
+    /* Clear certificate validity flags */
+    for (i = 0; i < SSL_PKEY_NUM; i++)
+        s->s3->tmp.valid_flags[i] = 0;
 
     if (SSL_IS_TLS13(s)) {
-        PACKET reqctx;
+        PACKET reqctx, extensions;
+        RAW_EXTENSION *rawexts = NULL;
 
         /* Free and zero certificate types: it is not present in TLS 1.3 */
         OPENSSL_free(s->s3->tmp.ctype);
         s->s3->tmp.ctype = NULL;
         s->s3->tmp.ctype_len = 0;
+
         /* TODO(TLS1.3) need to process request context, for now ignore */
         if (!PACKET_get_length_prefixed_1(pkt, &reqctx)) {
             SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
                    SSL_R_LENGTH_MISMATCH);
             goto err;
         }
-    } else {
-        PACKET ctypes;
-
-        /* get the certificate types */
-        if (!PACKET_get_length_prefixed_1(pkt, &ctypes)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
-                   SSL_R_LENGTH_MISMATCH);
-            goto err;
-        }
-
-        if (!PACKET_memdup(&ctypes, &s->s3->tmp.ctype, &s->s3->tmp.ctype_len)) {
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
-            goto err;
-        }
-    }
 
-    if (SSL_USE_SIGALGS(s)) {
-        PACKET sigalgs;
-
-        if (!PACKET_get_length_prefixed_2(pkt, &sigalgs)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
-                   SSL_R_LENGTH_MISMATCH);
-            goto err;
+        if (!PACKET_get_length_prefixed_2(pkt, &extensions)) {
+                SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, SSL_R_BAD_LENGTH);
+                goto err;
         }
-
-        /* Clear certificate validity flags */
-        for (i = 0; i < SSL_PKEY_NUM; i++)
-            s->s3->tmp.valid_flags[i] = 0;
-        if (!tls1_save_sigalgs(s, &sigalgs)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
-                   SSL_R_SIGNATURE_ALGORITHMS_ERROR);
+        if (!tls_collect_extensions(s, &extensions,
+                                    EXT_TLS1_3_CERTIFICATE_REQUEST,
+                                    &rawexts, &al, NULL)
+            || !tls_parse_all_extensions(s, EXT_TLS1_3_CERTIFICATE_REQUEST,
+                                         rawexts, NULL, 0, &al)) {
+            OPENSSL_free(rawexts);
             goto err;
         }
+        OPENSSL_free(rawexts);
         if (!tls1_process_sigalgs(s)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+            al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
             goto err;
         }
-    }
-
-    /* get the CA RDNs */
-    if (!PACKET_get_length_prefixed_2(pkt, &cadns)) {
-        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-        SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, SSL_R_LENGTH_MISMATCH);
-        goto err;
-    }
+    } else {
+        PACKET ctypes;
 
-    while (PACKET_remaining(&cadns)) {
-        if (!PACKET_get_net_2(&cadns, &name_len)
-            || !PACKET_get_bytes(&cadns, &namebytes, name_len)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+        /* get the certificate types */
+        if (!PACKET_get_length_prefixed_1(pkt, &ctypes)) {
             SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
                    SSL_R_LENGTH_MISMATCH);
             goto err;
         }
 
-        namestart = namebytes;
-
-        if ((xn = d2i_X509_NAME(NULL, (const unsigned char **)&namebytes,
-                                name_len)) == NULL) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_ASN1_LIB);
+        if (!PACKET_memdup(&ctypes, &s->s3->tmp.ctype, &s->s3->tmp.ctype_len)) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
             goto err;
         }
 
-        if (namebytes != (namestart + name_len)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
-                   SSL_R_CA_DN_LENGTH_MISMATCH);
-            goto err;
-        }
-        if (!sk_X509_NAME_push(ca_sk, xn)) {
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
-            goto err;
+        if (SSL_USE_SIGALGS(s)) {
+            PACKET sigalgs;
+
+            if (!PACKET_get_length_prefixed_2(pkt, &sigalgs)) {
+                SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
+                       SSL_R_LENGTH_MISMATCH);
+                goto err;
+            }
+
+            if (!tls1_save_sigalgs(s, &sigalgs)) {
+                SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
+                       SSL_R_SIGNATURE_ALGORITHMS_ERROR);
+                goto err;
+            }
+            if (!tls1_process_sigalgs(s)) {
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
+                       ERR_R_MALLOC_FAILURE);
+                goto err;
+            }
         }
-        xn = NULL;
-    }
-    /* TODO(TLS1.3) need to parse and process extensions, for now ignore */
-    if (SSL_IS_TLS13(s)) {
-        PACKET reqexts;
 
-        if (!PACKET_get_length_prefixed_2(pkt, &reqexts)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
-                   SSL_R_EXT_LENGTH_MISMATCH);
+        /* get the CA RDNs */
+        if (!parse_ca_names(s, pkt, &al))
             goto err;
-        }
     }
 
     if (PACKET_remaining(pkt) != 0) {
-        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
         SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, SSL_R_LENGTH_MISMATCH);
         goto err;
     }
 
     /* we should setup a certificate to return.... */
     s->s3->tmp.cert_req = 1;
-    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
-    s->s3->tmp.ca_names = ca_sk;
-    ca_sk = NULL;
 
     ret = MSG_PROCESS_CONTINUE_PROCESSING;
     goto done;
  err:
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
     ossl_statem_set_error(s);
  done:
-    X509_NAME_free(xn);
-    sk_X509_NAME_pop_free(ca_sk, X509_NAME_free);
     return ret;
 }
 
-static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b)
-{
-    return (X509_NAME_cmp(*a, *b));
-}
-
 MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
 {
     int al = SSL_AD_DECODE_ERROR;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 5164cc0..849310e 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1907,3 +1907,98 @@ int create_synthetic_message_hash(SSL *s)
 
     return 1;
 }
+
+static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b)
+{
+    return X509_NAME_cmp(*a, *b);
+}
+
+int parse_ca_names(SSL *s, PACKET *pkt, int *al)
+{
+    STACK_OF(X509_NAME) *ca_sk = sk_X509_NAME_new(ca_dn_cmp);
+    X509_NAME *xn = NULL;
+    PACKET cadns;
+
+    if (ca_sk == NULL) {
+        SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_MALLOC_FAILURE);
+        goto decerr;
+    }
+    /* get the CA RDNs */
+    if (!PACKET_get_length_prefixed_2(pkt, &cadns)) {
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_LENGTH_MISMATCH);
+        goto decerr;
+    }
+
+    while (PACKET_remaining(&cadns)) {
+        const unsigned char *namestart, *namebytes;
+        unsigned int name_len;
+
+        if (!PACKET_get_net_2(&cadns, &name_len)
+            || !PACKET_get_bytes(&cadns, &namebytes, name_len)) {
+            SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_LENGTH_MISMATCH);
+            goto decerr;
+        }
+
+        namestart = namebytes;
+        if ((xn = d2i_X509_NAME(NULL, &namebytes, name_len)) == NULL) {
+            SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_ASN1_LIB);
+            goto decerr;
+        }
+        if (namebytes != (namestart + name_len)) {
+            SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_CA_DN_LENGTH_MISMATCH);
+            goto decerr;
+        }
+
+        if (!sk_X509_NAME_push(ca_sk, xn)) {
+            SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_MALLOC_FAILURE);
+            *al = SSL_AD_INTERNAL_ERROR;
+            goto err;
+        }
+        xn = NULL;
+    }
+
+    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
+    s->s3->tmp.ca_names = ca_sk;
+
+    return 1;
+
+ decerr:
+    *al = SSL_AD_DECODE_ERROR;
+ err:
+    sk_X509_NAME_pop_free(ca_sk, X509_NAME_free);
+    X509_NAME_free(xn);
+    return 0;
+}
+
+int construct_ca_names(SSL *s, WPACKET *pkt)
+{
+    STACK_OF(X509_NAME) *ca_sk = SSL_get_client_CA_list(s);
+
+    /* Start sub-packet for client CA list */
+    if (!WPACKET_start_sub_packet_u16(pkt))
+        return 0;
+
+    if (ca_sk != NULL) {
+        int i;
+
+        for (i = 0; i < sk_X509_NAME_num(ca_sk); i++) {
+            unsigned char *namebytes;
+            X509_NAME *name = sk_X509_NAME_value(ca_sk, i);
+            int namelen;
+
+            if (name == NULL
+                    || (namelen = i2d_X509_NAME(name, NULL)) < 0
+                    || !WPACKET_sub_allocate_bytes_u16(pkt, namelen,
+                                                       &namebytes)
+                    || i2d_X509_NAME(name, &namebytes) != namelen) {
+                return 0;
+            }
+        }
+    }
+
+    if (!WPACKET_close(pkt))
+        return 0;
+
+    return 1;
+}
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 1c78a4d..9bf1d8a 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -53,6 +53,7 @@
 #define EXT_TLS1_3_HELLO_RETRY_REQUEST      0x0400
 #define EXT_TLS1_3_CERTIFICATE              0x0800
 #define EXT_TLS1_3_NEW_SESSION_TICKET       0x1000
+#define EXT_TLS1_3_CERTIFICATE_REQUEST      0x2000
 
 /* Dummy message type */
 #define SSL3_MT_DUMMY   -1
@@ -80,6 +81,9 @@ typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
 int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
                   size_t num_groups, int checkallow);
 int create_synthetic_message_hash(SSL *s);
+int parse_ca_names(SSL *s, PACKET *pkt, int *al);
+int construct_ca_names(SSL *s, WPACKET *pkt);
+
 /*
  * TLS/DTLS client state machine functions
  */
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 78f977f..ffb0685 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2499,8 +2499,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
 
 int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
 {
-    int i;
-    STACK_OF(X509_NAME) *sk = NULL;
+    int al = SSL_AD_INTERNAL_ERROR;
 
     if (SSL_IS_TLS13(s)) {
         /* TODO(TLS1.3) for now send empty request context */
@@ -2509,14 +2508,21 @@ int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
                    ERR_R_INTERNAL_ERROR);
             goto err;
         }
-    } else {
-        /* get the list of acceptable cert types */
-        if (!WPACKET_start_sub_packet_u8(pkt)
-            || !ssl3_get_req_cert_type(s, pkt) || !WPACKET_close(pkt)) {
+
+        if (!tls_construct_extensions(s, pkt, EXT_TLS1_3_CERTIFICATE_REQUEST,
+                                      NULL, 0, &al)) {
             SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST,
                    ERR_R_INTERNAL_ERROR);
             goto err;
         }
+        goto done;
+    }
+
+    /* get the list of acceptable cert types */
+    if (!WPACKET_start_sub_packet_u8(pkt)
+        || !ssl3_get_req_cert_type(s, pkt) || !WPACKET_close(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
+        goto err;
     }
 
     if (SSL_USE_SIGALGS(s)) {
@@ -2533,49 +2539,16 @@ int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
         }
     }
 
-    /* Start sub-packet for client CA list */
-    if (!WPACKET_start_sub_packet_u16(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    sk = SSL_get_client_CA_list(s);
-    if (sk != NULL) {
-        for (i = 0; i < sk_X509_NAME_num(sk); i++) {
-            unsigned char *namebytes;
-            X509_NAME *name = sk_X509_NAME_value(sk, i);
-            int namelen;
-
-            if (name == NULL
-                    || (namelen = i2d_X509_NAME(name, NULL)) < 0
-                    || !WPACKET_sub_allocate_bytes_u16(pkt, namelen,
-                                                       &namebytes)
-                    || i2d_X509_NAME(name, &namebytes) != namelen) {
-                SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST,
-                       ERR_R_INTERNAL_ERROR);
-                goto err;
-            }
-        }
-    }
-    /* else no CA names */
-    if (!WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-    /*
-     * TODO(TLS1.3) implement configurable certificate_extensions
-     * For now just send zero length extensions.
-     */
-    if (SSL_IS_TLS13(s) && !WPACKET_put_bytes_u16(pkt, 0)) {
+    if (!construct_ca_names(s, pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
         goto err;
     }
 
+ done:
     s->s3->tmp.cert_request = 1;
-
     return 1;
  err:
-    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
     return 0;
 }
 


More information about the openssl-commits mailing list