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 *<string>*.

Reported and PCAP provided by @martinvanhensbergen, thanks!

Closes zeek/spicy-ldap#27

src/analyzer/protocol/ldap/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(),
+        local anys: string = "";
-                               search_filter.FILTER_SUBSTR.assertionValueDecoded);
+        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<string>;
+
+  : 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;

testing/btest/Baseline/scripts.base.protocols.ldap.ldap_substring_search/conn.log

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	history	service
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	D	ldap_tcp

testing/btest/Baseline/scripts.base.protocols.ldap.ldap_substring_search/ldap_search.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#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	message_id	scope	deref_aliases	base_object	result_count	result	diagnostic_message	filter	attributes
+#types	time	string	addr	port	addr	port	int	string	string	string	count	string	string	string	vector[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.10.152	34581	192.168.10.186	389	6	tree	always	DC=matrix,DC=local	0	-	-	(gPCUserExtensionNames=[*])	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.ldap_substring_search/output

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/scripts/base/protocols/ldap/ldap_substring_search.zeek

@@ -0,0 +1,13 @@
+# Copyright (c) 2024 by the Zeek Project. See LICENSE for details.
+
+# @TEST-REQUIRES: have-spicy
+# @TEST-EXEC: zeek -b -C -r ${TRACES}/ldap/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 base/protocols/ldap