This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new c1f013551f Add support for cert verification to QUIC (#11787)
c1f013551f is described below
commit c1f013551fcc04886138acbc56c3ec3aee345d6f
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Thu Sep 26 11:13:54 2024 -0600
Add support for cert verification to QUIC (#11787)
* Add support for cert verifycation to QUIC
* fix typos
---
include/iocore/net/TLSBasicSupport.h | 28 +++++++++++++++++++++++++
src/api/InkAPI.cc | 8 +++----
src/iocore/net/P_QUICNetVConnection.h | 4 ++++
src/iocore/net/P_SSLNetVConnection.h | 24 +--------------------
src/iocore/net/QUICNetVConnection.cc | 39 ++++++++++++++++++++++++++++++++++-
src/iocore/net/SSLClientUtils.cc | 13 ++++++------
src/iocore/net/SSLNetVConnection.cc | 21 +++++++++++++++++++
src/iocore/net/SSLUtils.cc | 14 ++++---------
src/iocore/net/TLSBasicSupport.cc | 17 +++++++++++++++
9 files changed, 123 insertions(+), 45 deletions(-)
diff --git a/include/iocore/net/TLSBasicSupport.h
b/include/iocore/net/TLSBasicSupport.h
index 4d64f4325c..c269c51c0f 100644
--- a/include/iocore/net/TLSBasicSupport.h
+++ b/include/iocore/net/TLSBasicSupport.h
@@ -47,11 +47,30 @@ public:
const char *get_tls_curve() const;
ink_hrtime get_tls_handshake_begin_time() const;
ink_hrtime get_tls_handshake_end_time() const;
+ /**
+ * Returns a certificate that need to be verified.
+ *
+ * Note: This function is only available when verify_certification is being
called.
+ * This function is probably just for TSVConnSslVerifyCTXGet.
+ * We could (and probably should) pass a cert to verify as an argument of
TS_EVENT_SSL_VERIFY_CLIENT/SERVER event.
+ */
+ X509_STORE_CTX *get_tls_cert_to_verify() const;
void set_valid_tls_version_min(int min);
void set_valid_tls_version_max(int max);
void set_valid_tls_protocols(unsigned long proto_mask, unsigned long
max_mask);
+ /**
+ * Give the plugin access to the data structure passed in during the
underlying
+ * openssl callback so the plugin can make more detailed decisions about the
+ * validity of the certificate in their cases.
+ *
+ * This function is supposed to be called from callback functions for TLS
libraries.
+ *
+ * @return 1 if verification failed
+ */
+ int verify_certificate(X509_STORE_CTX *ctx);
+
protected:
void clear();
@@ -61,9 +80,18 @@ protected:
void _record_tls_handshake_begin_time();
void _record_tls_handshake_end_time();
+ /**
+ * Implementation should schedule either TS_EVENT_SSL_VERIFY_SERVER or
TS_EVENT_SSL_VERIFY_CLIENT accordingly.
+ *
+ * @return 1 if verification failed
+ */
+ virtual int _verify_certificate(X509_STORE_CTX *ctx) = 0;
+
private:
static int _ex_data_index;
+ X509_STORE_CTX *_cert_to_verify = nullptr;
+
ink_hrtime _tls_handshake_begin_time = 0;
ink_hrtime _tls_handshake_end_time = 0;
};
diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc
index cf957ac97f..b455de1abd 100644
--- a/src/api/InkAPI.cc
+++ b/src/api/InkAPI.cc
@@ -7950,10 +7950,10 @@ TSVConnSslSniGet(TSVConn sslp, int *length)
TSSslVerifyCTX
TSVConnSslVerifyCTXGet(TSVConn sslp)
{
- NetVConnection *vc = reinterpret_cast<NetVConnection *>(sslp);
- SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection *>(vc);
- if (ssl_vc != nullptr) {
- return reinterpret_cast<TSSslVerifyCTX>(ssl_vc->get_verify_cert());
+ NetVConnection *vc = reinterpret_cast<NetVConnection *>(sslp);
+ TLSBasicSupport *tlsbs = vc->get_service<TLSBasicSupport>();
+ if (tlsbs != nullptr) {
+ return reinterpret_cast<TSSslVerifyCTX>(tlsbs->get_tls_cert_to_verify());
}
return nullptr;
}
diff --git a/src/iocore/net/P_QUICNetVConnection.h
b/src/iocore/net/P_QUICNetVConnection.h
index 019f870eaa..9797c24984 100644
--- a/src/iocore/net/P_QUICNetVConnection.h
+++ b/src/iocore/net/P_QUICNetVConnection.h
@@ -165,6 +165,7 @@ protected:
// TLSBasicSupport
SSL *_get_ssl_object() const override;
ssl_curve_id _get_tls_curve() const override;
+ int _verify_certificate(X509_STORE_CTX *ctx) override;
// TLSSNISupport
in_port_t _get_local_port() override;
@@ -235,6 +236,9 @@ private:
QUICStreamManager *_stream_manager = nullptr;
QUICApplicationMap *_application_map = nullptr;
+
+ bool _is_verifying_cert = false;
+ bool _is_cert_verified = false;
};
extern ClassAllocator<QUICNetVConnection> quicNetVCAllocator;
diff --git a/src/iocore/net/P_SSLNetVConnection.h
b/src/iocore/net/P_SSLNetVConnection.h
index edce1e1206..9b437a6d20 100644
--- a/src/iocore/net/P_SSLNetVConnection.h
+++ b/src/iocore/net/P_SSLNetVConnection.h
@@ -126,12 +126,6 @@ public:
return retval;
}
- SSLHandshakeStatus
- getSSLHandshakeStatus() const
- {
- return sslHandshakeStatus;
- }
-
bool
getSSLHandShakeComplete() const override
{
@@ -252,21 +246,6 @@ public:
bool protocol_mask_set = false;
unsigned long protocol_mask = 0;
- // Only applies during the VERIFY certificate hooks (client and server side)
- // Means to give the plugin access to the data structure passed in during
the underlying
- // openssl callback so the plugin can make more detailed decisions about the
- // validity of the certificate in their cases
- X509_STORE_CTX *
- get_verify_cert()
- {
- return verify_cert;
- }
- void
- set_verify_cert(X509_STORE_CTX *ctx)
- {
- verify_cert = ctx;
- }
-
bool
peer_provided_cert() const override
{
@@ -327,6 +306,7 @@ protected:
return this->ssl;
}
ssl_curve_id _get_tls_curve() const override;
+ int _verify_certificate(X509_STORE_CTX *ctx) override;
// TLSSessionResumptionSupport
const IpEndpoint &
@@ -376,8 +356,6 @@ private:
int64_t redoWriteSize = 0;
- X509_STORE_CTX *verify_cert = nullptr;
-
// Null-terminated string, or nullptr if there is no SNI server name.
std::unique_ptr<char[]> _ca_cert_file;
std::unique_ptr<char[]> _ca_cert_dir;
diff --git a/src/iocore/net/QUICNetVConnection.cc
b/src/iocore/net/QUICNetVConnection.cc
index d6c002221c..2fd225df24 100644
--- a/src/iocore/net/QUICNetVConnection.cc
+++ b/src/iocore/net/QUICNetVConnection.cc
@@ -769,8 +769,13 @@ QUICNetVConnection::get_quic_connection()
}
void
-QUICNetVConnection::reenable(int /* event ATS_UNUSED */)
+QUICNetVConnection::reenable(int event)
{
+ this->_is_verifying_cert = false;
+
+ if (event == TS_EVENT_ERROR) {
+ this->_is_cert_verified = false;
+ }
}
Continuation *
@@ -807,6 +812,38 @@ QUICNetVConnection::_get_tls_curve() const
}
}
+int
+QUICNetVConnection::_verify_certificate(X509_STORE_CTX * /* ctx ATS_UNUSED */)
+{
+ TSEvent eventId;
+ TSHttpHookID hookId;
+ APIHook *hook = nullptr;
+
+ // TODO Simply call callHooks once QUICNetVC implements TLSEventSupport
+ if (get_context() == NET_VCONNECTION_IN) {
+ eventId = TS_EVENT_SSL_VERIFY_CLIENT;
+ hookId = TS_SSL_VERIFY_CLIENT_HOOK;
+ } else {
+ eventId = TS_EVENT_SSL_VERIFY_SERVER;
+ hookId = TS_SSL_VERIFY_SERVER_HOOK;
+ }
+ hook = SSLAPIHooks::instance()->get(TSSslHookInternalID(hookId));
+ if (hook != nullptr) {
+ this->_is_verifying_cert = true;
+ WEAK_SCOPED_MUTEX_LOCK(lock, hook->m_cont->mutex, this_ethread());
+ hook->invoke(eventId, this);
+ }
+
+ // According to the implementation in SSLNetVC,
+ // we can assume that reenable() is called during the event handling.
+ ink_assert(this->_is_verifying_cert == false);
+ if (this->_is_cert_verified) {
+ return 1;
+ }
+
+ return 0;
+}
+
in_port_t
QUICNetVConnection::_get_local_port()
{
diff --git a/src/iocore/net/SSLClientUtils.cc b/src/iocore/net/SSLClientUtils.cc
index aef0ecf9f3..851351d2bf 100644
--- a/src/iocore/net/SSLClientUtils.cc
+++ b/src/iocore/net/SSLClientUtils.cc
@@ -130,15 +130,14 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
return !enforce_mode;
}
}
+
// If the previous configured checks passed, give the hook a try
- netvc->set_verify_cert(ctx);
- TLSEventSupport *es = TLSEventSupport::getInstance(ssl);
- if (es) {
- es->callHooks(TS_EVENT_SSL_VERIFY_SERVER);
+ TLSBasicSupport *tbs = TLSBasicSupport::getInstance(ssl);
+ if (tbs == nullptr) {
+ Dbg(dbg_ctl_ssl_verify, "call back on stale netvc");
+ return false;
}
- netvc->set_verify_cert(nullptr);
-
- if (netvc->getSSLHandshakeStatus() ==
SSLHandshakeStatus::SSL_HANDSHAKE_ERROR) {
+ if (tbs->verify_certificate(ctx) == 1) {
// Verify server hook failed and set the status to SSL_HANDSHAKE_ERROR
unsigned char *sni_name;
char buff[INET6_ADDRSTRLEN];
diff --git a/src/iocore/net/SSLNetVConnection.cc
b/src/iocore/net/SSLNetVConnection.cc
index 0ca851ebc8..d49c79ec28 100644
--- a/src/iocore/net/SSLNetVConnection.cc
+++ b/src/iocore/net/SSLNetVConnection.cc
@@ -1964,6 +1964,27 @@ SSLNetVConnection::_get_tls_curve() const
}
}
+int
+SSLNetVConnection::_verify_certificate(X509_STORE_CTX * /* ctx ATS_UNUSED */)
+{
+ // Currently, TS_EVENT_SSL_VERIFY_CLIENT/SERVER are invoked only with a
NetVC instance.
+ // This requires plugins to call TSSslVerifyCTX in their event handler.
+ // We could pass a structure that has both a cert to verify and a NetVC.
+ // It would allow us to remove confusing TSSslVerifyCTX and its internal
implementation that are only available during a very
+ // limited time.
+ if (get_context() == NET_VCONNECTION_IN) {
+ this->callHooks(TS_EVENT_SSL_VERIFY_CLIENT /* , ctx */);
+ } else {
+ this->callHooks(TS_EVENT_SSL_VERIFY_SERVER /* , ctx */);
+ }
+
+ if (this->sslHandshakeStatus == SSLHandshakeStatus::SSL_HANDSHAKE_ERROR) {
+ return 1;
+ }
+
+ return 0;
+}
+
ssl_error_t
SSLNetVConnection::_ssl_accept()
{
diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc
index 95680bccb3..b519a0778f 100644
--- a/src/iocore/net/SSLUtils.cc
+++ b/src/iocore/net/SSLUtils.cc
@@ -283,20 +283,14 @@ ssl_verify_client_callback(int preverify_ok,
X509_STORE_CTX *ctx)
Dbg(dbg_ctl_ssl_verify, "Callback: verify client cert");
auto *ssl = static_cast<SSL
*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+ TLSBasicSupport *tbs = TLSBasicSupport::getInstance(ssl);
- if (!netvc || netvc->ssl != ssl) {
- Dbg(dbg_ctl_ssl_verify, "ssl_verify_client_callback call back on stale
netvc");
+ if (tbs == nullptr) {
+ Dbg(dbg_ctl_ssl_verify, "call back on stale netvc");
return false;
}
- netvc->set_verify_cert(ctx);
- TLSEventSupport *es = TLSEventSupport::getInstance(ssl);
- if (es) {
- es->callHooks(TS_EVENT_SSL_VERIFY_CLIENT);
- }
- netvc->set_verify_cert(nullptr);
-
- if (netvc->getSSLHandShakeComplete()) { // hook moved the handshake state to
terminal
+ if (tbs->verify_certificate(ctx) == 1) { // hook moved the handshake state
to terminal
Warning("TS_EVENT_SSL_VERIFY_CLIENT plugin failed the client certificate
check for %s.", netvc->options.sni_servername.get());
return false;
}
diff --git a/src/iocore/net/TLSBasicSupport.cc
b/src/iocore/net/TLSBasicSupport.cc
index d9b4d1e40b..723bdb16c2 100644
--- a/src/iocore/net/TLSBasicSupport.cc
+++ b/src/iocore/net/TLSBasicSupport.cc
@@ -166,6 +166,23 @@ TLSBasicSupport::set_valid_tls_version_max(int max)
SSL_set_max_proto_version(ssl, ver);
}
+int
+TLSBasicSupport::verify_certificate(X509_STORE_CTX *ctx)
+{
+ // See comments in SSLNetVConnection::_verify_certificate
+ this->_cert_to_verify = ctx;
+ int ret = this->_verify_certificate(ctx);
+ this->_cert_to_verify = nullptr;
+
+ return ret;
+}
+
+X509_STORE_CTX *
+TLSBasicSupport::get_tls_cert_to_verify() const
+{
+ return this->_cert_to_verify;
+}
+
void
TLSBasicSupport::_record_tls_handshake_begin_time()
{
