ldap: Fix assuming GSS-SPNEGO for all bindResponses

In retrospect that's an obvious bug.

src/analyzer/protocol/ldap/ldap.spicy

@@ -137,6 +137,7 @@ type MessageMode = enum {
 
 type Ctx = struct {
   messageMode: MessageMode;   # Message dispatching mode
+  saslMechanism: string;      # The SASL mechanism selected by the client.
   startTlsRequested: bool;    # Did the client use the StartTLS extended request?
 };
 
@@ -255,7 +256,7 @@ public type Message = unit(ctx: Ctx&) {
   }
 
   switch ( self.opcode ) {
-    ProtocolOpcode::BIND_REQUEST            -> BIND_REQUEST:            BindRequest(self);
+    ProtocolOpcode::BIND_REQUEST            -> BIND_REQUEST:            BindRequest(self, ctx);
     ProtocolOpcode::BIND_RESPONSE           -> BIND_RESPONSE:           BindResponse(self, ctx);
     ProtocolOpcode::UNBIND_REQUEST          -> UNBIND_REQUEST:          UnbindRequest(self);
     ProtocolOpcode::SEARCH_REQUEST          -> SEARCH_REQUEST:          SearchRequest(self);
@@ -307,7 +308,7 @@ type GSS_SPNEGO_negTokenInit = unit {
 };
 
 # Peak into GSS-SPNEGO payload and ensure it is indeed GSS-SPNEGO.
-type GSS_SPNEGO = unit {
+type GSS_SPNEGO_Init = unit {
   # This is the optional octet string in SaslCredentials.
   credentialsHeader: ASN1::ASN1Header &requires=($$.tag.type_ == ASN1::ASN1Type::OctetString);
 
@@ -333,12 +334,19 @@ type SaslCredentials = unit() {
 
   # Peak into GSS-SPNEGO payload if we have any.
   switch ( self.mechanism ) {
-    "GSS-SPNEGO" -> gss_spnego: GSS_SPNEGO;
+    "GSS-SPNEGO" -> gss_spnego: GSS_SPNEGO_Init;
     * -> : skip bytes &eod;
   };
 };
 
-type NegTokenResp = unit {
+type GSS_SPNEGO_Subsequent = unit {
+  token: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::ContextSpecific);
+  switch ( self.token.tag.type_ ) {
+    ASN1::ASN1Type(1) -> negTokenResp: GSS_SPNEGO_negTokenResp;
+  };
+};
+
+type GSS_SPNEGO_negTokenResp = unit {
   var accepted: bool;
   var supportedMech: ASN1::ASN1Message;
 
@@ -366,34 +374,13 @@ type NegTokenResp = unit {
   } &parse-from=self.supportedMech.application_data;
 };
 
-type ServerSaslCreds = unit {
-  serverSaslCreds: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::ContextSpecific && $$.tag.type_ == ASN1::ASN1Type(7));
-
-  # The PCAP missing_ldap_logs.pcapng has a1 81 b6 here for the GSS-SPNEGO response.
-  #
-  # This is context-specific ID 1, constructed, and a length of 182 as
-  # specified by in 4.2 of RFC4178.
-  #
-  # https://www.rfc-editor.org/rfc/rfc4178#section-4.2
-  #
-  # TODO: This is only valid for a GSS-SPNEGO negTokenResp.
-  # If you want to support something else, remove the requires
-  # and add more to the switch below.
-  choice: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::ContextSpecific);
-
-  switch ( self.choice.tag.type_ ) {
-    ASN1::ASN1Type(1) -> negTokenResp: NegTokenResp;
-    # ...
-  } &size=self.choice.len.len;
-};
-
 # TODO(fox-ds): A helper unit for requests for which no handling has been implemented.
 # Eventually all uses of this unit should be replaced with actual parsers so this unit can be removed.
 type NotImplemented = unit(inout message: Message) {
   : skip bytes &eod;
 };
 
-type BindRequest = unit(inout message: Message) {
+type BindRequest = unit(inout message: Message, ctx: Ctx&) {
   version: ASN1::ASN1Message(True) &convert=$$.body.num_value;
   name: ASN1::ASN1Message(True) &convert=$$.body.str_value {
     message.obj = self.name;
@@ -417,9 +404,15 @@ type BindRequest = unit(inout message: Message) {
   saslCreds: SaslCredentials() &parse-from=self.authData if ((self.authType == BindAuthType::BIND_AUTH_SASL) &&
                                                              (|self.authData| > 0)) {
     message.arg = self.saslCreds.mechanism;
+    ctx.saslMechanism = self.saslCreds.mechanism;
   }
 } &requires=(self?.authType && (self.authType != BindAuthType::Undef));
 
+type ServerSaslCreds = unit {
+  serverSaslCreds: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::ContextSpecific && $$.tag.type_ == ASN1::ASN1Type(7));
+  payload: bytes &size=self.serverSaslCreds.len.len;
+};
+
 type BindResponse = unit(inout message: Message, ctx: Ctx&) {
   : Result {
     message.result_ = $$;
@@ -432,10 +425,15 @@ type BindResponse = unit(inout message: Message, ctx: Ctx&) {
   # if the serverSaslCreds field exists or not. But, not sure we can
   # check if there's any bytes left at this point outside of passing
   # in the length and playing with offset().
-  serverSaslCreds: ServerSaslCreds[] &eod {
+  serverSaslCreds: ServerSaslCreds[] &eod;
-    if ( |self.serverSaslCreds| > 0 ) {
+
-      if ( self.serverSaslCreds[0]?.negTokenResp ) {
+  # If the client requested GSS-SPNEGO, try to parse the server's response
-        local token = self.serverSaslCreds[0].negTokenResp;
+  # to switch message mode.
+  gss_spnego: GSS_SPNEGO_Subsequent &parse-from=self.serverSaslCreds[0].payload
+      if (ctx.saslMechanism == "GSS-SPNEGO" && |self.serverSaslCreds| > 0) {
+
+    if ( $$?.negTokenResp ) {
+      local token = $$.negTokenResp;
       if ( token.accepted && token?.supportedMechOid ) {
         if ( token.supportedMechOid == GSSAPI_MECH_MS_KRB5 ) {
           ctx.messageMode = MessageMode::MS_KRB5;
@@ -443,7 +441,6 @@ type BindResponse = unit(inout message: Message, ctx: Ctx&) {
       }
     }
   }
-  }
 };
 
 #-----------------------------------------------------------------------------

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-ntlm/conn.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	conn
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	count	string	count	count	count	count	set[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	60126	127.0.1.1	389	tcp	ldap_tcp	2.290081	289	1509	SF	0	ShADadFf	12	921	15	2297	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-ntlm/ldap.log

@@ -0,0 +1,13 @@
+### 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
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	message_id	version	opcode	result	diagnostic_message	object	argument
+#types	time	string	addr	port	addr	port	int	int	string	string	string	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	60126	127.0.1.1	389	1	3	bind SASL	SASL bind in progress	SASL(0): successful result: 	-	NTLM
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	60126	127.0.1.1	389	2	3	bind SASL	success	-	-	NTLM
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	60126	127.0.1.1	389	4	-	unbind	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-ntlm/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	127.0.0.1	60126	127.0.1.1	389	3	tree	never	dc=example,dc=com	9	success	-	(objectclass=*)	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-scram-sha-512/conn.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	conn
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	count	string	count	count	count	count	set[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59552	127.0.1.1	389	tcp	ldap_tcp	2.231680	353	1772	SF	0	ShADadFf	11	933	15	2560	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-scram-sha-512/ldap.log

@@ -0,0 +1,13 @@
+### 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
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	message_id	version	opcode	result	diagnostic_message	object	argument
+#types	time	string	addr	port	addr	port	int	int	string	string	string	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59552	127.0.1.1	389	1	3	bind SASL	SASL bind in progress	SASL(0): successful result: user: sasladmin@slapd.ldap property: slapAuthzDN not found in sasldb	-	SCRAM-SHA-512
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59552	127.0.1.1	389	2	3	bind SASL	success	-	-	SCRAM-SHA-512
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59552	127.0.1.1	389	4	-	unbind	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ldap.sasl-scram-sha-512/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	127.0.0.1	59552	127.0.1.1	389	3	tree	never	dc=example,dc=com	9	success	-	(objectclass=*)	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ldap/sasl-ntlm.zeek

@@ -0,0 +1,12 @@
+# Copyright (c) 2024 by the Zeek Project. See LICENSE for details.
+
+# @TEST-REQUIRES: have-spicy
+# @TEST-EXEC: zeek -C -r ${TRACES}/ldap/sasl-ntlm.pcap %INPUT
+# @TEST-EXEC: cat conn.log | zeek-cut -Cn local_orig local_resp > conn.log2 && mv conn.log2 conn.log
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff ldap.log
+# @TEST-EXEC: btest-diff ldap_search.log
+# @TEST-EXEC: ! test -f dpd.log
+# @TEST-EXEC: ! test -f analyzer.log
+#
+# @TEST-DOC: This broke after #3826 got merged

testing/btest/scripts/base/protocols/ldap/sasl-scram-sha-512.zeek

@@ -0,0 +1,12 @@
+# Copyright (c) 2024 by the Zeek Project. See LICENSE for details.
+
+# @TEST-REQUIRES: have-spicy
+# @TEST-EXEC: zeek -C -r ${TRACES}/ldap/sasl-scram-sha-512.pcap %INPUT
+# @TEST-EXEC: cat conn.log | zeek-cut -Cn local_orig local_resp > conn.log2 && mv conn.log2 conn.log
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff ldap.log
+# @TEST-EXEC: btest-diff ldap_search.log
+# @TEST-EXEC: ! test -f dpd.log
+# @TEST-EXEC: ! test -f analyzer.log
+#
+# @TEST-DOC: This broke after #3826 got merged