Enforce the keyUsage extension in TLS 1.2 client certs.
I've left this independent of SSL_set_enforce_rsa_key_usage because
client certificates in TLS always use the digitalSignature bit, RSA or
otherwise, so it's less likely that someone has messed it up, unlike
TLS 1.2 RSA server certificates.
Update-Note: Client certificates which do not support the
digitalSignature key usage will be rejected. They should either include
that bit or omit the keyUsage extension.
Bug: 349
Change-Id: I97bbf0c8e394f219ff75b686e0c14019f6d8c9a8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41664
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 9625b8e..670e476 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1268,10 +1268,10 @@
uint32_t alg_k = hs->new_cipher->algorithm_mkey;
uint32_t alg_a = hs->new_cipher->algorithm_auth;
if (ssl_cipher_uses_certificate_auth(hs->new_cipher)) {
- CRYPTO_BUFFER *leaf =
+ const CRYPTO_BUFFER *leaf =
sk_CRYPTO_BUFFER_value(hs->new_session->certs.get(), 0);
CBS leaf_cbs;
- CBS_init(&leaf_cbs, CRYPTO_BUFFER_data(leaf), CRYPTO_BUFFER_len(leaf));
+ CRYPTO_BUFFER_init_CBS(leaf, &leaf_cbs);
// Check the key usage matches the cipher suite. We do this unconditionally
// for non-RSA certificates. In particular, it's needed to distinguish ECDH
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 924701f..2489428 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -1436,6 +1436,15 @@
return ssl_hs_error;
}
+ // The peer certificate must be valid for signing.
+ const CRYPTO_BUFFER *leaf =
+ sk_CRYPTO_BUFFER_value(hs->new_session->certs.get(), 0);
+ CBS leaf_cbs;
+ CRYPTO_BUFFER_init_CBS(leaf, &leaf_cbs);
+ if (!ssl_cert_check_key_usage(&leaf_cbs, key_usage_digital_signature)) {
+ return ssl_hs_error;
+ }
+
CBS certificate_verify = msg.body, signature;
// Determine the signature algorithm.
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ae9b40d..deddb03 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -14627,7 +14627,7 @@
testCases = append(testCases, testCase{
testType: clientTest,
- name: "ECDSAKeyUsage-" + ver.name,
+ name: "ECDSAKeyUsage-Client-" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14636,6 +14636,19 @@
shouldFail: true,
expectedError: ":KEY_USAGE_BIT_INCORRECT:",
})
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "ECDSAKeyUsage-Server-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{cert},
+ },
+ flags: []string{"-require-any-client-certificate"},
+ shouldFail: true,
+ expectedError: ":KEY_USAGE_BIT_INCORRECT:",
+ })
}
}
@@ -14705,7 +14718,7 @@
for _, ver := range tlsVersions {
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantSignature-GotEncipherment-" + ver.name,
+ name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14721,7 +14734,7 @@
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantSignature-GotSignature-" + ver.name,
+ name: "RSAKeyUsage-Client-WantSignature-GotSignature-" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14737,7 +14750,7 @@
if ver.version < VersionTLS13 {
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantEncipherment-GotEncipherment" + ver.name,
+ name: "RSAKeyUsage-Client-WantEncipherment-GotEncipherment" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14751,7 +14764,7 @@
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantEncipherment-GotSignature-" + ver.name,
+ name: "RSAKeyUsage-Client-WantEncipherment-GotSignature-" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14768,7 +14781,7 @@
// In 1.2 and below, we should not enforce without the enforce-rsa-key-usage flag.
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantSignature-GotEncipherment-Unenforced" + ver.name,
+ name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Unenforced" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14779,7 +14792,7 @@
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantEncipherment-GotSignature-Unenforced" + ver.name,
+ name: "RSAKeyUsage-Client-WantEncipherment-GotSignature-Unenforced" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14787,14 +14800,13 @@
CipherSuites: dsSuites,
},
})
-
}
if ver.version >= VersionTLS13 {
// In 1.3 and above, we enforce keyUsage even without the flag.
testCases = append(testCases, testCase{
testType: clientTest,
- name: "RSAKeyUsage-WantSignature-GotEncipherment-Enforced" + ver.name,
+ name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Enforced" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
@@ -14804,8 +14816,33 @@
shouldFail: true,
expectedError: ":KEY_USAGE_BIT_INCORRECT:",
})
-
}
+
+ // The server only uses signatures and always enforces it.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RSAKeyUsage-Server-WantSignature-GotEncipherment-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{encCert},
+ },
+ shouldFail: true,
+ expectedError: ":KEY_USAGE_BIT_INCORRECT:",
+ flags: []string{"-require-any-client-certificate"},
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RSAKeyUsage-Server-WantSignature-GotSignature-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{dsCert},
+ },
+ flags: []string{"-require-any-client-certificate"},
+ })
+
}
}
