Skip to content

Commit

Permalink
Fix string representation of extended search filters
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bbannier committed Sep 28, 2023
1 parent 4395c64 commit fd9b772
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 4 deletions.
8 changes: 5 additions & 3 deletions analyzer/ldap.spicy
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion tests/analyzer/functions.spicy
Original file line number Diff line number Diff line change
Expand Up @@ -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();
11 changes: 11 additions & 0 deletions tests/analyzer/search_filter_extended.zeek
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions tests/baseline/analyzer.search_filter_extended/ldap_search.log
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tests/traces/README
Original file line number Diff line number Diff line change
Expand Up @@ -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,
<https://github.com/zeek/spicy-ldap/issues/23>
Binary file added tests/traces/ldap-issue-32.pcapng
Binary file not shown.

0 comments on commit fd9b772

Please sign in to comment.