Get rid of CERT_PKEY slots in SESS_CERT.
This doesn't even change behavior. Unlike local configuration, the peer
can never have multiple certificates anyway. (Even with a renego, the
SESS_CERT is created anew.)
This does lose the implicit certificate type check, but the certificate
type is already checked in ssl3_get_server_certificate and later checked
post-facto in ssl3_check_cert_and_algorithm (except that one seems to
have some bugs like it accepts ECDSA certificates for RSA cipher suites,
to be cleaned up in a follow-up). Either way, we have the certificate
mismatch tests for this.
BUG=486295
Change-Id: I437bb723bb310ad54ee4150eda67c1cfe43377b3
Reviewed-on: https://boringssl-review.googlesource.com/5044
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h
index 7d9a5ad..0f40122 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -581,15 +581,13 @@
} CERT;
typedef struct sess_cert_st {
- STACK_OF(X509) *cert_chain; /* as received from peer (not for SSL2) */
+ /* cert_chain is the certificate chain sent by the peer. NOTE: for a client,
+ * this does includes the server's leaf certificate, but, for a server, this
+ * does NOT include the client's leaf. */
+ STACK_OF(X509) *cert_chain;
- /* The 'peer_...' members are used only by clients. */
- int peer_cert_type;
-
- CERT_PKEY *peer_key; /* points to an element of peer_pkeys (never NULL!) */
- CERT_PKEY peer_pkeys[SSL_PKEY_NUM];
- /* Obviously we don't have the private keys of these,
- * so maybe we shouldn't even use the CERT_PKEY type here. */
+ /* peer_cert, on a client, is the leaf certificate of the peer. */
+ X509 *peer_cert;
DH *peer_dh_tmp;
EC_KEY *peer_ecdh_tmp;
@@ -800,7 +798,6 @@
void ssl_cert_free(CERT *c);
SESS_CERT *ssl_sess_cert_new(void);
void ssl_sess_cert_free(SESS_CERT *sc);
-int ssl_set_peer_cert_type(SESS_CERT *c, int type);
int ssl_get_new_session(SSL *s, int session);
int ssl_get_prev_session(SSL *s, const struct ssl_early_callback_ctx *ctx);
STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, const CBS *cbs);
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 159e2d7..d7ed14f 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1028,10 +1028,8 @@
SSL_R_WRONG_CERTIFICATE_TYPE);
goto f_err;
}
- sc->peer_cert_type = i;
- X509_free(sc->peer_pkeys[i].x509);
- sc->peer_pkeys[i].x509 = X509_up_ref(x);
- sc->peer_key = &(sc->peer_pkeys[i]);
+ X509_free(sc->peer_cert);
+ sc->peer_cert = X509_up_ref(x);
X509_free(s->session->peer);
s->session->peer = X509_up_ref(x);
@@ -1195,8 +1193,7 @@
}
if (alg_a & SSL_aRSA) {
- pkey = X509_get_pubkey(
- s->session->sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
+ pkey = X509_get_pubkey(s->session->sess_cert->peer_cert);
}
/* else anonymous DH, so no certificate or pkey. */
@@ -1258,14 +1255,12 @@
/* The ECC/TLS specification does not mention the use of DSA to sign
* ECParameters in the server key exchange message. We do support RSA and
* ECDSA. */
- if (alg_a & SSL_aRSA) {
- pkey = X509_get_pubkey(
- s->session->sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
- } else if (alg_a & SSL_aECDSA) {
- pkey =
- X509_get_pubkey(s->session->sess_cert->peer_pkeys[SSL_PKEY_ECC].x509);
+ if (alg_a & (SSL_aRSA|SSL_aECDSA)) {
+ pkey = X509_get_pubkey(s->session->sess_cert->peer_cert);
+ } else {
+ /* Otherwise this is ECDHE_PSK, so no public key. */
+ assert(alg_a == SSL_aPSK);
}
- /* else anonymous ECDH, so no certificate or pkey. */
EC_KEY_set_public_key(ecdh, srvr_ecpoint);
s->session->sess_cert->peer_ecdh_tmp = ecdh;
ecdh = NULL;
@@ -1702,8 +1697,7 @@
goto err;
}
- pkey = X509_get_pubkey(
- s->session->sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
+ pkey = X509_get_pubkey(s->session->sess_cert->peer_cert);
if (pkey == NULL ||
pkey->type != EVP_PKEY_RSA ||
pkey->pkey.rsa == NULL) {
@@ -2159,9 +2153,7 @@
#define has_bits(i, m) (((i) & (m)) == (m))
int ssl3_check_cert_and_algorithm(SSL *s) {
- int i, idx;
long alg_k, alg_a;
- EVP_PKEY *pkey = NULL;
SESS_CERT *sc;
DH *dh;
@@ -2181,11 +2173,9 @@
dh = s->session->sess_cert->peer_dh_tmp;
- /* This is the passed certificate */
-
- idx = sc->peer_cert_type;
- if (idx == SSL_PKEY_ECC) {
- if (ssl_check_srvr_ecc_cert_and_alg(sc->peer_pkeys[idx].x509, s) == 0) {
+ int cert_type = X509_certificate_type(sc->peer_cert, NULL);
+ if (cert_type & EVP_PK_EC) {
+ if (ssl_check_srvr_ecc_cert_and_alg(sc->peer_cert, s) == 0) {
/* check failed */
OPENSSL_PUT_ERROR(SSL, ssl3_check_cert_and_algorithm, SSL_R_BAD_ECC_CERT);
goto f_err;
@@ -2197,25 +2187,21 @@
SSL_R_MISSING_ECDSA_SIGNING_CERT);
goto f_err;
}
- pkey = X509_get_pubkey(sc->peer_pkeys[idx].x509);
- i = X509_certificate_type(sc->peer_pkeys[idx].x509, pkey);
- EVP_PKEY_free(pkey);
/* Check that we have a certificate if we require one */
- if ((alg_a & SSL_aRSA) && !has_bits(i, EVP_PK_RSA | EVP_PKT_SIGN)) {
+ if ((alg_a & SSL_aRSA) && !has_bits(cert_type, EVP_PK_RSA | EVP_PKT_SIGN)) {
OPENSSL_PUT_ERROR(SSL, ssl3_check_cert_and_algorithm,
SSL_R_MISSING_RSA_SIGNING_CERT);
goto f_err;
}
- if ((alg_k & SSL_kRSA) && !has_bits(i, EVP_PK_RSA | EVP_PKT_ENC)) {
+ if ((alg_k & SSL_kRSA) && !has_bits(cert_type, EVP_PK_RSA | EVP_PKT_ENC)) {
OPENSSL_PUT_ERROR(SSL, ssl3_check_cert_and_algorithm,
SSL_R_MISSING_RSA_ENCRYPTING_CERT);
goto f_err;
}
- if ((alg_k & SSL_kDHE) &&
- !(has_bits(i, EVP_PK_DH | EVP_PKT_EXCH) || dh != NULL)) {
+ if ((alg_k & SSL_kDHE) && dh == NULL) {
OPENSSL_PUT_ERROR(SSL, ssl3_check_cert_and_algorithm, SSL_R_MISSING_DH_KEY);
goto f_err;
}
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index f1fd675..f27685d 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -409,35 +409,23 @@
}
memset(ret, 0, sizeof *ret);
- ret->peer_key = &(ret->peer_pkeys[SSL_PKEY_RSA_ENC]);
return ret;
}
void ssl_sess_cert_free(SESS_CERT *sc) {
- int i;
-
if (sc == NULL) {
return;
}
sk_X509_pop_free(sc->cert_chain, X509_free);
-
- for (i = 0; i < SSL_PKEY_NUM; i++) {
- X509_free(sc->peer_pkeys[i].x509);
- }
-
+ X509_free(sc->peer_cert);
DH_free(sc->peer_dh_tmp);
EC_KEY_free(sc->peer_ecdh_tmp);
OPENSSL_free(sc);
}
-int ssl_set_peer_cert_type(SESS_CERT *sc, int type) {
- sc->peer_cert_type = type;
- return 1;
-}
-
int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk) {
X509 *x;
int i;
