Add support for cert verification to QUIC (#11787)

* Add support for cert verifycation to QUIC

* fix typos

include/iocore/net/TLSBasicSupport.h

@@ -47,11 +47,30 @@ class TLSBasicSupport
   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 @@ class TLSBasicSupport
   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;
 };

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;
 }

src/iocore/net/P_QUICNetVConnection.h

@@ -165,6 +165,7 @@ class QUICNetVConnection : public UnixNetVConnection,
   // 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 @@ class QUICNetVConnection : public UnixNetVConnection,
 
   QUICStreamManager  *_stream_manager  = nullptr;
   QUICApplicationMap *_application_map = nullptr;
+
+  bool _is_verifying_cert = false;
+  bool _is_cert_verified  = false;
 };
 
 extern ClassAllocator<QUICNetVConnection> quicNetVCAllocator;

src/iocore/net/P_SSLNetVConnection.h

@@ -126,12 +126,6 @@ class SSLNetVConnection : public UnixNetVConnection,
     return retval;
   }
 
-  SSLHandshakeStatus
-  getSSLHandshakeStatus() const
-  {
-    return sslHandshakeStatus;
-  }
-
   bool
   getSSLHandShakeComplete() const override
   {
@@ -252,21 +246,6 @@ class SSLNetVConnection : public UnixNetVConnection,
   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 @@ class SSLNetVConnection : public UnixNetVConnection,
     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 @@ class SSLNetVConnection : public UnixNetVConnection,
 
   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;

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()
 {

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];

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()
 {

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;
   }

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()
 {