From 26a2428512f242a591631d2b8b1ac89cb4c858f1 Mon Sep 17 00:00:00 2001 From: Minei3oat Date: Tue, 25 Feb 2025 00:59:58 +0100 Subject: [PATCH 1/4] Fix metrics endpoint --- backend/routers/nfregex.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/routers/nfregex.py b/backend/routers/nfregex.py index ae09d22..f3a4498 100644 --- a/backend/routers/nfregex.py +++ b/backend/routers/nfregex.py @@ -340,7 +340,6 @@ async def metrics(): s.name, s.status, r.regex, - r.is_blacklist, r.mode, r.is_case_sensitive, r.blocked_packets, From 7cc005dfb23e47e66dfb7e478d494caed2905ef8 Mon Sep 17 00:00:00 2001 From: Domingo Dirutigliano Date: Tue, 25 Feb 2025 11:18:30 +0100 Subject: [PATCH 2/4] TCP packet used in matching fixed --- backend/binsrc/regex/regexfilter.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/binsrc/regex/regexfilter.cpp b/backend/binsrc/regex/regexfilter.cpp index 0ea15d2..bfb2407 100644 --- a/backend/binsrc/regex/regexfilter.cpp +++ b/backend/binsrc/regex/regexfilter.cpp @@ -179,6 +179,8 @@ public: static void on_data_recv(Stream& stream, RegexNfQueue* nfq, string data) { nfq->match_ctx.matching_has_been_called = true; nfq->match_ctx.already_closed = false; + nfq->match_ctx.pkt->data = data.data(); + nfq->match_ctx.pkt->data_size = data.size(); bool result = nfq->filter_action(nfq->match_ctx.pkt); if (!result){ nfq->sctx.clean_stream_by_id(nfq->match_ctx.pkt->sid); @@ -226,4 +228,4 @@ public: }; }} -#endif // REGEX_FILTER_CLASS_CPP \ No newline at end of file +#endif // REGEX_FILTER_CLASS_CPP From c979ad21eaedc65ec49922aaa898e56bd0b2cf1b Mon Sep 17 00:00:00 2001 From: Minei3oat Date: Wed, 5 Mar 2025 08:51:55 +0100 Subject: [PATCH 3/4] Add tests for metrics endpoint --- tests/nf_test.py | 34 ++++++++++++++++++++++++++++++---- tests/utils/firegexapi.py | 4 ++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/tests/nf_test.py b/tests/nf_test.py index cc780bd..1fb8260 100644 --- a/tests/nf_test.py +++ b/tests/nf_test.py @@ -81,7 +81,12 @@ else: #Check if regex is present in the service n_blocked = 0 -def checkRegex(regex, should_work=True, upper=False): +def getMetric(metric_name, regex): + for metric in firegex.nf_get_metrics().split("\n"): + if metric.startswith(metric_name + "{") and f'regex="{regex}"' in metric: + return int(metric.split(" ")[-1]) + +def checkRegex(regex, should_work=True, upper=False, deleted=False): if should_work: global n_blocked for r in firegex.nf_get_service_regexes(service_id): @@ -93,9 +98,19 @@ def checkRegex(regex, should_work=True, upper=False): n_blocked += 1 time.sleep(1) if firegex.nf_get_regex(r["id"])["n_packets"] == n_blocked: - puts("The packed was reported as blocked ✔", color=colors.green) + puts("The packet was reported as blocked in the API ✔", color=colors.green) else: - puts("Test Failed: The packed wasn't reported as blocked ✗", color=colors.red) + puts("Test Failed: The packet wasn't reported as blocked in the API ✗", color=colors.red) + exit_test(1) + if getMetric("firegex_blocked_packets", secret.decode()) == n_blocked: + puts("The packet was reported as blocked in the metrics ✔", color=colors.green) + else: + puts("Test Failed: The packet wasn't reported as blocked in the metrics ✗", color=colors.red) + exit_test(1) + if getMetric("firegex_active", secret.decode()) == 1: + puts("The regex was reported as active in the metrics ✔", color=colors.green) + else: + puts("Test Failed: The regex wasn't reported as active in the metrics ✗", color=colors.red) exit_test(1) else: puts("Test Failed: The request wasn't blocked ✗", color=colors.red) @@ -109,6 +124,12 @@ def checkRegex(regex, should_work=True, upper=False): else: puts("Test Failed: The request was blocked when it shouldn't have", color=colors.red) exit_test(1) + if not deleted: + if getMetric("firegex_active", secret.decode()) == 0: + puts("The regex was reported as inactive in the metrics ✔", color=colors.green) + else: + puts("Test Failed: The regex wasn't reported as inactive in the metrics ✗", color=colors.red) + exit_test(1) def clear_regexes(): global n_blocked @@ -121,6 +142,11 @@ def clear_regexes(): puts("Test Failed: Coulnd't delete the regex ✗", color=colors.red) exit_test(1) break + if f'regex="{secret.decode()}"' not in firegex.nf_get_metrics(): + puts(f"No regex metrics after deletion ✔", color=colors.green) + else: + puts("Test Failed: Metrics found after deleting the regex ✗", color=colors.red) + exit_test(1) checkRegex(regex) @@ -172,7 +198,7 @@ checkRegex(regex) clear_regexes() #Check if it's actually deleted -checkRegex(regex,should_work=False) +checkRegex(regex,should_work=False,deleted=True) #Add case insensitive regex if(firegex.nf_add_regex(service_id,regex,"B",active=True, is_case_sensitive=False)): diff --git a/tests/utils/firegexapi.py b/tests/utils/firegexapi.py index 13f7a0c..2e878c5 100644 --- a/tests/utils/firegexapi.py +++ b/tests/utils/firegexapi.py @@ -132,6 +132,10 @@ class FiregexAPI: json={"name":name,"port":port, "proto": proto, "ip_int": ip_int}) return req.json()["service_id"] if verify(req) else False + def nf_get_metrics(self): + req = self.s.get(f"{self.address}api/nfregex/metrics") + return req.text + #PortHijack def ph_get_services(self): req = self.s.get(f"{self.address}api/porthijack/services") From 5a523817cca2b39d814d749648f856ebe917732d Mon Sep 17 00:00:00 2001 From: Domingo Dirutigliano Date: Wed, 5 Mar 2025 10:04:28 +0100 Subject: [PATCH 4/4] Update regexfilter.cpp --- backend/binsrc/regex/regexfilter.cpp | 118 ++++++++++----------------- 1 file changed, 41 insertions(+), 77 deletions(-) diff --git a/backend/binsrc/regex/regexfilter.cpp b/backend/binsrc/regex/regexfilter.cpp index bfb2407..2aef9ff 100644 --- a/backend/binsrc/regex/regexfilter.cpp +++ b/backend/binsrc/regex/regexfilter.cpp @@ -20,6 +20,7 @@ #include "../classes/netfilter.cpp" #include "stream_ctx.cpp" #include "regex_rules.cpp" +#include "../utils.cpp" using namespace std; @@ -30,22 +31,14 @@ namespace Regex { using Tins::TCPIP::Stream; using Tins::TCPIP::StreamFollower; - - class RegexNfQueue : public NfQueue::ThreadNfQueue { public: stream_ctx sctx; u_int16_t latest_config_ver = 0; StreamFollower follower; - struct { - bool matching_has_been_called = false; - bool already_closed = false; - bool result; - NfQueue::PktRequest* pkt; - } match_ctx; - + NfQueue::PktRequest* pkt; - bool filter_action(NfQueue::PktRequest* pkt){ + bool filter_action(NfQueue::PktRequest* pkt, const string& data){ shared_ptr conf = regex_config; auto current_version = conf->ver(); @@ -91,12 +84,12 @@ public: stream_match = stream_search->second; } err = hs_scan_stream( - stream_match,pkt->data, pkt->data_size, + stream_match, data.c_str(), data.size(), 0, scratch_space, match_func, &match_res ); }else{ err = hs_scan( - regex_matcher,pkt->data, pkt->data_size, + regex_matcher, data.c_str(), data.size(), 0, scratch_space, match_func, &match_res ); } @@ -108,7 +101,7 @@ public: throw invalid_argument("Cannot close stream match on hyperscan"); } if (err != HS_SUCCESS && err != HS_SCAN_TERMINATED) { - cerr << "[error] [filter_callback] Error while matching the stream (hs)" << endl; + cerr << "[error] [filter_callback] Error while matching the stream (hs) " << err << endl; throw invalid_argument("Error while matching the stream with hyperscan"); } if (match_res.has_matched){ @@ -119,85 +112,30 @@ public: return true; } - void handle_next_packet(NfQueue::PktRequest* pkt) override{ - bool empty_payload = pkt->data_size == 0; - if (pkt->tcp){ - match_ctx.matching_has_been_called = false; - match_ctx.pkt = pkt; - - if (pkt->ipv4){ - follower.process_packet(*pkt->ipv4); - }else{ - follower.process_packet(*pkt->ipv6); - } - - // Do an action only is an ordered packet has been received - if (match_ctx.matching_has_been_called){ - - //In this 2 cases we have to remove all data about the stream - if (!match_ctx.result || match_ctx.already_closed){ - sctx.clean_stream_by_id(pkt->sid); - //If the packet has data, we have to remove it - if (!empty_payload){ - Tins::PDU* data_layer = pkt->tcp->release_inner_pdu(); - if (data_layer != nullptr){ - delete data_layer; - } - } - //For the first matched data or only for data packets, we set FIN bit - //This only for client packets, because this will trigger server to close the connection - //Packets will be filtered anyway also if client don't send packets - if ((!match_ctx.result || !empty_payload) && pkt->is_input){ - pkt->tcp->set_flag(Tins::TCP::FIN,1); - pkt->tcp->set_flag(Tins::TCP::ACK,1); - pkt->tcp->set_flag(Tins::TCP::SYN,0); - } - //Send the edited packet to the kernel - return pkt->mangle(); - } - } - return pkt->accept(); - }else{ - if (!pkt->udp){ - throw invalid_argument("Only TCP and UDP are supported"); - } - if(empty_payload){ - return pkt->accept(); - }else if (filter_action(pkt)){ - return pkt->accept(); - }else{ - return pkt->drop(); - } - } - } //If the stream has already been matched, drop all data, and try to close the connection static void keep_fin_packet(RegexNfQueue* nfq){ - nfq->match_ctx.matching_has_been_called = true; - nfq->match_ctx.already_closed = true; + nfq->pkt->reject(); // This is needed because the callback has to take the updated pkt pointer! } - static void on_data_recv(Stream& stream, RegexNfQueue* nfq, string data) { - nfq->match_ctx.matching_has_been_called = true; - nfq->match_ctx.already_closed = false; - nfq->match_ctx.pkt->data = data.data(); - nfq->match_ctx.pkt->data_size = data.size(); - bool result = nfq->filter_action(nfq->match_ctx.pkt); - if (!result){ - nfq->sctx.clean_stream_by_id(nfq->match_ctx.pkt->sid); + static void on_data_recv(Stream& stream, RegexNfQueue* nfq, const string& data) { + if (!nfq->filter_action(nfq->pkt, data)){ + nfq->sctx.clean_stream_by_id(nfq->pkt->sid); stream.client_data_callback(bind(keep_fin_packet, nfq)); stream.server_data_callback(bind(keep_fin_packet, nfq)); + nfq->pkt->reject(); } - nfq->match_ctx.result = result; } //Input data filtering static void on_client_data(Stream& stream, RegexNfQueue* nfq) { - on_data_recv(stream, nfq, string(stream.client_payload().begin(), stream.client_payload().end())); + auto data = stream.client_payload(); + on_data_recv(stream, nfq, string((char*)data.data(), data.size())); } //Server data filtering static void on_server_data(Stream& stream, RegexNfQueue* nfq) { - on_data_recv(stream, nfq, string(stream.server_payload().begin(), stream.server_payload().end())); + auto data = stream.server_payload(); + on_data_recv(stream, nfq, string((char*)data.data(), data.size())); } // A stream was terminated. The second argument is the reason why it was terminated @@ -216,6 +154,32 @@ public: stream.stream_closed_callback(bind(on_stream_close, placeholders::_1, nfq)); } + void handle_next_packet(NfQueue::PktRequest* _pkt) override{ + pkt = _pkt; // Setting packet context + if (pkt->tcp){ + if (pkt->ipv4){ + follower.process_packet(*pkt->ipv4); + }else{ + follower.process_packet(*pkt->ipv6); + } + //Fallback to the default action + if (pkt->get_action() == NfQueue::FilterAction::NOACTION){ + return pkt->accept(); + } + }else{ + if (!pkt->udp){ + throw invalid_argument("Only TCP and UDP are supported"); + } + if(pkt->data_size() == 0){ + return pkt->accept(); + }else if (filter_action(pkt, string(pkt->data(), pkt->data_size()))){ + return pkt->accept(); + }else{ + return pkt->drop(); + } + } + } + void before_loop() override{ follower.new_stream_callback(bind(on_new_stream, placeholders::_1, this)); follower.stream_termination_callback(bind(on_stream_close, placeholders::_1, this));