From fd9b7722e05ba8e16a6a679a54190a98597d84e1 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Thu, 28 Sep 2023 11:16:02 +0200 Subject: [PATCH] Fix string representation of extended search filters We want to render search filters similar to e.g., Wireshark. In order to calculate a string representation of extended search filter we need to output their values in a order which is different from other fields. This patch fixes this directly, but the bigger issue is probably that `DecodedAttributeValue` is a too generic unit and we should have lower-level types for constructs like extended search filters which know how to render themselves correctly. Closes #23. --- analyzer/ldap.spicy | 8 +++++--- tests/analyzer/functions.spicy | 2 +- tests/analyzer/search_filter_extended.zeek | 11 +++++++++++ .../ldap_search.log | 12 ++++++++++++ tests/traces/README | 2 ++ tests/traces/ldap-issue-32.pcapng | Bin 0 -> 1436 bytes 6 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 tests/analyzer/search_filter_extended.zeek create mode 100644 tests/baseline/analyzer.search_filter_extended/ldap_search.log create mode 100644 tests/traces/ldap-issue-32.pcapng diff --git a/analyzer/ldap.spicy b/analyzer/ldap.spicy index 63c63bd..a7be8af 100644 --- a/analyzer/ldap.spicy +++ b/analyzer/ldap.spicy @@ -578,9 +578,11 @@ public function string_representation(search_filter: SearchFilter): string { # The following FilterTypes are leaf nodes and can thus be represented in a statement case FilterType::FILTER_EXT: { - repr = "(%s:%s:=%s)" % (search_filter.FILTER_EXT.attributeDesc.decode(), - search_filter.FILTER_EXT.assertionValueDecoded, - search_filter.FILTER_EXT.matchValue.decode()); + # For extended search filters the meaning of the individual fields in + # `DecodedAttributeValue` is slightly different. + repr = "(%s:%s:=%s)" % (search_filter.FILTER_EXT.assertionValueDecoded, + search_filter.FILTER_EXT.attributeDesc.decode(), + search_filter.FILTER_EXT.matchValue); } case FilterType::FILTER_APPROX: { repr = "(%s~=%s)" % (search_filter.FILTER_APPROX.attributeDesc.decode(), diff --git a/tests/analyzer/functions.spicy b/tests/analyzer/functions.spicy index 35ff80c..5679dba 100644 --- a/tests/analyzer/functions.spicy +++ b/tests/analyzer/functions.spicy @@ -126,6 +126,6 @@ function test_string_representation() { local repr3 = make_nested_repr(vector("foo", "bar", "baz")); assert repr3 == "(|(|(foo=*)(bar=*))(baz=*))": repr3; -# "(|(|(foo=*)(bar=*))(baz=*))" } + test_string_representation(); diff --git a/tests/analyzer/search_filter_extended.zeek b/tests/analyzer/search_filter_extended.zeek new file mode 100644 index 0000000..2e91b2e --- /dev/null +++ b/tests/analyzer/search_filter_extended.zeek @@ -0,0 +1,11 @@ +# Copyright (c) 2021 by the Zeek Project. See LICENSE for details. + +# @TEST-DOC: This test case is a regression test for #23. +# +# @TEST-EXEC: zeek -C -r ${TRACES}/ldap-issue-32.pcapng %INPUT +# @TEST-EXEC: cat ldap_search.log | zeek-cut -C uid filter base_object > ldap_search.log2 && mv ldap_search.log2 ldap_search.log +# @TEST-EXEC: btest-diff ldap_search.log +# +# @TEST-DOC: Test LDAP analyzer with small trace. + +@load analyzer diff --git a/tests/baseline/analyzer.search_filter_extended/ldap_search.log b/tests/baseline/analyzer.search_filter_extended/ldap_search.log new file mode 100644 index 0000000..555a1c7 --- /dev/null +++ b/tests/baseline/analyzer.search_filter_extended/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 uid filter base_object +#types string string vector[string] +#close XXXX-XX-XX-XX-XX-XX +CHhAvVGS1DHFjwGM9 (departmentNumber:2.16.840.1.113730.3.3.2.46.1:=>=N4709) DC=matrix\x2cDC=local diff --git a/tests/traces/README b/tests/traces/README index c85b3c9..4858d99 100644 --- a/tests/traces/README +++ b/tests/traces/README @@ -11,3 +11,5 @@ We collect them here for convenience only. - the LDAP flow selected (filtered out the Kerberos packets) - truncated to 10 packets (where packet 10 contains the SASL encrypted LDAP message) - one `\x30` byte in the cyphertext changed to `\x00` +- ldap-issue-32.pcapng: Provided by GH user martinvanhensbergen, + diff --git a/tests/traces/ldap-issue-32.pcapng b/tests/traces/ldap-issue-32.pcapng new file mode 100644 index 0000000000000000000000000000000000000000..8ed316dd51f5a6cc15291ecdf73b87d2e7062fed GIT binary patch literal 1436 zcmai!&rcIU6vt=Fwm^kg5&0zv6BEsr((Nq$ZG@Dz$d3qVK%?=*(yp+f-7ULYpz1*c z5);D3faOR$;SH}QCMKvPCLWFc111O_Jb2eP+nUgZ;@f?@Z+2!r^Y%M0V{dGmVGg;FCcCg;Q%DTPJ7)bA#)lr${}Il&1mYBYD6cN}~koUUMj3`8Z5>Y=9|zBHVX z51m2h+Q2adfxT3pwSN2BQ82;u8w-&QH8}To^9a?!AD~}dM-ddUtvq_SYu^Tcv$@5r zDaW3}^=Qj#qV4=>VGSX&{K3N5l6kf%9_!19h9)odK$KRKV%pAF>`aY?L2xq+dKF&o z?B#_3AMz1i;03`O^b+0+7f2l7g{O{`lod52BPqssE;B7@E4BT7;w(#5t2z%a3b*Y5`wC>xj)_VSclw zE>xfG6Z-scafL-_QwJ;RvvIIo)Q6b~wMWnsTlW{L4b_NQ+uWksD92}8+8FcKYqn7!aQf!5JEX3DJEGxt(jjr(*ROeGp literal 0 HcmV?d00001