Move the NULL case in ssl_add_cert_chain up.
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)
Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 0157072..ee7b5a2 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -103,6 +103,7 @@
SSL,function,193,ssl3_get_server_key_exchange
SSL,function,194,ssl3_get_v2_client_hello
SSL,function,195,ssl3_handshake_mac
+SSL,function,275,ssl3_output_cert_chain
SSL,function,196,ssl3_prf
SSL,function,197,ssl3_read_bytes
SSL,function,198,ssl3_read_n
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 4ee2084..12ce73f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2678,6 +2678,7 @@
#define SSL_F_SSL_CTX_set1_tls_channel_id 272
#define SSL_F_SSL_set1_tls_channel_id 273
#define SSL_F_SSL_set_tlsext_host_name 274
+#define SSL_F_ssl3_output_cert_chain 275
#define SSL_R_APP_DATA_IN_HANDSHAKE 100
#define SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT 101
#define SSL_R_BAD_ALERT 102
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 889a732..4dd9e52 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -305,7 +305,13 @@
uint8_t *p;
unsigned long l = 3 + SSL_HM_HEADER_LENGTH(s);
- if (!ssl_add_cert_chain(s, cpk, &l)) {
+ if (cpk == NULL) {
+ /* TLSv1 sends a chain with nothing in it, instead of an alert. */
+ if (!BUF_MEM_grow_clean(s->init_buf, 10)) {
+ OPENSSL_PUT_ERROR(SSL, ssl3_output_cert_chain, ERR_R_BUF_LIB);
+ return 0;
+ }
+ } else if (!ssl_add_cert_chain(s, cpk, &l)) {
return 0;
}
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index fdab572..3e8eaf1 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -792,12 +792,13 @@
int no_chain = 0;
size_t i;
- X509 *x = NULL;
- STACK_OF(X509) * extra_certs;
+ X509 *x = cpk->x509;
+ STACK_OF(X509) *extra_certs;
X509_STORE *chain_store;
- if (cpk) {
- x = cpk->x509;
+ if (x == NULL) {
+ OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, SSL_R_NO_CERTIFICATE_SET);
+ return 0;
}
if (s->cert->chain_store) {
@@ -817,44 +818,36 @@
no_chain = 1;
}
- /* TLSv1 sends a chain with nothing in it, instead of an alert. */
- if (!BUF_MEM_grow_clean(buf, 10)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_BUF_LIB);
- return 0;
- }
-
- if (x != NULL) {
- if (no_chain) {
- if (!ssl_add_cert_to_buf(buf, l, x)) {
- return 0;
- }
- } else {
- X509_STORE_CTX xs_ctx;
-
- if (!X509_STORE_CTX_init(&xs_ctx, chain_store, x, NULL)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_X509_LIB);
- return 0;
- }
- X509_verify_cert(&xs_ctx);
- /* Don't leave errors in the queue */
- ERR_clear_error();
- for (i = 0; i < sk_X509_num(xs_ctx.chain); i++) {
- x = sk_X509_value(xs_ctx.chain, i);
-
- if (!ssl_add_cert_to_buf(buf, l, x)) {
- X509_STORE_CTX_cleanup(&xs_ctx);
- return 0;
- }
- }
- X509_STORE_CTX_cleanup(&xs_ctx);
- }
- }
-
- for (i = 0; i < sk_X509_num(extra_certs); i++) {
- x = sk_X509_value(extra_certs, i);
+ if (no_chain) {
if (!ssl_add_cert_to_buf(buf, l, x)) {
return 0;
}
+
+ for (i = 0; i < sk_X509_num(extra_certs); i++) {
+ x = sk_X509_value(extra_certs, i);
+ if (!ssl_add_cert_to_buf(buf, l, x)) {
+ return 0;
+ }
+ }
+ } else {
+ X509_STORE_CTX xs_ctx;
+
+ if (!X509_STORE_CTX_init(&xs_ctx, chain_store, x, NULL)) {
+ OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_X509_LIB);
+ return 0;
+ }
+ X509_verify_cert(&xs_ctx);
+ /* Don't leave errors in the queue */
+ ERR_clear_error();
+ for (i = 0; i < sk_X509_num(xs_ctx.chain); i++) {
+ x = sk_X509_value(xs_ctx.chain, i);
+
+ if (!ssl_add_cert_to_buf(buf, l, x)) {
+ X509_STORE_CTX_cleanup(&xs_ctx);
+ return 0;
+ }
+ }
+ X509_STORE_CTX_cleanup(&xs_ctx);
}
return 1;
