From 1ebfdb9261eb3d202b176a2cf2372c264bcfe853 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Fri, 22 Sep 2023 12:39:51 +0200 Subject: [PATCH 1/3] Move computation of filter string representation into testable function --- analyzer/ldap.spicy | 185 +++++++++++++++++++++++--------------------- 1 file changed, 97 insertions(+), 88 deletions(-) diff --git a/analyzer/ldap.spicy b/analyzer/ldap.spicy index 5681aa8..10205e6 100644 --- a/analyzer/ldap.spicy +++ b/analyzer/ldap.spicy @@ -509,6 +509,102 @@ public function uint32_to_hex_repr(bts: bytes) : string { return ret; } +# Helper to compute a string representation of a `SearchFilter`. +public function string_representation(search_filter: SearchFilter): string { + local repr: string; + + switch ( local fType = search_filter.filterType ) { + # The NOT, AND and OR filter types are trees and may hold many leaf nodes. So recursively get + # the stringPresentations for the leaf nodes and add them all in one final statement. + + case FilterType::FILTER_NOT: { + repr = "(!%s)" % search_filter.FILTER_NOT.searchfilter.stringRepresentation; + } + + case FilterType::FILTER_AND, FilterType::FILTER_OR: { + local nestedObj: ParseNestedAndOr; + local printChar = ""; + local printCount = 0; + + if ( fType == FilterType::FILTER_AND ) { + printChar = "&"; + nestedObj = search_filter.FILTER_AND; + } else { + printChar = "|"; + nestedObj = search_filter.FILTER_OR; + } + + # Build the nested AND/OR statement in this loop. When we encounter the first element, + # we open the statement. At the second element, we close our first complete statement. For every + # following statement, we extend the AND/OR statement by wrapping it around the already completed + # statement. Although it is also valid to not do this wrapping, which is logically equivalent, e.g: + # + # (1) (2) + # (?(a=b)(c=d)(e=f)) vs (?(?(a=b)(c=d))(e=f)) + # + # the latter version is also shown by Wireshark. So although the parsed structure actually represents + # version (1) of the query, we now choose to print version (2). If this is not desirable, swap the code + # for the following: + # + # # Construct the nested structure, like (1) + # for ( SF in nestedObj.searchfilters ) { + # self.stringRepresentation = self.stringRepresentation + SF.stringRepresentation + # } + # # Close it with brackets and put the correct printChar for AND/OR in the statement + # self.stringRepresentation = "(%s" % printChar + self.stringRepresentation + ")"; + # + + for ( searchFilter in nestedObj.searchfilters ) { + switch ( printCount ) { + case 0: { + repr = "(%s%s" % (printChar, searchFilter.stringRepresentation); + } + case 1: { + repr = repr + searchFilter.stringRepresentation + ")"; + } + default: { + repr = "(%s" % printChar + repr + searchFilter.stringRepresentation + ")"; + } + } + printCount += 1; + } + } + + # 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()); + } + case FilterType::FILTER_APPROX: { + repr = "(%s~=%s)" % (search_filter.FILTER_APPROX.attributeDesc.decode(), + search_filter.FILTER_APPROX.assertionValueDecoded); + } + case FilterType::FILTER_EQ: { + repr = "(%s=%s)" % (search_filter.FILTER_EQ.attributeDesc.decode(), + search_filter.FILTER_EQ.assertionValueDecoded); + } + case FilterType::FILTER_GE: { + repr = "(%s>=%s)" % (search_filter.FILTER_GE.attributeDesc.decode(), + search_filter.FILTER_GE.assertionValueDecoded); + } + case FilterType::FILTER_LE: { + repr = "(%s<=%s)" % (search_filter.FILTER_LE.attributeDesc.decode(), + search_filter.FILTER_LE.assertionValueDecoded); + } + case FilterType::FILTER_SUBSTR: { + repr = "(%s=*%s*)" % (search_filter.FILTER_SUBSTR.attributeDesc.decode(), + search_filter.FILTER_SUBSTR.assertionValueDecoded); + } + case FilterType::FILTER_PRESENT: { + repr = "(%s=*)" % search_filter.FILTER_PRESENT; + } + } + + return repr; +} + # Represents an (extended) key-value pair present in SearchFilters type DecodedAttributeValue = unit(fType: FilterType) { var assertionValueDecoded: string = ""; @@ -609,94 +705,7 @@ type SearchFilter = unit { # recursively get the stringRepresentations for those leafs, which are SearchFilters on %done { - switch ( local fType = self.filterType ) { - # The NOT, AND and OR filter types are trees and may hold many leaf nodes. So recursively get - # the stringPresentations for the leaf nodes and add them all in one final statement. - - case FilterType::FILTER_NOT: { - self.stringRepresentation = "(!%s)" % self.FILTER_NOT.searchfilter.stringRepresentation; - } - - case FilterType::FILTER_AND, FilterType::FILTER_OR: { - local nestedObj: ParseNestedAndOr; - local printChar = ""; - local printCount = 0; - - if ( fType == FilterType::FILTER_AND ) { - printChar = "&"; - nestedObj = self.FILTER_AND; - } else { - printChar = "|"; - nestedObj = self.FILTER_OR; - } - - # Build the nested AND/OR statement in this loop. When we encounter the first element, - # we open the statement. At the second element, we close our first complete statement. For every - # following statement, we extend the AND/OR statement by wrapping it around the already completed - # statement. Although it is also valid to not do this wrapping, which is logically equivalent, e.g: - # - # (1) (2) - # (?(a=b)(c=d)(e=f)) vs (?(?(a=b)(c=d))(e=f)) - # - # the latter version is also shown by Wireshark. So although the parsed structure actually represents - # version (1) of the query, we now choose to print version (2). If this is not desirable, swap the code - # for the following: - # - # # Construct the nested structure, like (1) - # for ( SF in nestedObj.searchfilters ) { - # self.stringRepresentation = self.stringRepresentation + SF.stringRepresentation - # } - # # Close it with brackets and put the correct printChar for AND/OR in the statement - # self.stringRepresentation = "(%s" % printChar + self.stringRepresentation + ")"; - # - - for ( searchFilter in nestedObj.searchfilters ) { - switch ( printCount ) { - case 0: { - self.stringRepresentation = "(%s%s" % (printChar, searchFilter.stringRepresentation); - } - case 1: { - self.stringRepresentation = self.stringRepresentation + searchFilter.stringRepresentation + ")"; - } - default: { - self.stringRepresentation = "(%s" % printChar + self.stringRepresentation + searchFilter.stringRepresentation + ")"; - } - } - printCount += 1; - } - } - - # The following FilterTypes are leaf nodes and can thus be represented in a statement - - case FilterType::FILTER_EXT: { - self.stringRepresentation = "(%s:%s:=%s)" % (self.FILTER_EXT.attributeDesc.decode(), - self.FILTER_EXT.assertionValueDecoded, - self.FILTER_EXT.matchValue.decode()); - } - case FilterType::FILTER_APPROX: { - self.stringRepresentation = "(%s~=%s)" % (self.FILTER_APPROX.attributeDesc.decode(), - self.FILTER_APPROX.assertionValueDecoded); - } - case FilterType::FILTER_EQ: { - self.stringRepresentation = "(%s=%s)" % (self.FILTER_EQ.attributeDesc.decode(), - self.FILTER_EQ.assertionValueDecoded); - } - case FilterType::FILTER_GE: { - self.stringRepresentation = "(%s>=%s)" % (self.FILTER_GE.attributeDesc.decode(), - self.FILTER_GE.assertionValueDecoded); - } - case FilterType::FILTER_LE: { - self.stringRepresentation = "(%s<=%s)" % (self.FILTER_LE.attributeDesc.decode(), - self.FILTER_LE.assertionValueDecoded); - } - case FilterType::FILTER_SUBSTR: { - self.stringRepresentation = "(%s=*%s*)" % (self.FILTER_SUBSTR.attributeDesc.decode(), - self.FILTER_SUBSTR.assertionValueDecoded); - } - case FilterType::FILTER_PRESENT: { - self.stringRepresentation = "(%s=*)" % self.FILTER_PRESENT; - } - } + self.stringRepresentation = string_representation(self); } on %error { From 832fab529d490de38503bbbcd9bcbb98d4c4eed5 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Fri, 22 Sep 2023 13:03:06 +0200 Subject: [PATCH 2/3] Add failing test case for #21 --- tests/analyzer/functions.spicy | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/analyzer/functions.spicy b/tests/analyzer/functions.spicy index b11de43..bd41892 100644 --- a/tests/analyzer/functions.spicy +++ b/tests/analyzer/functions.spicy @@ -91,3 +91,24 @@ assert LDAP::uint32_to_hex_repr(b"\x00\x00\x00\x00") == "0x00000000"; # 4 times \xff assert LDAP::uint32_to_hex_repr(b"\xff\xff\xff\xff") == "0xffffffff"; + +# ---------------------------------------------------------------------------------- +# function string_representation() +function test_string_representation() { + local or_: LDAP::SearchFilter; + or_.filterType = LDAP::FilterType::FILTER_PRESENT; + or_.FILTER_PRESENT = "foo"; + or_.stringRepresentation = LDAP::string_representation(or_); + + local nestedOr: LDAP::ParseNestedAndOr; + nestedOr.searchfilters = vector(); + nestedOr.searchfilters.push_back(or_); + + local searchFilter: LDAP::SearchFilter; + searchFilter.filterType = LDAP::FilterType::FILTER_OR; + searchFilter.FILTER_OR = nestedOr; + + local repr = LDAP::string_representation(searchFilter); + assert repr == "(|(foo=*))": repr; +} +test_string_representation(); From 104f41e6eaf17533eb1eddb29177db967bbea81f Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Fri, 22 Sep 2023 13:47:29 +0200 Subject: [PATCH 3/3] Fix formatting of search filters which contain a single elements Closes #21. --- analyzer/ldap.spicy | 13 +++++++++---- tests/analyzer/functions.spicy | 35 +++++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/analyzer/ldap.spicy b/analyzer/ldap.spicy index 10205e6..63c63bd 100644 --- a/analyzer/ldap.spicy +++ b/analyzer/ldap.spicy @@ -524,7 +524,6 @@ public function string_representation(search_filter: SearchFilter): string { case FilterType::FILTER_AND, FilterType::FILTER_OR: { local nestedObj: ParseNestedAndOr; local printChar = ""; - local printCount = 0; if ( fType == FilterType::FILTER_AND ) { printChar = "&"; @@ -554,10 +553,16 @@ public function string_representation(search_filter: SearchFilter): string { # self.stringRepresentation = "(%s" % printChar + self.stringRepresentation + ")"; # + local i = 0; for ( searchFilter in nestedObj.searchfilters ) { - switch ( printCount ) { + switch ( i ) { case 0: { - repr = "(%s%s" % (printChar, searchFilter.stringRepresentation); + repr = "(%s%s%s" % ( + printChar, + searchFilter.stringRepresentation, + # If we have exactly one element immediately close the statement since we are done. + |nestedObj.searchfilters| == 1 ? ")" : "" + ); } case 1: { repr = repr + searchFilter.stringRepresentation + ")"; @@ -566,7 +571,7 @@ public function string_representation(search_filter: SearchFilter): string { repr = "(%s" % printChar + repr + searchFilter.stringRepresentation + ")"; } } - printCount += 1; + i += 1; } } diff --git a/tests/analyzer/functions.spicy b/tests/analyzer/functions.spicy index bd41892..35ff80c 100644 --- a/tests/analyzer/functions.spicy +++ b/tests/analyzer/functions.spicy @@ -94,21 +94,38 @@ assert LDAP::uint32_to_hex_repr(b"\xff\xff\xff\xff") == "0xffffffff"; # ---------------------------------------------------------------------------------- # function string_representation() -function test_string_representation() { - local or_: LDAP::SearchFilter; - or_.filterType = LDAP::FilterType::FILTER_PRESENT; - or_.FILTER_PRESENT = "foo"; - or_.stringRepresentation = LDAP::string_representation(or_); - +function make_nested_repr(filters: vector): string { local nestedOr: LDAP::ParseNestedAndOr; nestedOr.searchfilters = vector(); - nestedOr.searchfilters.push_back(or_); + + for (f in filters) { + local or_: LDAP::SearchFilter; + or_.filterType = LDAP::FilterType::FILTER_PRESENT; + or_.FILTER_PRESENT = f; + or_.stringRepresentation = LDAP::string_representation(or_); + + nestedOr.searchfilters.push_back(or_); + } local searchFilter: LDAP::SearchFilter; searchFilter.filterType = LDAP::FilterType::FILTER_OR; searchFilter.FILTER_OR = nestedOr; - local repr = LDAP::string_representation(searchFilter); - assert repr == "(|(foo=*))": repr; + return LDAP::string_representation(searchFilter); +} + +function test_string_representation() { + local repr0 = make_nested_repr(vector()); + assert repr0 == "": repr0; + + local repr1 = make_nested_repr(vector("foo")); + assert repr1 == "(|(foo=*))": repr1; + + local repr2 = make_nested_repr(vector("foo", "bar")); + assert repr2 == "(|(foo=*)(bar=*))": repr2; + + local repr3 = make_nested_repr(vector("foo", "bar", "baz")); + assert repr3 == "(|(|(foo=*)(bar=*))(baz=*))": repr3; +# "(|(|(foo=*)(bar=*))(baz=*))" } test_string_representation();