bneradt commented on code in PR #13378:
URL: https://github.com/apache/trafficserver/pull/13378#discussion_r4201327090
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -2531,6 +2568,203 @@ SSLGetGroupName([[maybe_unused]] SSL *ssl)
#endif // HAVE_SSL_GET0_GROUP_NAME
}
+namespace
+{
+bool
+is_grease_signature_algorithm(uint16_t algorithm)
+{
+ return (algorithm & 0x0f0f) == 0x0a0a && (algorithm >> 12) == ((algorithm >>
4) & 0x0f);
+}
+
+#if HAVE_SSL_GET_LOCAL_SIGNATURE_NIDS && HAVE_SSL_GET_SHARED_SIGALGS
+bool
+signature_algorithm_matches_private_key(SSL *ssl, uint16_t algorithm)
+{
+ EVP_PKEY *private_key = SSL_get_privatekey(ssl);
+
+ if (private_key == nullptr) {
+ return false;
+ }
+
+ int const key_type = EVP_PKEY_base_id(private_key);
+
+ switch (algorithm) {
+ case 0x0804: // rsa_pss_rsae_sha256
+ case 0x0805: // rsa_pss_rsae_sha384
+ case 0x0806: // rsa_pss_rsae_sha512
+ return key_type == EVP_PKEY_RSA;
+ case 0x0809: // rsa_pss_pss_sha256
+ case 0x080a: // rsa_pss_pss_sha384
+ case 0x080b: // rsa_pss_pss_sha512
+ return key_type == EVP_PKEY_RSA_PSS;
+ default:
+ break;
+ }
+
+#if defined(TLS1_3_VERSION)
+ if (SSL_version(ssl) >= TLS1_3_VERSION && key_type == EVP_PKEY_EC) {
+ int expected_curve = NID_undef;
+
+ switch (algorithm) {
+ case 0x0403: // ecdsa_secp256r1_sha256
+ expected_curve = NID_X9_62_prime256v1;
+ break;
+ case 0x0503: // ecdsa_secp384r1_sha384
+ expected_curve = NID_secp384r1;
+ break;
+ case 0x0603: // ecdsa_secp521r1_sha512
+ expected_curve = NID_secp521r1;
+ break;
+ case 0x081a: // ecdsa_brainpoolP256r1tls13_sha256
+ expected_curve = NID_brainpoolP256r1;
+ break;
+ case 0x081b: // ecdsa_brainpoolP384r1tls13_sha384
+ expected_curve = NID_brainpoolP384r1;
+ break;
+ case 0x081c: // ecdsa_brainpoolP512r1tls13_sha512
+ expected_curve = NID_brainpoolP512r1;
+ break;
+ default:
+ break;
+ }
+
+ if (expected_curve != NID_undef) {
+#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3
+ char group_name[80];
+ size_t group_name_length = 0;
+
+ return EVP_PKEY_get_utf8_string_param(private_key,
OSSL_PKEY_PARAM_GROUP_NAME, group_name, sizeof(group_name),
+ &group_name_length) == 1 &&
+ OBJ_txt2nid(group_name) == expected_curve;
+#else
+ EC_KEY const *ec_key = EVP_PKEY_get0_EC_KEY(private_key);
+
+ if (ec_key != nullptr) {
+ EC_GROUP const *group = EC_KEY_get0_group(ec_key);
+ return group != nullptr && EC_GROUP_get_curve_name(group) ==
expected_curve;
+ }
+#endif
+ }
+ }
+#endif
+
+ return true;
+}
+#endif
+} // namespace
+
+std::string
+SSLFormatSignatureAlgorithms(swoc::MemSpan<uint16_t const> algorithms)
+{
+ std::string result;
+
+ for (uint16_t const algorithm : algorithms) {
+ if (is_grease_signature_algorithm(algorithm)) {
+ continue;
+ }
+ if (!result.empty()) {
+ result.push_back('-');
+ }
+ result.append(std::to_string(algorithm));
+ }
+
+ return result;
+}
+
+std::string
+SSLGetOfferedSignatureAlgorithms([[maybe_unused]] SSL *ssl)
+{
+#if HAVE_SSL_GET0_PEER_VERIFY_ALGORITHMS
+ uint16_t const *algorithms = nullptr;
+ size_t const count = SSL_get0_peer_verify_algorithms(ssl,
&algorithms);
+
+ return SSLFormatSignatureAlgorithms({algorithms, count});
+#elif HAVE_SSL_GET_SIGALGS
+ int const count = SSL_get_sigalgs(ssl, 0, nullptr, nullptr, nullptr,
nullptr, nullptr);
+
+ if (count <= 0) {
+ return {};
+ }
+
+ std::vector<uint16_t> algorithms;
+ algorithms.reserve(count);
+ for (int index = 0; index < count; ++index) {
+ unsigned char signature = 0;
+ unsigned char hash = 0;
+
+ if (SSL_get_sigalgs(ssl, index, nullptr, nullptr, nullptr, &signature,
&hash) > 0) {
+ algorithms.push_back(static_cast<uint16_t>(hash) << 8 | signature);
+ }
+ }
+
+ return SSLFormatSignatureAlgorithms(algorithms);
+#else
+ return {};
+#endif
+}
+
+std::string
+SSLGetNegotiatedSignatureAlgorithm(SSL *ssl)
+{
+ // A resumed handshake creates no new signature, even if the library retains
signature metadata.
+ if (ssl == nullptr || SSL_session_reused(ssl)) {
+ return {};
+ }
+
+#if HAVE_SSL_GET_SIGNATURE_ALGORITHM_USED
+ uint16_t const algorithm = SSL_get_signature_algorithm_used(ssl);
+
+ return algorithm == 0 ? "" : std::to_string(algorithm);
+#elif HAVE_SSL_GET_LOCAL_SIGNATURE_NIDS && HAVE_SSL_GET_SHARED_SIGALGS
+ // When the TLS library does not expose the exact IANA scheme, derive it
from the local signature type and digest, the
+ // shared list, and the certificate key. Return no value if more than one
compatible code point remains.
+ int signature_type = NID_undef;
+ int hash = NID_undef;
+
+ if (SSL_get_signature_type_nid(ssl, &signature_type) != 1 ||
SSL_get_signature_nid(ssl, &hash) != 1) {
Review Comment:
Confirmed and fixed. I reproduced it first with a new gold line before
touching the code — a TLS 1.2 handshake forced to `AES128-GCM-SHA256` logged
`cqssin 1025` even though the server never signed anything:
```
- tls12-static-rsa TLSv1.2 1025 - 0 0
+ tls12-static-rsa TLSv1.2 1025 1025 0 0
```
Took your guard, with a null check on the cipher, in the `#elif` branch
ahead of the `SSL_get_signature_type_nid()` call:
```cpp
SSL_CIPHER const *cipher = SSL_get_current_cipher(ssl);
if (cipher == nullptr || SSL_CIPHER_get_kx_nid(cipher) == NID_kx_rsa) {
return {};
}
```
I dropped the `SSL_version(ssl) < TLS1_3_VERSION` half of your suggestion:
TLS 1.3 suites report `NID_kx_any`, so the kx test alone can't fire there, and
leaving the version check out avoids another `TLS1_3_VERSION` guard. Say the
word if you'd rather keep it explicit.
The BoringSSL branch needs nothing — `SSL_get_signature_algorithm_used()`
already returns 0 when nothing was signed.
Also added a `/tls12-static-rsa` run to `tls_signature_algorithms.test.py`
asserting `-`, so the OpenSSL path is pinned, and a sentence in
`formatting.en.rst` noting static RSA key exchange alongside the resumption
case.
Verified on Fedora with OpenSSL 3.5.7: all 72 `gold_tests/tls` autests pass,
`test_net` signature cases pass, docs build clean. Reverting just the
`SSLUtils.cc` hunk puts the gold failure back, so the new case is doing real
work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
