Raw Public Keys: Determine and send negotiated client_certificate_type This implements the server's logic to receive the client_certificate_type list sent by the client, and determine which type of client certificate to request by intersecting it with its (the server's) list of accepted peer cert types. The server will only request a client certificate if verify_mode is SSL_VERIFY_PEER. If it is, then the negotiated client_certificate_type value determines whether the server requests an X.509 cert or a Raw Public Key. Bug: 467663225 Change-Id: I07b06edd7974e4483a21911becfba0be6a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/89828 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Lily Chen <chlily@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index fe6b26b..e7690cf 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata
@@ -246,6 +246,7 @@ SSL,234,UNKNOWN_SSL_VERSION SSL,235,UNKNOWN_STATE SSL,236,UNSAFE_LEGACY_RENEGOTIATION_DISABLED +SSL,334,UNSUPPORTED_CERTIFICATE SSL,237,UNSUPPORTED_CIPHER SSL,238,UNSUPPORTED_COMPRESSION_ALGORITHM SSL,327,UNSUPPORTED_CREDENTIAL_LIST
diff --git a/gen/crypto/err_data.cc b/gen/crypto/err_data.cc index 1c70e15..a60da8b 100644 --- a/gen/crypto/err_data.cc +++ b/gen/crypto/err_data.cc
@@ -205,51 +205,51 @@ 0x283500f7, 0x28358cc1, 0x2836099a, - 0x2c3234aa, + 0x2c3234c2, 0x2c32943b, - 0x2c3334b8, - 0x2c33b4ca, - 0x2c3434de, - 0x2c34b4f0, - 0x2c35350b, - 0x2c35b51d, - 0x2c36354d, + 0x2c3334d0, + 0x2c33b4e2, + 0x2c3434f6, + 0x2c34b508, + 0x2c353523, + 0x2c35b535, + 0x2c363565, 0x2c36833a, - 0x2c37355a, - 0x2c37b586, - 0x2c3835c4, - 0x2c38b5db, - 0x2c3935f9, - 0x2c39b609, - 0x2c3a361b, - 0x2c3ab62f, - 0x2c3b3640, - 0x2c3bb65f, + 0x2c373572, + 0x2c37b59e, + 0x2c3835dc, + 0x2c38b5f3, + 0x2c393611, + 0x2c39b621, + 0x2c3a3633, + 0x2c3ab647, + 0x2c3b3658, + 0x2c3bb677, 0x2c3c144d, 0x2c3c9463, - 0x2c3d36a4, + 0x2c3d36bc, 0x2c3d947c, - 0x2c3e36ce, - 0x2c3eb6dc, - 0x2c3f36f4, - 0x2c3fb70c, - 0x2c403736, + 0x2c3e36e6, + 0x2c3eb6f4, + 0x2c3f370c, + 0x2c3fb724, + 0x2c40374e, 0x2c409330, - 0x2c413747, - 0x2c41b75a, + 0x2c41375f, + 0x2c41b772, 0x2c4212f6, - 0x2c42b76b, + 0x2c42b783, 0x2c43076d, - 0x2c43b651, - 0x2c443599, - 0x2c44b719, - 0x2c453530, - 0x2c45b56c, - 0x2c4635e9, - 0x2c46b673, - 0x2c473688, - 0x2c47b6c1, - 0x2c4835ab, + 0x2c43b669, + 0x2c4435b1, + 0x2c44b731, + 0x2c453548, + 0x2c45b584, + 0x2c463601, + 0x2c46b68b, + 0x2c4736a0, + 0x2c47b6d9, + 0x2c4835c3, 0x30320000, 0x30328015, 0x3033001f, @@ -525,21 +525,21 @@ 0x4075b20c, 0x4076321a, 0x407693f3, - 0x4077323f, - 0x4077b29b, - 0x407832b6, - 0x4078b2ef, - 0x40793306, - 0x4079b31c, - 0x407a3348, - 0x407ab35b, - 0x407b3370, - 0x407bb382, - 0x407c33b3, - 0x407cb3bc, + 0x40773257, + 0x4077b2b3, + 0x407832ce, + 0x4078b307, + 0x4079331e, + 0x4079b334, + 0x407a3360, + 0x407ab373, + 0x407b3388, + 0x407bb39a, + 0x407c33cb, + 0x407cb3d4, 0x407d2a62, 0x407da2ef, - 0x407e32cb, + 0x407e32e3, 0x407ea581, 0x407f1eca, 0x407fa0ad, @@ -565,7 +565,7 @@ 0x40899c5f, 0x408a2ce1, 0x408a9a7d, - 0x408b3397, + 0x408b33af, 0x408bb0f3, 0x408c2667, 0x408d1ffe, @@ -585,7 +585,7 @@ 0x40941f2a, 0x4094acfa, 0x40952866, - 0x4095b328, + 0x4095b340, 0x40963050, 0x4096a27a, 0x409723b5, @@ -598,7 +598,7 @@ 0x409a9a99, 0x409b1f84, 0x409b9faf, - 0x409c327d, + 0x409c3295, 0x409c9fd7, 0x409d2236, 0x409da1ca, @@ -613,13 +613,14 @@ 0x40a22351, 0x40a2a735, 0x40a327a9, - 0x40a3b261, + 0x40a3b279, 0x40a4239b, 0x40a4a1fc, 0x40a51f06, 0x40a5a309, 0x40a62651, 0x40a6a21e, + 0x40a7323f, 0x41f42b9b, 0x41f92c2d, 0x41fe2b20, @@ -710,71 +711,71 @@ 0x4c419545, 0x4c4216ae, 0x4c42948d, - 0x5032377d, - 0x5032b78c, - 0x50333797, - 0x5033b7a7, - 0x503437c0, - 0x5034b7da, - 0x503537e8, - 0x5035b7fe, - 0x50363810, - 0x5036b826, - 0x5037383f, - 0x5037b852, - 0x5038386a, - 0x5038b87b, - 0x50393890, - 0x5039b8a4, - 0x503a38c4, - 0x503ab8da, - 0x503b38f2, - 0x503bb904, - 0x503c3920, - 0x503cb937, - 0x503d3950, - 0x503db966, - 0x503e3973, - 0x503eb989, - 0x503f399b, + 0x50323795, + 0x5032b7a4, + 0x503337af, + 0x5033b7bf, + 0x503437d8, + 0x5034b7f2, + 0x50353800, + 0x5035b816, + 0x50363828, + 0x5036b83e, + 0x50373857, + 0x5037b86a, + 0x50383882, + 0x5038b893, + 0x503938a8, + 0x5039b8bc, + 0x503a38dc, + 0x503ab8f2, + 0x503b390a, + 0x503bb91c, + 0x503c3938, + 0x503cb94f, + 0x503d3968, + 0x503db97e, + 0x503e398b, + 0x503eb9a1, + 0x503f39b3, 0x503f83b3, - 0x504039ae, - 0x5040b9be, - 0x504139d8, - 0x5041b9e7, - 0x50423a01, - 0x5042ba1e, - 0x50433a2e, - 0x5043ba3e, - 0x50443a5b, + 0x504039c6, + 0x5040b9d6, + 0x504139f0, + 0x5041b9ff, + 0x50423a19, + 0x5042ba36, + 0x50433a46, + 0x5043ba56, + 0x50443a73, 0x50448469, - 0x50453a6f, - 0x5045ba8d, - 0x50463aa0, - 0x5046bab6, - 0x50473ac8, - 0x5047badd, - 0x50483b03, - 0x5048bb11, - 0x50493b24, - 0x5049bb39, - 0x504a3b4f, - 0x504abb5f, - 0x504b3b7f, - 0x504bbb92, - 0x504c3bb5, - 0x504cbbe3, - 0x504d3c10, - 0x504dbc2d, - 0x504e3c48, - 0x504ebc64, - 0x504f3c76, - 0x504fbc8d, - 0x50503c9c, + 0x50453a87, + 0x5045baa5, + 0x50463ab8, + 0x5046bace, + 0x50473ae0, + 0x5047baf5, + 0x50483b1b, + 0x5048bb29, + 0x50493b3c, + 0x5049bb51, + 0x504a3b67, + 0x504abb77, + 0x504b3b97, + 0x504bbbaa, + 0x504c3bcd, + 0x504cbbfb, + 0x504d3c28, + 0x504dbc45, + 0x504e3c60, + 0x504ebc7c, + 0x504f3c8e, + 0x504fbca5, + 0x50503cb4, 0x50508729, - 0x50513caf, - 0x5051ba4d, - 0x50523bf5, + 0x50513cc7, + 0x5051ba65, + 0x50523c0d, 0x58321011, 0x68320fd3, 0x68328d2b, @@ -819,19 +820,19 @@ 0x7c32130c, 0x80321558, 0x80328090, - 0x80333479, + 0x80333491, 0x803380b9, - 0x80343488, - 0x8034b3f0, - 0x8035340e, - 0x8035b49c, - 0x80363450, - 0x8036b3ff, - 0x80373442, - 0x8037b3dd, - 0x80383463, - 0x8038b41f, - 0x80393434, + 0x803434a0, + 0x8034b408, + 0x80353426, + 0x8035b4b4, + 0x80363468, + 0x8036b417, + 0x8037345a, + 0x8037b3f5, + 0x8038347b, + 0x8038b437, + 0x8039344c, 0x84320bb0, 0x84328bc9, }; @@ -1424,6 +1425,7 @@ "UNKNOWN_SSL_VERSION\0" "UNKNOWN_STATE\0" "UNSAFE_LEGACY_RENEGOTIATION_DISABLED\0" + "UNSUPPORTED_CERTIFICATE\0" "UNSUPPORTED_COMPRESSION_ALGORITHM\0" "UNSUPPORTED_CREDENTIAL_LIST\0" "UNSUPPORTED_ECH_SERVER_CONFIG\0"
diff --git a/include/openssl/prefix_symbols.h b/include/openssl/prefix_symbols.h index 2b51e60..5af618a 100644 --- a/include/openssl/prefix_symbols.h +++ b/include/openssl/prefix_symbols.h
@@ -2201,6 +2201,7 @@ #pragma redefine_extname SSL_get_max_proto_version BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_max_proto_version) #pragma redefine_extname SSL_get_min_proto_version BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_min_proto_version) #pragma redefine_extname SSL_get_mode BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_mode) +#pragma redefine_extname SSL_get_negotiated_client_cert_type BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_negotiated_client_cert_type) #pragma redefine_extname SSL_get_negotiated_group BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_negotiated_group) #pragma redefine_extname SSL_get_options BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_options) #pragma redefine_extname SSL_get_peer_cert_chain BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_get_peer_cert_chain) @@ -5288,6 +5289,7 @@ #define SSL_get_max_proto_version BORINGSSL_ADD_PREFIX(SSL_get_max_proto_version) #define SSL_get_min_proto_version BORINGSSL_ADD_PREFIX(SSL_get_min_proto_version) #define SSL_get_mode BORINGSSL_ADD_PREFIX(SSL_get_mode) +#define SSL_get_negotiated_client_cert_type BORINGSSL_ADD_PREFIX(SSL_get_negotiated_client_cert_type) #define SSL_get_negotiated_group BORINGSSL_ADD_PREFIX(SSL_get_negotiated_group) #define SSL_get_options BORINGSSL_ADD_PREFIX(SSL_get_options) #define SSL_get_peer_cert_chain BORINGSSL_ADD_PREFIX(SSL_get_peer_cert_chain)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index fe86f5d..bad8b3e 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -3911,6 +3911,11 @@ const uint8_t *values, size_t num_values); +// SSL_get_negotiated_client_cert_type returns the connection's negotiated value +// of client_certificate_type. If no type has been negotiated explicitly, it +// returns |TLSEXT_cert_type_x509| by default. +OPENSSL_EXPORT int SSL_get_negotiated_client_cert_type(const SSL *ssl); + // Password Authenticated Key Exchange (PAKE). // @@ -6858,6 +6863,7 @@ #define SSL_R_INVALID_PSK_FOR_CONNECTION 331 #define SSL_R_NO_SUPPORTED_PSK_MODE 332 #define SSL_R_INVALID_CERT_TYPES_LIST 333 +#define SSL_R_UNSUPPORTED_CERTIFICATE 334 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index d3fa785..5c49764 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h
@@ -65,6 +65,7 @@ #define TLSEXT_TYPE_application_layer_protocol_negotiation 16 // ExtensionType values from RFC 7250 +#define TLSEXT_TYPE_client_cert_type 19 #define TLSEXT_TYPE_server_cert_type 20 // ExtensionType value from RFC 7685
diff --git a/ssl/extensions.cc b/ssl/extensions.cc index 8556615..442a259 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc
@@ -40,6 +40,7 @@ #include "../crypto/bytestring/internal.h" #include "../crypto/internal.h" +#include "../crypto/mem_internal.h" #include "../crypto/spake2plus/internal.h" #include "internal.h" @@ -3694,10 +3695,111 @@ return true; } -// Server certificate type +// Client certificate type & Server certificate type // // https://www.rfc-editor.org/rfc/rfc7250.html#section-3 +// parse_clienthello_cert_types_list returns a span containing the cert type +// values from a client_certificate_type or server_certificate_type ClientHello +// extension. Returns std::nullopt on error, or returns a nonempty span +// containing the values. This rejects the invalid empty list and list +// containing only the default X.509. This does not filter out unrecognized +// values. We don't check for duplicates here, even though a list containing +// duplicates should be considered invalid, to avoid quadratic behavior. +static std::optional<Span<const uint8_t>> parse_clienthello_cert_types_list( + uint8_t *out_alert, CBS *contents) { + CBS cert_types; + if (!CBS_get_u8_length_prefixed(contents, &cert_types) || + CBS_len(contents) != 0) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + return std::nullopt; + } + Span<const uint8_t> cert_types_list(cert_types); + // The client must omit the extension if the only type would be the default, + // X.509. + if (cert_types_list.empty() || + (cert_types_list.size() == 1 && cert_types_list[0] == kDefaultCertType)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + return std::nullopt; + } + return cert_types_list; +} + +bool ssl_negotiate_client_certificate_type( + const SSL_HANDSHAKE *hs, uint8_t *out_alert, + const SSL_CLIENT_HELLO *client_hello) { + assert(!hs->config->accepted_peer_cert_types.empty()); + const SSL *const ssl = hs->ssl; + ssl->s3->client_cert_type.reset(); + CBS contents; + Span<const uint8_t> peer_available_client_cert_types; + if (ssl_client_hello_get_extension(client_hello, &contents, + TLSEXT_TYPE_client_cert_type)) { + std::optional<Span<const uint8_t>> client_hello_client_cert_types = + parse_clienthello_cert_types_list(out_alert, &contents); + if (!client_hello_client_cert_types.has_value()) { + return false; + } + peer_available_client_cert_types = *client_hello_client_cert_types; + } + // If the client didn't send the extension, assume the client supports X.509 + // only by default. + if (peer_available_client_cert_types.empty()) { + peer_available_client_cert_types = + Span<const uint8_t>(&kDefaultCertType, 1u); + } + for (const uint8_t cert_type : hs->config->accepted_peer_cert_types) { + if (std::find(peer_available_client_cert_types.begin(), + peer_available_client_cert_types.end(), + cert_type) == peer_available_client_cert_types.end()) { + continue; + } + ssl->s3->client_cert_type.emplace(cert_type); + break; + } + if (hs->cert_request && !ssl->s3->client_cert_type.has_value()) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_CERTIFICATE); + *out_alert = SSL_AD_UNSUPPORTED_CERTIFICATE; + return false; + } + return true; +} + +static bool ext_client_cert_type_add_clienthello(const SSL_HANDSHAKE *hs, + CBB *out, + CBB *out_compressible, + ssl_client_hello_type_t type) { + // TODO(crbug.com/467663225): Implement this. + return true; +} + +static bool ext_client_cert_type_parse_serverhello(SSL_HANDSHAKE *hs, + uint8_t *out_alert, + CBS *contents) { + // TODO(crbug.com/467663225): Implement this. + return true; +} + +static bool ext_client_cert_type_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) { + // Only send client_certificate_type if we plan to send a CertificateRequest. + if (!hs->cert_request) { + return true; + } + // If no client_certificate_type value was negotiated, we would have failed + // earlier. + assert(hs->ssl->s3->client_cert_type.has_value()); + CBB contents; + if (!CBB_add_u16(out, TLSEXT_TYPE_client_cert_type) || + !CBB_add_u16_length_prefixed(out, &contents) || + !CBB_add_u8(&contents, *hs->ssl->s3->client_cert_type) || // + !CBB_flush(out)) { + return false; + } + return true; +} + static bool ext_server_cert_type_add_clienthello(const SSL_HANDSHAKE *hs, CBB *out, CBB *out_compressible, @@ -3945,6 +4047,15 @@ ext_trust_anchors_add_serverhello, }, { + TLSEXT_TYPE_client_cert_type, + ext_client_cert_type_add_clienthello, + ext_client_cert_type_parse_serverhello, + // client_certificate_type is negotiated late in + // `ssl_negotiate_client_cert_type`. + ignore_parse_clienthello, + ext_client_cert_type_add_serverhello, + }, + { TLSEXT_TYPE_server_cert_type, ext_server_cert_type_add_clienthello, ext_server_cert_type_parse_serverhello,
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 3e85edf..738d6fe 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -844,9 +844,14 @@ } } + uint8_t alert = SSL_AD_DECODE_ERROR; + if (!ssl_negotiate_client_certificate_type(hs, &alert, &client_hello)) { + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); + return ssl_hs_error; + } + // HTTP/2 negotiation depends on the cipher suite, so ALPN negotiation was // deferred. Complete it now. - uint8_t alert = SSL_AD_DECODE_ERROR; if (!ssl_negotiate_alpn(hs, &alert, &client_hello)) { ssl_send_alert(ssl, SSL3_AL_FATAL, alert); return ssl_hs_error;
diff --git a/ssl/internal.h b/ssl/internal.h index e30164c..575b0f1 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1571,7 +1571,7 @@ uint16_t *out_sigalg); -// Server certificate type. +// Client certificate type & Server certificate type. inline constexpr uint8_t kCertTypes[] = { TLSEXT_cert_type_x509, @@ -1580,6 +1580,15 @@ inline constexpr size_t kNumCertTypes = std::size(kCertTypes); inline constexpr uint8_t kDefaultCertType = TLSEXT_cert_type_x509; +// ssl_negotiate_client_certificate_type negotiates the client_certificate_type +// extension, if applicable. It sets `hs->ssl->s3->client_cert_type` iff a value +// was successfully negotiated. If a certificate request will be sent to the +// client, a value must be negotiated. It returns true if successful, or returns +// false and sets `*out_alert` to an alert on error. +bool ssl_negotiate_client_certificate_type( + const SSL_HANDSHAKE *hs, uint8_t *out_alert, + const SSL_CLIENT_HELLO *client_hello); + // Handshake functions. @@ -2933,6 +2942,11 @@ // srtp_profile is the selected SRTP protection profile for // DTLS-SRTP. const SRTP_PROTECTION_PROFILE *srtp_profile = nullptr; + + // client_cert_type, if non-nullopt, is the negotiated client cert type for + // the connection. If this is nullopt, the peer did not send the + // client_certificate_type extension, or no suitable value was negotiated. + std::optional<uint8_t> client_cert_type; }; // lengths of messages
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 6444344..b50ca0c 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -3622,3 +3622,7 @@ return set1_cert_types(&ssl->config->accepted_peer_cert_types, Span(values, num_values)); } + +int SSL_get_negotiated_client_cert_type(const SSL *ssl) { + return ssl->s3->client_cert_type.value_or(kDefaultCertType); +}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 4e306a1..dc73c12 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -715,6 +715,16 @@ return false; } + if (const auto &expected = config->expect_client_certificate_type; + expected.has_value()) { + const uint8_t negotiated = SSL_get_negotiated_client_cert_type(ssl); + if (*expected != negotiated) { + fprintf(stderr, "Negotiated client_certificate_type %d, but wanted %d.\n", + negotiated, *expected); + return false; + } + } + // Check all the selected parameters are covered by the string APIs. if (!CheckListContains("version", SSL_get_all_version_names, SSL_get_version(ssl)) ||
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index b378a83..847c262 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -205,6 +205,7 @@ extensionUseSRTP uint16 = 14 extensionALPN uint16 = 16 extensionSignedCertificateTimestamp uint16 = 18 + extensionClientCertificateType uint16 = 19 extensionServerCertificateType uint16 = 20 extensionPadding uint16 = 21 extensionExtendedMasterSecret uint16 = 23 @@ -2246,10 +2247,20 @@ // flag set. ExpectResumptionAcrossNames *bool + // ExpectClientCertificateTypes, if not nil, causes the server or client to + // expect the client_certificate_type extension sent by the peer to contain + // exactly the given values. + ExpectClientCertificateTypes []CertificateType + // ExpectServerCertificateTypes, if not nil, causes the server to // expect the server_certificate_type extension sent by the peer to contain // exactly the given values. ExpectServerCertificateTypes []CertificateType + + // SendClientCertificateTypes, if not nil, causes the server or client to + // send a client_certificate_type extension containing the given values. + // For a server, this may not contain more than 1 value. + SendClientCertificateTypes []CertificateType } func (c *Config) serverInit() {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index aafa6e7..e77f86f 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -542,6 +542,7 @@ emptyExtensions: c.config.Bugs.EmptyExtensions, delegatedCredential: c.config.DelegatedCredentialAlgorithms, trustAnchors: c.config.RequestTrustAnchors, + clientCertificateTypes: c.config.Bugs.SendClientCertificateTypes, } // Translate the bugs that modify ClientHello extension order into a @@ -2162,6 +2163,19 @@ c.peerApplicationSettingsOld = hs.session.peerApplicationSettingsOld } + if expected := c.config.Bugs.ExpectClientCertificateTypes; expected != nil { + if len(expected) > 1 { + panic("Expected client_certificate_type must not contain more than 1 value.") + } + var found []CertificateType + if serverExtensions.clientCertificateType != nil { + found = []CertificateType{*serverExtensions.clientCertificateType} + } + if !slices.Equal(found, expected) { + return fmt.Errorf("tls: server sent client certificate type %v, but expected %v", found, expected) + } + } + return nil }
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index d0d8536..385473d 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -254,6 +254,7 @@ pakeShares []pakeShare certificateAuthorities [][]byte trustAnchors [][]byte + clientCertificateTypes []CertificateType serverCertificateTypes []CertificateType outerExtensions []uint16 reorderOuterExtensionsWithoutCompressing bool @@ -635,6 +636,18 @@ body: body.BytesOrPanic(), }) } + if m.clientCertificateTypes != nil { + body := cryptobyte.NewBuilder(nil) + body.AddUint8LengthPrefixed(func(certTypesList *cryptobyte.Builder) { + for _, certType := range m.clientCertificateTypes { + certTypesList.AddUint8(uint8(certType)) + } + }) + extensions = append(extensions, extension{ + id: extensionClientCertificateType, + body: body.BytesOrPanic(), + }) + } if m.serverCertificateTypes != nil { body := cryptobyte.NewBuilder(nil) body.AddUint8LengthPrefixed(func(certTypesList *cryptobyte.Builder) { @@ -871,6 +884,7 @@ m.pakeClientID = nil m.pakeServerID = nil m.pakeShares = nil + m.clientCertificateTypes = nil m.serverCertificateTypes = nil if len(reader) == 0 { @@ -1200,6 +1214,23 @@ if !parseTrustAnchors(&body, &m.trustAnchors) || len(body) != 0 { return false } + case extensionClientCertificateType: + var certTypes cryptobyte.String + if !body.ReadUint8LengthPrefixed(&certTypes) || len(body) != 0 { + return false + } + for len(certTypes) > 0 { + var certType uint8 + if !certTypes.ReadUint8(&certType) { + return false + } + m.clientCertificateTypes = append(m.clientCertificateTypes, CertificateType(certType)) + } + // A client must omit the extension if empty or if the only type is the default, X.509. + if len(m.clientCertificateTypes) == 0 || + (len(m.clientCertificateTypes) == 1 && m.clientCertificateTypes[0] == certTypeX509) { + return false + } case extensionServerCertificateType: var certTypes cryptobyte.String if !body.ReadUint8LengthPrefixed(&certTypes) || len(body) != 0 { @@ -1630,6 +1661,7 @@ hasApplicationSettingsOld bool echRetryConfigs []byte trustAnchors [][]byte + clientCertificateType *CertificateType } func (m *serverExtensions) marshal(extensions *cryptobyte.Builder) { @@ -1774,6 +1806,11 @@ }) }) } + if m.clientCertificateType != nil { + extensions.AddUint16(extensionClientCertificateType) + extensions.AddUint16(1) // Length + extensions.AddUint8(uint8(*m.clientCertificateType)) + } } func (m *serverExtensions) unmarshal(data cryptobyte.String, version version) bool { @@ -1913,6 +1950,12 @@ if !parseTrustAnchors(&body, &m.trustAnchors) || len(m.trustAnchors) == 0 || len(body) != 0 { return false } + case extensionClientCertificateType: + var certType uint8 + if !body.ReadUint8(&certType) || len(body) != 0 { + return false + } + m.clientCertificateType = ptrTo(CertificateType(certType)) default: // Unknown extensions are illegal from the server. return false
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 286325b..3b82098 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -441,6 +441,12 @@ } } + if expected := c.config.Bugs.ExpectClientCertificateTypes; expected != nil { + if !slices.Equal(expected, hs.clientHello.clientCertificateTypes) { + return fmt.Errorf("tls: client offered client certificate types %v, but expected %v", hs.clientHello.clientCertificateTypes, expected) + } + } + if expected := c.config.Bugs.ExpectServerCertificateTypes; expected != nil { if !slices.Equal(expected, hs.clientHello.serverCertificateTypes) { return fmt.Errorf("tls: client offered server certificate types %v, but expected %v", hs.clientHello.serverCertificateTypes, expected) @@ -1782,6 +1788,17 @@ } } + if sendClientCertType := c.config.Bugs.SendClientCertificateTypes; sendClientCertType != nil { + if len(sendClientCertType) > 1 { + panic("tls: client_certificate_type must not contain more than 1 value.") + } + if len(sendClientCertType) == 0 { + serverExtensions.clientCertificateType = nil + } else { + serverExtensions.clientCertificateType = ptrTo(sendClientCertType[0]) + } + } + return nil }
diff --git a/ssl/test/runner/raw_public_key_tests.go b/ssl/test/runner/raw_public_key_tests.go index 1046d76..f68ff13 100644 --- a/ssl/test/runner/raw_public_key_tests.go +++ b/ssl/test/runner/raw_public_key_tests.go
@@ -16,13 +16,19 @@ import ( "fmt" + "strconv" ) +const certTypeBogus CertificateType = 5 + var ( - certTypesListRPKOnly = []CertificateType{certTypeRawPublicKey} - certTypesListRPKX509 = []CertificateType{certTypeRawPublicKey, certTypeX509} - certTypesListX509RPK = []CertificateType{certTypeX509, certTypeRawPublicKey} - certTypesListX509Only = []CertificateType{certTypeX509} + certTypesListRPKOnly = []CertificateType{certTypeRawPublicKey} + certTypesListRPKX509 = []CertificateType{certTypeRawPublicKey, certTypeX509} + certTypesListX509RPK = []CertificateType{certTypeX509, certTypeRawPublicKey} + certTypesListX509Only = []CertificateType{certTypeX509} + certTypesListUnknown = []CertificateType{certTypeBogus} + certTypesListUnknownX509 = []CertificateType{certTypeX509, certTypeBogus} + certTypesListRPKUnknown = []CertificateType{certTypeRawPublicKey, certTypeBogus} ) func addServerCertTypeTests() { @@ -72,6 +78,171 @@ } } +func addClientCertTypeTests() { + // Tests receiving a client_certificate_type extension from the client and + // selecting and sending our most-preferred shared cert type. + for _, ver := range allVersions(tls) { + for _, test := range []struct { + name string + clientCertTypesReceived []CertificateType + clientCertTypesAccepted []CertificateType + expectedServerHelloExtension []CertificateType + expectedNegotiated CertificateType + expectedError string + expectedLocalError string + }{ + { + name: "RPKReceived-RPKAccepted", + clientCertTypesReceived: certTypesListRPKOnly, + clientCertTypesAccepted: certTypesListRPKOnly, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + name: "RPKX509Received-RPKAccepted", + clientCertTypesReceived: certTypesListRPKX509, + clientCertTypesAccepted: certTypesListRPKOnly, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + name: "X509RPKReceived-RPKAccepted", + clientCertTypesReceived: certTypesListX509RPK, + clientCertTypesAccepted: certTypesListRPKOnly, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + name: "RPKX509Received-RPKX509Accepted", + clientCertTypesReceived: certTypesListRPKX509, + clientCertTypesAccepted: certTypesListRPKX509, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + name: "X509RPKReceived-RPKX509Accepted", + clientCertTypesReceived: certTypesListX509RPK, + clientCertTypesAccepted: certTypesListRPKX509, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + name: "RPKX509Received-X509RPKAccepted", + clientCertTypesReceived: certTypesListRPKX509, + clientCertTypesAccepted: certTypesListX509RPK, + expectedServerHelloExtension: certTypesListX509Only, + expectedNegotiated: certTypeX509, + }, + { + name: "X509RPKReceived-X509RPKAccepted", + clientCertTypesReceived: certTypesListX509RPK, + clientCertTypesAccepted: certTypesListX509RPK, + expectedServerHelloExtension: certTypesListX509Only, + expectedNegotiated: certTypeX509, + }, + { + name: "RejectsInvalidEmptyExtension", + clientCertTypesReceived: []CertificateType{}, + clientCertTypesAccepted: certTypesListX509RPK, + expectedError: ":DECODE_ERROR:", + expectedLocalError: "remote error: illegal parameter", + }, + { + // The client should have omitted the extension if only the default is + // accepted. + name: "RejectsInvalidDefaultOnly", + clientCertTypesReceived: certTypesListX509Only, + clientCertTypesAccepted: certTypesListX509RPK, + expectedError: ":DECODE_ERROR:", + expectedLocalError: "remote error: illegal parameter", + }, + { + // The client's list contains only an unknown value, which is ignored. + // Negotiating a client cert type value fails. + name: "IgnoresUnknownValue-NoOtherType", + clientCertTypesReceived: certTypesListUnknown, + clientCertTypesAccepted: certTypesListX509RPK, + expectedError: ":UNSUPPORTED_CERTIFICATE:", + expectedLocalError: "remote error: unsupported certificate", + }, + { + // The client's list contains an unknown value, which is ignored, and + // a recognized value, which is not shared with the server. + name: "IgnoresUnknownValue-NoSharedType", + clientCertTypesReceived: certTypesListUnknownX509, + clientCertTypesAccepted: certTypesListRPKOnly, + expectedError: ":UNSUPPORTED_CERTIFICATE:", + expectedLocalError: "remote error: unsupported certificate", + }, + { + // The client's list contains an unknown value, which is ignored, and + // a recognized value, which is accepted successfully. + name: "IgnoresUnknownValue-RPKAccepted", + clientCertTypesReceived: certTypesListRPKUnknown, + clientCertTypesAccepted: certTypesListX509RPK, + expectedServerHelloExtension: certTypesListRPKOnly, + expectedNegotiated: certTypeRawPublicKey, + }, + { + // If the client does not send the extension, the server should treat it + // as X.509 only by default. + name: "NoClientHelloCertTypes-SelectsX509ByDefault", + clientCertTypesReceived: nil, + clientCertTypesAccepted: certTypesListRPKX509, + expectedServerHelloExtension: []CertificateType{}, + expectedNegotiated: certTypeX509, + }, + { + // If the client does not send the extension, but the server is + // configured to only accept RPKs, the connection should fail. + name: "NoClientHelloCertTypes-NoSharedType", + clientCertTypesReceived: nil, + clientCertTypesAccepted: certTypesListRPKOnly, + expectedError: ":UNSUPPORTED_CERTIFICATE:", + expectedLocalError: "remote error: unsupported certificate", + }, + } { + flags := + append(flagCertTypes("-accepted-peer-cert-types", test.clientCertTypesAccepted), + "-require-any-client-certificate") + // The handshake currently fails because the rest of the RPK client cert + // flow isn't yet implemented. + // TODO(crbug.com/467663225): Test client response and rest of the handshake. + shouldFail := true + expectedError := ":PEER_DID_NOT_RETURN_A_CERTIFICATE:" + expectedLocalError := "remote error: handshake failure" + if ver.version == VersionTLS13 { + expectedLocalError = "remote error: certificate required" + } + if test.expectedError != "" { + shouldFail = true + expectedError = test.expectedError + expectedLocalError = test.expectedLocalError + } else { + flags = append(flags, + "-expect-client-certificate-type", strconv.Itoa(int(test.expectedNegotiated))) + } + testCases = append(testCases, testCase{ + testType: serverTest, + name: fmt.Sprintf("ClientCertificateType-Server-%s-%s", test.name, ver.name), + config: Config{ + MinVersion: ver.version, + MaxVersion: ver.version, + Bugs: ProtocolBugs{ + SendClientCertificateTypes: test.clientCertTypesReceived, + ExpectClientCertificateTypes: test.expectedServerHelloExtension, + }, + }, + flags: flags, + shouldFail: shouldFail, + expectedError: expectedError, + expectedLocalError: expectedLocalError, + }) + } + } +} + func addRawPublicKeyTests() { addServerCertTypeTests() + addClientCertTypeTests() }
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index a0c7516..df84832 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -629,6 +629,8 @@ BoolFlag("-no-server-name-ack", &TestConfig::no_server_name_ack), IntVectorFlag("-accepted-peer-cert-types", &TestConfig::accepted_peer_cert_types), + OptionalIntFlag("-expect-client-certificate-type", + &TestConfig::expect_client_certificate_type), }; std::sort(ret.begin(), ret.end(), FlagNameComparator{}); return ret;
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index abab643..1e8d399 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -251,6 +251,7 @@ std::optional<bool> expect_resumable_across_names; bool no_server_name_ack = false; std::vector<uint8_t> accepted_peer_cert_types; + std::optional<uint8_t> expect_client_certificate_type; std::vector<const char *> handshaker_args;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 4b6d840..eccf8f5 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -633,6 +633,15 @@ return ssl_hs_error; } + if (using_certificate(hs)) { + // Determine whether to request a client certificate. + hs->cert_request = !!(hs->config->verify_mode & SSL_VERIFY_PEER); + } + if (!ssl_negotiate_client_certificate_type(hs, &alert, &client_hello)) { + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); + return ssl_hs_error; + } + // Record connection properties in the new session. hs->new_session->cipher = hs->new_cipher; @@ -1045,11 +1054,6 @@ return ssl_hs_error; } - if (using_certificate(hs)) { - // Determine whether to request a client certificate. - hs->cert_request = !!(hs->config->verify_mode & SSL_VERIFY_PEER); - } - // Send a CertificateRequest, if necessary. if (hs->cert_request) { CBB cert_request_extensions, sigalg_contents, sigalgs_cbb;