From 73b915a28230c5a923ba43c564f5e13386f84279 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 5 Jan 2024 15:26:06 +0100 Subject: [PATCH] ldap: Fix substring filter parsing and rendering The initial (prefix) and final (suffix) strings are specified individually with a variable number of "any" matches that can occur between these. The previous implementation assumed a single string and rendered it as **. Reported and PCAP provided by @martinvanhensbergen, thanks! Closes #27 --- analyzer/ldap.spicy | 43 +++++++++++++++--- tests/analyzer/ldap_substring_search.zeek | 12 +++++ .../analyzer.ldap_substring_search/conn.log | 4 ++ .../ldap_search.log | 12 +++++ .../analyzer.ldap_substring_search/output | 2 + tests/traces/ldap_star_single.pcap | Bin 0 -> 636 bytes 6 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 tests/analyzer/ldap_substring_search.zeek create mode 100644 tests/baseline/analyzer.ldap_substring_search/conn.log create mode 100644 tests/baseline/analyzer.ldap_substring_search/ldap_search.log create mode 100644 tests/baseline/analyzer.ldap_substring_search/output create mode 100644 tests/traces/ldap_star_single.pcap diff --git a/analyzer/ldap.spicy b/analyzer/ldap.spicy index a7be8af..2bf45a9 100644 --- a/analyzer/ldap.spicy +++ b/analyzer/ldap.spicy @@ -601,8 +601,14 @@ public function string_representation(search_filter: SearchFilter): string { search_filter.FILTER_LE.assertionValueDecoded); } case FilterType::FILTER_SUBSTR: { - repr = "(%s=*%s*)" % (search_filter.FILTER_SUBSTR.attributeDesc.decode(), - search_filter.FILTER_SUBSTR.assertionValueDecoded); + local anys: string = ""; + if ( |search_filter.FILTER_SUBSTR.anys| > 0 ) + anys = b"*".join(search_filter.FILTER_SUBSTR.anys).decode() + "*"; + + repr = "(%s=%s*%s%s)" % (search_filter.FILTER_SUBSTR.attributeDesc.decode(), + search_filter.FILTER_SUBSTR.initial, + anys, + search_filter.FILTER_SUBSTR.final); } case FilterType::FILTER_PRESENT: { repr = "(%s=*)" % search_filter.FILTER_PRESENT; @@ -620,10 +626,6 @@ type DecodedAttributeValue = unit(fType: FilterType) { attributeDesc_len: uint8; attributeDesc: bytes &size=self.attributeDesc_len; - # For some reason, two intermediate uint8 values are present in the FILTER_SUBSTR type. - : uint8 if ( fType == FilterType::FILTER_SUBSTR ); - : uint8 if ( fType == FilterType::FILTER_SUBSTR ); - : uint8; assertionValue_len: uint8; assertionValue: bytes &size=self.assertionValue_len; @@ -662,6 +664,33 @@ type DecodedAttributeValue = unit(fType: FilterType) { } }; +type SubstringFilter = unit { + var initial: string; + var final: string; + var anys: vector; + + : uint8; # filter tag + attributeDesc_len: uint8; + attributeDesc: bytes &size=self.attributeDesc_len; + + # Crunch through the sequence/choice of substrings. + # + # https://datatracker.ietf.org/doc/html/rfc4511#section-4.5.1 + header: ASN1::ASN1Header; + : ASN1::ASN1Message(False)[] &size=self.header.len.len foreach { + local data = $$.application_data.decode(); + if ( $$.application_id == 0 ) { + self.initial = data; + } else if ( $$.application_id == 1 ) { + self.anys.push_back(data); + } else if ( $$.application_id == 2 ) { + self.final = data; + } else { + throw "invalid substring choice %s" % $$.application_id; + } + } +}; + type SearchFilter = unit { var filterType: FilterType = FilterType::Undef; var filterBytes: bytes = b""; @@ -693,7 +722,7 @@ type SearchFilter = unit { FilterType::FILTER_EQ -> FILTER_EQ: DecodedAttributeValue(FilterType::FILTER_EQ) &parse-from=self.filterBytes; - FilterType::FILTER_SUBSTR -> FILTER_SUBSTR: DecodedAttributeValue(FilterType::FILTER_SUBSTR) + FilterType::FILTER_SUBSTR -> FILTER_SUBSTR: SubstringFilter &parse-from=self.filterBytes; FilterType::FILTER_GE -> FILTER_GE: DecodedAttributeValue(FilterType::FILTER_GE) &parse-from=self.filterBytes; diff --git a/tests/analyzer/ldap_substring_search.zeek b/tests/analyzer/ldap_substring_search.zeek new file mode 100644 index 0000000..a61fdfa --- /dev/null +++ b/tests/analyzer/ldap_substring_search.zeek @@ -0,0 +1,12 @@ +# Copyright (c) 2024 by the Zeek Project. See LICENSE for details. + +# @TEST-EXEC: zeek -C -r ${TRACES}/ldap_star_single.pcap %INPUT >output 2>&1 +# @TEST-EXEC: btest-diff output +# @TEST-EXEC: cat conn.log | zeek-cut -m ts uid history service > conn.log2 && mv conn.log2 conn.log +# @TEST-EXEC: btest-diff conn.log +# @TEST-EXEC: btest-diff ldap_search.log +# +# @TEST-DOC: Test substring filter parsed and rendered properly when initial and final are present, but no anys. + +@load base/protocols/conn +@load analyzer diff --git a/tests/baseline/analyzer.ldap_substring_search/conn.log b/tests/baseline/analyzer.ldap_substring_search/conn.log new file mode 100644 index 0000000..3041150 --- /dev/null +++ b/tests/baseline/analyzer.ldap_substring_search/conn.log @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +### NOTE: This file has been sorted with diff-sort. +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 D spicy_ldap_tcp +ts uid history service diff --git a/tests/baseline/analyzer.ldap_substring_search/ldap_search.log b/tests/baseline/analyzer.ldap_substring_search/ldap_search.log new file mode 100644 index 0000000..3ada189 --- /dev/null +++ b/tests/baseline/analyzer.ldap_substring_search/ldap_search.log @@ -0,0 +1,12 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +### NOTE: This file has been sorted with diff-sort. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path ldap_search +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto message_id scope deref base_object result_count result diagnostic_message filter attributes +#types time string addr port addr port string int set[string] set[string] vector[string] count set[string] vector[string] string vector[string] +#close XXXX-XX-XX-XX-XX-XX +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.168.10.152 34581 192.168.10.186 389 tcp 6 tree always DC=matrix\x2cDC=local 0 - - (gPCUserExtensionNames=[*]) - diff --git a/tests/baseline/analyzer.ldap_substring_search/output b/tests/baseline/analyzer.ldap_substring_search/output new file mode 100644 index 0000000..b1bb951 --- /dev/null +++ b/tests/baseline/analyzer.ldap_substring_search/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +### NOTE: This file has been sorted with diff-sort. diff --git a/tests/traces/ldap_star_single.pcap b/tests/traces/ldap_star_single.pcap new file mode 100644 index 0000000000000000000000000000000000000000..e7821a89e3b9beb28ef3104c0534cedd57553619 GIT binary patch literal 636 zcmaJ<&1(}u6o0dsY?Gx`6yq0#4xVLT(;C2`c^pg8l<~6GZST`Zn34_?VgBZ|1#uAM-|8SWw;r!1{&d(**HP zCK8;7OT*ahtJgGU`k|{{U)Pw|EDPRf)H=-UZn74u=muZw+-2$=FWzMv8*M|cXpn;v zT=9m#40-}8r8 zt$`bH`5w7>pWNc7C^<`y4dckQlhBLDjvLvb7sP(}qRp4+aiLfAhQW0qsQOlpD!l_7Qyrr{0(8mg3#ad9Bj(= im^rgXN>G^Ee)_UC$z{BV_b768<_bGf$k+_h7=Hm|nvxIz literal 0 HcmV?d00001