maskit commented on code in PR #13548:
URL: https://github.com/apache/trafficserver/pull/13548#discussion_r3780114606
##########
src/iocore/net/SSLClientUtils.cc:
##########
@@ -155,6 +225,171 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
return true;
}
+#if HAVE_SSL_CREDENTIAL_NEW_RAW_PUBLIC_KEY
+// BoringSSL rejects raw public keys outright unless a custom verify callback
is installed, and
+// SSL_set_custom_verify() displaces SSL_set_verify() (and with it BoringSSL's
automatic chain
+// verification) for the whole connection. So this callback owns both cases:
pin the peer's raw
+// public key, or -- when the next hop negotiated X.509 after all, the normal
state mid-rollout --
+// rebuild and verify the chain by hand before deferring to the usual
policy/name/hook logic.
+static enum ssl_verify_result_t
+ssl_client_custom_verify_callback(SSL *ssl, uint8_t *out_alert)
+{
+ SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+ if (netvc == nullptr) {
+ Dbg(dbg_ctl_ssl_verify, "WARNING, NetVC is NULL in custom cert verify
callback");
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return ssl_verify_invalid;
+ }
+ if (netvc->options.verifyServerPolicy == YamlSNIConfig::Policy::DISABLED) {
+ return ssl_verify_ok;
+ }
+
+ bool const enforce_mode = netvc->options.verifyServerPolicy ==
YamlSNIConfig::Policy::ENFORCED;
+
+ TLSBasicSupport *tbs = TLSBasicSupport::getInstance(ssl);
+ if (tbs == nullptr) {
+ Dbg(dbg_ctl_ssl_verify, "custom verify callback on stale netvc");
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return ssl_verify_invalid;
+ }
+
+ if (SSL_get_peer_cert_type(ssl) == TLSEXT_cert_type_rpk) {
+ EVP_PKEY *peer_rpk = SSL_get0_peer_rpk(ssl);
+ const SSLRPKUtils::TrustedKeySet *trusted = ssl_get_trusted_rpk(ssl);
+ bool pin_ok = trusted != nullptr &&
SSLRPKUtils::pinnedKeyMatches(peer_rpk, *trusted);
+ Dbg(dbg_ctl_ssl_verify, "Origin authenticated with a raw public key (RFC
7250), pin match=%s", pin_ok ? "yes" : "no");
+ if (!pin_ok) {
+ char buff[INET6_ADDRSTRLEN];
+ ats_ip_ntop(netvc->get_effective_remote_addr(), buff, INET6_ADDRSTRLEN);
+ Warning("Origin raw public key did not match any trusted key. Action=%s
server=%s(%s)",
+ enforce_mode ? "Terminate" : "Continue",
netvc->options.ssl_servername.get(), buff);
+ }
+
+ // There is no X509_STORE_CTX to hand the hook for a raw public key, but
the hook still runs
+ // on every attempt, as on the X.509 paths.
+ if (tbs->verify_certificate(nullptr) == 1) {
+ Warning("TS_EVENT_SSL_VERIFY_SERVER plugin failed the origin raw public
key check for %s. Action=%s",
+ netvc->options.ssl_servername.get(), enforce_mode ? "Terminate"
: "Continue");
+ if (enforce_mode) {
+ *out_alert = SSL_AD_CERTIFICATE_UNKNOWN;
+ return ssl_verify_invalid;
+ }
+ return ssl_verify_ok;
+ }
+ if (!pin_ok && enforce_mode) {
+ *out_alert = SSL_AD_CERTIFICATE_UNKNOWN;
+ return ssl_verify_invalid;
+ }
+ return ssl_verify_ok;
+ }
+
+ // X.509 fallback. Rebuild the chain BoringSSL hands back as CRYPTO_BUFFERs
so the shared
+ // verify_callback() logic (signature/name/policy/hook) can run against a
real X509_STORE_CTX.
+ const STACK_OF(CRYPTO_BUFFER) *chain = SSL_get0_peer_certificates(ssl);
+ if (chain == nullptr || sk_CRYPTO_BUFFER_num(chain) == 0) {
+ if (enforce_mode) {
+ *out_alert = SSL_AD_CERTIFICATE_REQUIRED;
+ return ssl_verify_invalid;
+ }
+ return ssl_verify_ok;
+ }
+
+ 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 an origin certificate on a RPK-enabled
connection");
+ 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);
+ }
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);
+ }
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]
