commit 364f7a6d21f73d099f4f80800973174a8f71e90f
author Adam Langley <agl@google.com> Mon Dec 12 10:51:00 2016 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Dec 13 17:18:53 2016 +0000
tree 7379dd7f90e0594ce3992d7cd5a0e3be7288e663
parent cb0c29ff75045fb9f7c2f9d827da4492a1840ba8

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>

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 5121f99..cca9bd6 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1483,6 +1483,8 @@
     goto err;
   }
 
+  X509 *leaf = NULL;
+
   if (sk_X509_num(chain) == 0) {
     /* No client certificate so the handshake buffer may be discarded. */
     ssl3_free_handshake_buffer(ssl);
@@ -1506,6 +1508,7 @@
      * classed by them as a bug, but it's assumed by at least NGINX. */
     ssl->s3->new_session->verify_result = X509_V_OK;
   } else {
+    leaf = sk_X509_value(chain, 0);
     /* The hash would have been filled in. */
     if (ssl->retain_only_sha256_of_client_certs) {
       ssl->s3->new_session->peer_sha256_valid = 1;
@@ -1517,13 +1520,16 @@
     }
   }
 
-  X509_free(ssl->s3->new_session->x509_peer);
-  ssl->s3->new_session->x509_peer = sk_X509_shift(chain);
-
   sk_X509_pop_free(ssl->s3->new_session->x509_chain, X509_free);
   ssl->s3->new_session->x509_chain = chain;
-  /* Inconsistency alert: x509_chain does *not* include the peer's own
-   * certificate, while we do include it in s3_clnt.c */
+  sk_X509_pop_free(ssl->s3->new_session->x509_chain_without_leaf, X509_free);
+  ssl->s3->new_session->x509_chain_without_leaf = NULL;
+
+  X509_free(ssl->s3->new_session->x509_peer);
+  if (leaf) {
+    X509_up_ref(leaf);
+  }
+  ssl->s3->new_session->x509_peer = leaf;
 
   return 1;