Remove the last of SESS_CERT.
Move cert_chain to the SSL_SESSION. Now everything on an SSL_SESSION is
properly serialized. The cert_chain field is, unfortunately, messed up
since it means different things between client and server.
There exists code which calls SSL_get_peer_cert_chain as both client and
server and assumes the existing semantics for each. Since that function
doesn't return a newly-allocated STACK_OF(X509), normalizing between the
two formats is a nuisance (we'd either need to store both cert_chain and
cert_chain_full on the SSL_SESSION or create one of the two variants
on-demand and stash it into the SSL).
This CL does not resolve this and retains the client/server difference
in SSL_SESSION. The SSL_SESSION serialization is a little inefficient
(two copies of the leaf certificate) for a client, but clients don't
typically serialize sessions. Should we wish to resolve it in the
future, we can use a different tag number. Because this was historically
unserialized, existing code must already allow for cert_chain not being
preserved across i2d/d2i.
In keeping with the semantics of retain_only_sha256_of_client_certs,
cert_chain is not retained when that flag is set.
Change-Id: Ieb72fc62c3076dd59750219e550902f1ad039651
Reviewed-on: https://boringssl-review.googlesource.com/5759
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 576a861..485c29f 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -950,7 +950,6 @@
unsigned long n;
X509 *x = NULL;
STACK_OF(X509) *sk = NULL;
- SESS_CERT *sc;
EVP_PKEY *pkey = NULL;
CBS cbs, certificate_list;
const uint8_t *data;
@@ -1019,17 +1018,10 @@
goto f_err;
}
- sc = ssl_sess_cert_new();
- if (sc == NULL) {
- goto err;
- }
-
- ssl_sess_cert_free(s->session->sess_cert);
- s->session->sess_cert = sc;
-
/* NOTE: Unlike the server half, the client's copy of |cert_chain| includes
* the leaf. */
- sc->cert_chain = sk;
+ sk_X509_pop_free(s->session->cert_chain, X509_free);
+ s->session->cert_chain = sk;
sk = NULL;
X509_free(s->session->peer);
@@ -1080,19 +1072,9 @@
return -1;
}
- /* In plain PSK ciphersuite, ServerKeyExchange can be
- omitted if no identity hint is sent. Set session->sess_cert anyway to
- avoid problems later.*/
+ /* In plain PSK ciphersuite, ServerKeyExchange may be omitted to send no
+ * identity hint. */
if (s->s3->tmp.new_cipher->algorithm_auth & SSL_aPSK) {
- /* PSK ciphersuites that also send a Certificate would have already
- * initialized |sess_cert|. */
- if (s->session->sess_cert == NULL) {
- s->session->sess_cert = ssl_sess_cert_new();
- if (s->session->sess_cert == NULL) {
- return -1;
- }
- }
-
/* TODO(davidben): This should be reset in one place with the rest of the
* handshake state. */
OPENSSL_free(s->s3->tmp.peer_psk_identity_hint);
@@ -1106,13 +1088,6 @@
CBS_init(&server_key_exchange, s->init_msg, n);
server_key_exchange_orig = server_key_exchange;
- if (s->session->sess_cert == NULL) {
- s->session->sess_cert = ssl_sess_cert_new();
- if (s->session->sess_cert == NULL) {
- return -1;
- }
- }
-
alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
alg_a = s->s3->tmp.new_cipher->algorithm_auth;
EVP_MD_CTX_init(&md_ctx);
@@ -1482,15 +1457,6 @@
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
goto err;
}
- if (s->session->sess_cert != NULL) {
- /* |sess_cert| is not serialized and must be duplicated explicitly. */
- assert(new_session->sess_cert == NULL);
- new_session->sess_cert = ssl_sess_cert_dup(s->session->sess_cert);
- if (new_session->sess_cert == NULL) {
- SSL_SESSION_free(new_session);
- goto err;
- }
- }
SSL_SESSION_free(s->session);
s->session = new_session;
@@ -1677,12 +1643,6 @@
goto err;
}
- if (s->session->sess_cert == NULL) {
- /* We should always have a server certificate with SSL_kRSA. */
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- goto err;
- }
-
pkey = X509_get_pubkey(s->session->peer);
if (pkey == NULL ||
pkey->type != EVP_PKEY_RSA ||