maskit commented on code in PR #13548:
URL: https://github.com/apache/trafficserver/pull/13548#discussion_r3780089887
##########
src/iocore/net/SSLClientUtils.cc:
##########
@@ -236,6 +482,92 @@ ssl_new_session_callback(SSL *ssl, SSL_SESSION *sess)
return 0;
}
+#if TS_USE_RPK
+bool
+ssl_client_setup_rpk(SSL *ssl, bool offer_rpk, const std::string
&trusted_key_file)
+{
+ if (!offer_rpk && trusted_key_file.empty()) {
+ return true;
+ }
+
+ static std::once_flag rpk_index_once;
+ std::call_once(rpk_index_once, []() {
+ ssl_server_rpk_index = SSL_get_ex_new_index(0, (void *)"Trusted next-hop
RPK keys", nullptr, nullptr, ssl_server_rpk_ex_free);
+ });
+ if (ssl_server_rpk_index < 0) {
+ SSLError("failed to reserve an ex_data index for next-hop raw public
keys");
+ return false;
+ }
+
+ if (!trusted_key_file.empty()) {
+ auto *trusted = new SSLRPKUtils::TrustedKeySet();
+ if (!SSLRPKUtils::loadTrustedKeys(trusted_key_file.c_str(), *trusted)) {
+ delete trusted;
+ return false;
+ }
+ // ssl_server_rpk_ex_free() releases `trusted` when ssl is freed.
+ if (!SSL_set_ex_data(ssl, ssl_server_rpk_index, trusted)) {
+ delete trusted;
+ SSLError("failed to attach trusted next-hop raw public keys to the
connection");
+ return false;
+ }
+
+ // Accept a raw public key from the next hop, still preferring it over
X.509 only when the
+ // peer also supports it.
+ static const unsigned char server_types[] = {TLSEXT_cert_type_rpk,
TLSEXT_cert_type_x509};
+#if HAVE_SSL_CTX_SET1_SERVER_CERT_TYPE
+ if (!SSL_set1_server_cert_type(ssl, server_types, sizeof(server_types))) {
+#else
+ if (!SSL_set1_accepted_peer_cert_types(ssl, server_types,
sizeof(server_types))) {
+#endif
+ SSLError("failed to enable RPK server cert type negotiation for the
outbound connection");
+ return false;
+ }
+ }
+
+ if (offer_rpk) {
+ static const unsigned char client_types[] = {TLSEXT_cert_type_rpk,
TLSEXT_cert_type_x509};
+ // Both libraries derive/wrap the offered raw public key from the client
certificate/key
+ // already configured on the context -- there is nothing to offer if
that's unset.
+ if (SSL_CTX_get0_privatekey(SSL_get_SSL_CTX(ssl)) == nullptr) {
+ SSLError("client_rpk_enabled requires a client certificate/key
configured for this next hop");
+ return false;
+ }
+#if HAVE_SSL_CTX_SET1_SERVER_CERT_TYPE
+ // OpenSSL derives the offered raw public key from the certificate/key
already on the context.
+ if (!SSL_set1_client_cert_type(ssl, client_types, sizeof(client_types))) {
+ SSLError("failed to enable RPK client cert type negotiation for the
outbound connection");
+ return false;
+ }
+#else
+ // BoringSSL needs an explicit credential, wrapping that same
already-configured key.
+ EVP_PKEY *pkey = SSL_CTX_get0_privatekey(SSL_get_SSL_CTX(ssl));
+ SSL_CREDENTIAL *cred = SSL_CREDENTIAL_new_raw_public_key(pkey);
+ if (cred == nullptr || !SSL_add1_credential(ssl, cred)) {
+ SSLError("failed to add the outbound RPK credential");
+ SSL_CREDENTIAL_free(cred);
+ return false;
+ }
+ SSL_CREDENTIAL_free(cred);
+ if (!SSL_set1_available_client_cert_types(ssl, client_types,
sizeof(client_types))) {
+ SSLError("failed to advertise RPK client cert types for the outbound
connection");
+ return false;
+ }
+#endif
+ }
+
+#if HAVE_SSL_CREDENTIAL_NEW_RAW_PUBLIC_KEY
+ // BoringSSL rejects raw public keys unless a custom verify callback is
installed, and this
+ // displaces the SSL_set_verify()/verify_callback() pair the caller already
set for this
+ // connection. Only RPK-configured next hops take this path; every other
outbound connection
+ // keeps the classic callback untouched.
+ SSL_set_custom_verify(ssl, SSL_VERIFY_PEER,
ssl_client_custom_verify_callback);
+#endif
Review Comment:
Fixed
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -205,6 +246,107 @@ ssl_verify_client_callback(int preverify_ok,
X509_STORE_CTX *ctx)
return preverify_ok;
}
+#if HAVE_SSL_CREDENTIAL_NEW_RAW_PUBLIC_KEY
+// BoringSSL's SSL_CTX_set_custom_verify(), required to accept RPK client
certs, replaces its
+// automatic X.509 chain verification entirely -- unlike OpenSSL's classic
SSL_CTX_set_verify(),
+// which only lets ssl_verify_client_callback() observe/override a chain
BoringSSL already
+// validated. For the X.509 fallback case (the client didn't offer an RPK this
time), this
+// callback must therefore redo that validation manually via the legacy
X509_STORE_CTX API,
+// against the same certificate store _setup_client_cert_verification()
configured on this ctx.
+static enum ssl_verify_result_t
+ssl_custom_verify_client_callback(SSL *ssl, uint8_t *out_alert)
+{
+ Dbg(dbg_ctl_ssl_verify, "Callback: custom verify client cert (RPK-enabled
ctx)");
+ SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+ TLSBasicSupport *tbs = TLSBasicSupport::getInstance(ssl);
+ if (tbs == nullptr) {
+ Dbg(dbg_ctl_ssl_verify, "ssl_custom_verify_client_callback call back on
stale netvc");
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return ssl_verify_invalid;
+ }
+
+ SSL_CTX *ctx = SSL_get_SSL_CTX(ssl);
+
+ if (SSL_get_peer_cert_type(ssl) == TLSEXT_cert_type_rpk) {
+ EVP_PKEY *peer_rpk = SSL_get0_peer_rpk(ssl);
+ auto *trusted = static_cast<SSLRPKUtils::TrustedKeySet
*>(SSL_CTX_get_ex_data(ctx, ssl_client_rpk_ca_index));
+ bool pin_ok = trusted != nullptr &&
SSLRPKUtils::pinnedKeyMatches(peer_rpk, *trusted);
+ if (!pin_ok) {
+ Warning("client raw public key did not match any trusted key for %s",
netvc->options.sni_servername.get());
+ }
+ // As above: the hook always runs, and can add rejection but not override
a failed pin match.
+ if (tbs->verify_certificate(nullptr) == 1 || !pin_ok) {
+ *out_alert = SSL_AD_CERTIFICATE_UNKNOWN;
+ return ssl_verify_invalid;
+ }
+ return ssl_verify_ok;
+ }
+
+ // X.509 fallback: BoringSSL hands back the raw chain as CRYPTO_BUFFERs, not
X509 objects, so
+ // rebuild it and run the same verification SSL_CTX_set_verify() would
otherwise do for us.
+ const STACK_OF(CRYPTO_BUFFER) *chain = SSL_get0_peer_certificates(ssl);
+ if (chain == nullptr || sk_CRYPTO_BUFFER_num(chain) == 0) {
+ *out_alert = SSL_AD_CERTIFICATE_REQUIRED;
+ return ssl_verify_invalid;
+ }
+
+ X509 *leaf = nullptr;
+ STACK_OF(X509) *intermediates = sk_X509_new_null();
+ if (intermediates == nullptr) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return ssl_verify_invalid;
+ }
+ for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(chain); i++) {
+ const CRYPTO_BUFFER *buf = sk_CRYPTO_BUFFER_value(chain, i);
+ const uint8_t *data = CRYPTO_BUFFER_data(buf);
+ X509 *cert = d2i_X509(nullptr, &data,
CRYPTO_BUFFER_len(buf));
+ if (cert == nullptr) {
+ SSLError("failed to parse a client certificate offered to a RPK-enabled
context");
+ X509_free(leaf);
+ sk_X509_pop_free(intermediates, X509_free);
+ *out_alert = SSL_AD_BAD_CERTIFICATE;
+ return ssl_verify_invalid;
+ }
+ if (i == 0) {
+ leaf = cert;
+ } else {
+ sk_X509_push(intermediates, cert);
+ }
+ }
+
+ X509_STORE_CTX *store_ctx = X509_STORE_CTX_new();
+ bool initialized = store_ctx != nullptr && X509_STORE_CTX_init(store_ctx,
SSL_CTX_get_cert_store(ctx), leaf, intermediates);
Review Comment:
Fixed
--
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]
