Push the difference in chain semantics to the edge.
OpenSSL includes a leaf certificate in a certificate chain when it's a
client, but doesn't when it's a server. This is also reflected in the
serialisation of sessions.
This change makes the internal semantics consistent: the leaf is always
included in the chain in memory, and never duplicated when serialised.
To maintain the same API, SSL_get_peer_cert_chain will construct a copy
of the chain without the leaf if needed.
Since the serialised format of a client session has changed, an
|is_server| boolean is added to the ASN.1 that defaults to true. Thus
any old client sessions will be parsed as server sessions and (silently)
discarded by a client.
Change-Id: Ibcf72bc8a130cedb423bc0fd3417868e0af3ca3e
Reviewed-on: https://boringssl-review.googlesource.com/12704
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index aab6052..67d9df8 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -122,6 +122,7 @@
* keyExchangeInfo [18] INTEGER OPTIONAL,
* certChain [19] SEQUENCE OF Certificate OPTIONAL,
* ticketAgeAdd [21] OCTET STRING OPTIONAL,
+ * isServer [22] BOOLEAN DEFAULT TRUE,
* }
*
* Note: historically this serialization has included other optional
@@ -170,6 +171,8 @@
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 19;
static const int kTicketAgeAddTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 21;
+static const int kIsServerTag =
+ CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 22;
static int SSL_SESSION_to_bytes_full(const SSL_SESSION *in, uint8_t **out_data,
size_t *out_len, int for_ticket) {
@@ -340,12 +343,13 @@
/* The certificate chain is only serialized if the leaf's SHA-256 isn't
* serialized instead. */
- if (in->x509_chain != NULL && !in->peer_sha256_valid) {
+ if (in->x509_chain != NULL && !in->peer_sha256_valid &&
+ sk_X509_num(in->x509_chain) >= 2) {
if (!CBB_add_asn1(&session, &child, kCertChainTag)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
goto err;
}
- for (size_t i = 0; i < sk_X509_num(in->x509_chain); i++) {
+ for (size_t i = 1; i < sk_X509_num(in->x509_chain); i++) {
if (!ssl_add_cert_to_cbb(&child, sk_X509_value(in->x509_chain, i))) {
goto err;
}
@@ -361,6 +365,15 @@
}
}
+ if (!in->is_server) {
+ if (!CBB_add_asn1(&session, &child, kIsServerTag) ||
+ !CBB_add_asn1(&child, &child2, CBS_ASN1_BOOLEAN) ||
+ !CBB_add_u8(&child2, 0x00)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ goto err;
+ }
+ }
+
if (!CBB_finish(&cbb, out_data, out_len)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
goto err;
@@ -658,8 +671,20 @@
}
sk_X509_pop_free(ret->x509_chain, X509_free);
ret->x509_chain = NULL;
- if (has_cert_chain) {
+ if (ret->x509_peer != NULL) {
ret->x509_chain = sk_X509_new_null();
+ if (ret->x509_chain == NULL ||
+ !sk_X509_push(ret->x509_chain, ret->x509_peer)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ goto err;
+ }
+ X509_up_ref(ret->x509_peer);
+ }
+ if (has_cert_chain) {
+ if (ret->x509_peer == NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+ goto err;
+ }
if (ret->x509_chain == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
goto err;
@@ -688,6 +713,17 @@
}
ret->ticket_age_add_valid = age_add_present;
+ int is_server;
+ if (!CBS_get_optional_asn1_bool(&session, &is_server, kIsServerTag,
+ 1 /* default to true */)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+ goto err;
+ }
+ /* TODO: in time we can include |is_server| for servers too, then we can
+ enforce that client and server sessions are never mixed up. */
+
+ ret->is_server = is_server;
+
if (CBS_len(&session) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
goto err;