This is an automated email from the ASF dual-hosted git repository. shinrich pushed a commit to branch avx-53179 in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 82b7f9dc8d9ba6fc8e494359362148ad782eaead Author: Susan Hinrichs <[email protected]> AuthorDate: Wed Jul 10 20:25:57 2024 +0000 AVX-53179: Make 'Ensure TLS' an action and not a match condition --- aviatrix/plugins/avx_policy_driver/policy.h | 1 + .../plugins/avx_policy_driver/policy_driver.cc | 4 +- aviatrix/plugins/avx_policy_driver/web_filter.cc | 90 +++++++++++----------- 3 files changed, 48 insertions(+), 47 deletions(-) diff --git a/aviatrix/plugins/avx_policy_driver/policy.h b/aviatrix/plugins/avx_policy_driver/policy.h index 72ffbd163b..eb76587806 100644 --- a/aviatrix/plugins/avx_policy_driver/policy.h +++ b/aviatrix/plugins/avx_policy_driver/policy.h @@ -18,6 +18,7 @@ enum PolicyResult { POLICY_IDS, POLICY_CONTINUE, POLICY_ERROR, + POLICY_REQUIRE_TLS, POLICY_END, }; diff --git a/aviatrix/plugins/avx_policy_driver/policy_driver.cc b/aviatrix/plugins/avx_policy_driver/policy_driver.cc index 0acabbfa7e..19898e8fe9 100644 --- a/aviatrix/plugins/avx_policy_driver/policy_driver.cc +++ b/aviatrix/plugins/avx_policy_driver/policy_driver.cc @@ -145,6 +145,7 @@ policy_sni(TSCont cont, TSEvent event, void *edata) TSUserArgSet(ssl_vc, CertifierPolicyIndex, reinterpret_cast<void *>(static_cast<intptr_t>(1))); TSVConnReenable(ssl_vc); break; + case POLICY_REQUIRE_TLS: case POLICY_DROP: default: Dbg(dbg_ctl, "Filter drop"); @@ -188,9 +189,10 @@ transaction_start(TSCont cont, TSEvent event, void *edata) // Continue processing TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); break; + case POLICY_REQUIRE_TLS: case POLICY_DROP: default: - Dbg(dbg_ctl, "URL Filter drop"); + Dbg(dbg_ctl, "Txn Filter drop"); // TODO, I assume continuing with ERROR will cause the transaction to fail // This will end up returning a 50x status code, we could replace this with a 401 // unauthorized or something configurable to the customer's pleasing. We would need to diff --git a/aviatrix/plugins/avx_policy_driver/web_filter.cc b/aviatrix/plugins/avx_policy_driver/web_filter.cc index 3a90a3ee16..e7437bd2dd 100644 --- a/aviatrix/plugins/avx_policy_driver/web_filter.cc +++ b/aviatrix/plugins/avx_policy_driver/web_filter.cc @@ -85,14 +85,14 @@ public: private: bool logAndWatchResult(const char *stage, PolicyResult result, struct sockaddr_in *client_addr, struct sockaddr_in *server_addr, const char *sni_hostname, const char *url, int policy_offset, int decrypt_policy_offset); - PolicyResult checkAction(int policy_offset); + PolicyResult checkAction(TSVConn vc, int policy_offset); MatchResult MatchTuple(int policy_offset, struct sockaddr_in *client_addr, struct sockaddr_in *server_addr); MatchResult MatchWebFilter(int policy_offset, std::string_view sni_target, std::string_view url_target); bool HasWebFilters(int policy_offset); bool IsLogPolicy(int policy_offset); bool IsWatchPolicy(int policy_offset); - bool MeetFlowRequirements(int policy_offset, TSHttpTxn txnp); bool AllowsDecrypt(int policy_offset); + bool TlsRequirementMet(TSVConn vc, int policy_offst); void SetGenerationNumber(PolicyEvalInfo *policy_eval_info); int GetGenerationNumber(PolicyEvalInfo *policy_eval_info); void SetPolicyOffset(PolicyEvalInfo *policy_eval_info, int policy_offset); @@ -317,11 +317,35 @@ PolicyHolder::AllowsDecrypt(int policy_offset) return false; } +bool +PolicyHolder::TlsRequirementMet(TSVConn vc, int policy_offset) +{ + if (policy_offset < this->current_config->policies_size()) { + const Layer7Policy &policy = this->current_config->policies(policy_offset); + if (policy.flow_app_requirement() == Layer7Policy_FlowApp_TLS_REQUIRED) { + Dbg(dbg_ctl, "Check TLS_REQUIRED"); + if (!TSVConnIsSsl(vc)) { + Dbg(dbg_ctl, "Failed TLS_REQUIRED"); + return false; + } + } + Dbg(dbg_ctl, "Passed TLS_REQUIRED"); + return true; + } + Dbg(dbg_ctl, "Check TLS_REQUIRED includes too high %d", policy_offset); + return false; +} + PolicyResult -PolicyHolder::checkAction(int policy_offset) +PolicyHolder::checkAction(TSVConn vc, int policy_offset) { if (policy_offset < this->current_config->policies_size()) { const Layer7Policy &policy = this->current_config->policies(policy_offset); + // Is this required to be TLS? + if (!this->TlsRequirementMet(vc, policy_offset)) { + return POLICY_REQUIRE_TLS; + } + // Passed the TLS requirement if present switch (policy.action()) { case Layer7Policy_Action_INTRUSION_DETECTION: return POLICY_IDS; @@ -483,24 +507,28 @@ PolicyHolder::internalEvaluatePolicySni(TSVConn ssl_vc, PolicyEvalInfo *policy_e auto web_filter_result = this->MatchWebFilter(i, sni_view, empty_url_view); if (web_filter_result == MatchValues) { Dbg(dbg_ctl, "Match SNI"); - result = this->checkAction(i); + result = this->checkAction(ssl_vc, i); } else if (web_filter_result == NoAttributeToMatch) { if (this->HasWebFilters(i)) { - Dbg(dbg_ctl, "Evaluate URL filter"); - // Are we allowed to decrypt, if not fail now - if (!this->AllowsDecrypt(i)) { - Dbg(dbg_ctl, "Decrypt is not allowed, drop"); - result = POLICY_DROP; - // The web filters must be for URL - } else if (i >= this->current_config->policies_size()) { - result = POLICY_END; + if (!this->TlsRequirementMet(ssl_vc, policy_offset)) { + result = POLICY_REQUIRE_TLS; } else { - result = POLICY_CONTINUE; + Dbg(dbg_ctl, "Evaluate URL filter"); + // Are we allowed to decrypt, if not fail now + if (!this->AllowsDecrypt(i)) { + Dbg(dbg_ctl, "Decrypt is not allowed, drop"); + result = POLICY_DROP; + // The web filters must be for URL + } else if (i >= this->current_config->policies_size()) { + result = POLICY_END; + } else { + result = POLICY_CONTINUE; + } } } else { // Otherwise there were no web_filters on this rule, so we are done Dbg(dbg_ctl, "No SNI filter"); - result = this->checkAction(i); + result = this->checkAction(ssl_vc, i); } } // Otherwise, look at the next policy } else if (tuple_result != NotMatchValues) { @@ -552,33 +580,6 @@ EvaluatePolicyTxn(TSHttpTxn txnp, uint64_t *policy_eval_info) return activePolicy.internalEvaluatePolicyTxn(txnp, temp); } -bool -PolicyHolder::MeetFlowRequirements(int policy_offset, TSHttpTxn txnp) -{ - if (policy_offset >= this->current_config->policies_size() || policy_offset < 0) { - Dbg(dbg_ctl, "FlowRequirements: No rule offset=%d size=%d", policy_offset, this->current_config->policies_size()); - return false; - } - const Layer7Policy &policy = this->current_config->policies(policy_offset); - // Is this required to be TLS? - if (policy.flow_app_requirement() == Layer7Policy_FlowApp_TLS_REQUIRED) { - TSHttpSsn ssnp = TSHttpTxnSsnGet(txnp); - if (ssnp == nullptr) { - TSError("[%s] Failed to access Txn Session", PLUGIN_NAME); - return false; - } - TSVConn vc = TSHttpSsnClientVConnGet(ssnp); - if (vc == nullptr) { - TSError("[%s] Failed to access Txn connection", PLUGIN_NAME); - return false; - } - bool retval = TSVConnIsSsl(vc); - Dbg(dbg_ctl, "Enforce TLS test=%d", retval); - return retval; - } - return true; // Don't care if this is TLS -} - PolicyResult PolicyHolder::internalEvaluatePolicyTxn(TSHttpTxn txnp, PolicyEvalInfo *policy_eval_info) { @@ -649,16 +650,13 @@ PolicyHolder::internalEvaluatePolicyTxn(TSHttpTxn txnp, PolicyEvalInfo *policy_e continue; } - if (!this->MeetFlowRequirements(i, txnp)) { - // Does not match the TLS requirement, try the next rule - continue; - } + auto vc = TSHttpSsnClientVConnGet(TSHttpTxnSsnGet(txnp)); auto tuple_result = this->MatchTuple(i, &client_addr, &origin_addr); if (tuple_result == MatchValues) { // Check the SNI auto web_filter_result = this->MatchWebFilter(i, sni_view, url_view); if (web_filter_result == MatchValues || web_filter_result == NoAttributeToMatch) { - result = this->checkAction(i); + result = this->checkAction(vc, i); // need this for logging if (isFinalPolicyResult(result)) {
