Preserve the peer signature algorithm across resumes.
So we can report it cleanly out of DevTools, it should behave like
SSL_get_curve_id and be reported on resumption too.
BUG=chromium:658905
Change-Id: I0402e540a1e722e09eaebadf7fb4785d8880c389
Reviewed-on: https://boringssl-review.googlesource.com/12694
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 9410fdd..aad0de2 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1287,7 +1287,7 @@
if (!tls12_check_peer_sigalg(ssl, &al, signature_algorithm)) {
goto f_err;
}
- ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
+ ssl->s3->new_session->peer_signature_algorithm = signature_algorithm;
} else if (hs->peer_pubkey->type == EVP_PKEY_RSA) {
signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
} else if (hs->peer_pubkey->type == EVP_PKEY_EC) {
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index d9234af..d41685e 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1826,7 +1826,7 @@
if (!tls12_check_peer_sigalg(ssl, &al, signature_algorithm)) {
goto f_err;
}
- ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
+ ssl->s3->new_session->peer_signature_algorithm = signature_algorithm;
} else if (hs->peer_pubkey->type == EVP_PKEY_RSA) {
signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
} else if (hs->peer_pubkey->type == EVP_PKEY_EC) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 98c5cdf..76e37b4 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1531,10 +1531,6 @@
* messages, but it doesn't matter if the session that's being resumed
* didn't use it to create the master secret initially. */
char extended_master_secret;
-
- /* peer_signature_algorithm is the signature algorithm used to authenticate
- * the peer, or zero if not applicable. */
- uint16_t peer_signature_algorithm;
} tmp;
/* new_session is the new mutable session being established by the current
diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index defe2cc..902b580 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -126,6 +126,7 @@
* certChain [19] SEQUENCE OF Certificate OPTIONAL,
* ticketAgeAdd [21] OCTET STRING OPTIONAL,
* isServer [22] BOOLEAN DEFAULT TRUE,
+ * peerSignatureAlgorithm [23] INTEGER OPTIONAL,
* }
*
* Note: historically this serialization has included other optional
@@ -176,6 +177,8 @@
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 21;
static const int kIsServerTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 22;
+static const int kPeerSignatureAlgorithmTag =
+ CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 23;
static int SSL_SESSION_to_bytes_full(const SSL_SESSION *in, uint8_t **out_data,
size_t *out_len, int for_ticket) {
@@ -381,6 +384,13 @@
}
}
+ if (in->peer_signature_algorithm != 0 &&
+ (!CBB_add_asn1(&session, &child, kPeerSignatureAlgorithmTag) ||
+ !CBB_add_asn1_uint64(&child, in->peer_signature_algorithm))) {
+ 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;
@@ -531,6 +541,19 @@
return 1;
}
+static int SSL_SESSION_parse_u16(CBS *cbs, uint16_t *out, unsigned tag,
+ uint16_t default_value) {
+ uint64_t value;
+ if (!CBS_get_optional_asn1_uint64(cbs, &value, tag,
+ (uint64_t)default_value) ||
+ value > 0xffff) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+ return 0;
+ }
+ *out = (uint16_t)value;
+ return 1;
+}
+
static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
SSL_SESSION *ret = SSL_SESSION_new();
if (ret == NULL) {
@@ -748,7 +771,9 @@
ret->is_server = is_server;
- if (CBS_len(&session) != 0) {
+ if (!SSL_SESSION_parse_u16(&session, &ret->peer_signature_algorithm,
+ kPeerSignatureAlgorithmTag, 0) ||
+ CBS_len(&session) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
goto err;
}
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 8410a30..078b62b 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2843,7 +2843,14 @@
}
uint16_t SSL_get_peer_signature_algorithm(const SSL *ssl) {
- return ssl->s3->tmp.peer_signature_algorithm;
+ /* TODO(davidben): This checks the wrong session if there is a renegotiation
+ * in progress. */
+ SSL_SESSION *session = SSL_get_session(ssl);
+ if (session == NULL) {
+ return 0;
+ }
+
+ return session->peer_signature_algorithm;
}
size_t SSL_get_client_random(const SSL *ssl, uint8_t *out, size_t max_out) {
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index f99ba5b..d6d7dab 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -255,6 +255,8 @@
}
}
+ new_session->peer_signature_algorithm = session->peer_signature_algorithm;
+
new_session->timeout = session->timeout;
new_session->time = session->time;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 87a79a3..5e930ce 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -6546,6 +6546,9 @@
"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
"-enable-all-curves",
},
+ // Resume the session to assert the peer signature
+ // algorithm is reported on both handshakes.
+ resumeSession: !shouldVerifyFail,
shouldFail: shouldVerifyFail,
expectedError: verifyError,
})
@@ -6592,6 +6595,9 @@
"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
"-enable-all-curves",
},
+ // Resume the session to assert the peer signature
+ // algorithm is reported on both handshakes.
+ resumeSession: !shouldVerifyFail,
shouldFail: shouldVerifyFail,
expectedError: verifyError,
})
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index dd4a041..25dc852 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -358,7 +358,7 @@
ssl3_send_alert(ssl, SSL3_AL_FATAL, al);
goto err;
}
- ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
+ ssl->s3->new_session->peer_signature_algorithm = signature_algorithm;
if (!tls13_get_cert_verify_signature_input(
ssl, &msg, &msg_len,
