From b8e573a3b9e4e0d777c65cb4ee36a3c20b94588b Mon Sep 17 00:00:00 2001
From: Arne Welzel <arne.welzel@corelight.com>
Date: Tue, 15 Apr 2025 20:07:23 +0200
Subject: [PATCH] ldap: Clean up from code review

Co-authored-by: Benjamin Bannier <benjamin.bannier@corelight.com>
---
 scripts/base/protocols/ldap/consts.zeek |  2 ++
 scripts/base/protocols/ldap/main.zeek   | 20 +++++++----
 src/analyzer/protocol/ldap/ldap.spicy   | 45 ++++++++++++++-----------
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/scripts/base/protocols/ldap/consts.zeek b/scripts/base/protocols/ldap/consts.zeek
index 5b29fd22e4..9eeb9f89af 100644
--- a/scripts/base/protocols/ldap/consts.zeek
+++ b/scripts/base/protocols/ldap/consts.zeek
@@ -26,6 +26,8 @@ export {
 
 	const BIND_SIMPLE = "bind simple";
 	const BIND_SASL = "bind SASL";
+	const BIND_SICILY_NEGOTIATE = "sicily_negotiate";
+	const BIND_SICILY_RESPONSE= "sicily_response";
 
 	const RESULT_CODES = { [ LDAP::ResultCode_SUCCESS ] = "success", [
 	    LDAP::ResultCode_OPERATIONS_ERROR ] = "operations error", [
diff --git a/scripts/base/protocols/ldap/main.zeek b/scripts/base/protocols/ldap/main.zeek
index 9033333c04..c0a2f90bb3 100644
--- a/scripts/base/protocols/ldap/main.zeek
+++ b/scripts/base/protocols/ldap/main.zeek
@@ -376,17 +376,23 @@ event LDAP::bind_request(c: connection,
   if ( m?$opcode )
     Reporter::conn_weird("LDAP_bind_opcode_already_set", c, m$opcode, "LDAP");
 
-  if (authType == LDAP::BindAuthType_BIND_AUTH_SIMPLE) {
+  switch ( authType ) {
+  case LDAP::BindAuthType_BIND_AUTH_SIMPLE:
     m$opcode = BIND_SIMPLE;
-  } else if (authType == LDAP::BindAuthType_BIND_AUTH_SASL) {
+    break;
+  case LDAP::BindAuthType_BIND_AUTH_SASL:
     m$opcode = BIND_SASL;
-  } else if (authType == LDAP::BindAuthType_SICILY_NEGOTIATE ) {
-    m$opcode = "sicily_negotiate";
-  } else if (authType == LDAP::BindAuthType_SICILY_RESPONSE ) {
-    m$opcode = "sicily_response";
-  } else {
+    break;
+  case LDAP::BindAuthType_SICILY_NEGOTIATE:
+    m$opcode = BIND_SICILY_NEGOTIATE;
+    break;
+  case LDAP::BindAuthType_SICILY_RESPONSE:
+    m$opcode = BIND_SICILY_RESPONSE;
+    break;
+  default:
     Reporter::conn_weird("LDAP_unknown_auth_type", c, cat(authType), "LDAP");
     m$opcode = cat(authType);
+    break;
   }
 }
 
diff --git a/src/analyzer/protocol/ldap/ldap.spicy b/src/analyzer/protocol/ldap/ldap.spicy
index 0fef1e2b8a..4848109616 100644
--- a/src/analyzer/protocol/ldap/ldap.spicy
+++ b/src/analyzer/protocol/ldap/ldap.spicy
@@ -416,10 +416,11 @@ type SaslCredentials = unit() {
   };
 };
 
-type SicilyMessage = unit(bat: BindAuthType) {
-    # Just ensure the signature matches. We could do more,
-    # but it'd be better to forward to an NTLM analyzer.
-    signature: b"NTLMSSP";
+type SicilyMessage = unit() {
+  # Just ensure the signature matches. We could do more,
+  # but it'd be better to forward to an NTLM analyzer.
+  signature: skip b"NTLMSSP";
+  var signature_decoded: string = "NTLMSSP";
 };
 
 type GSS_SPNEGO_Subsequent = unit {
@@ -481,26 +482,30 @@ type BindRequest = unit(inout message: Message, ctx: Ctx&) {
       self.authType = cast<BindAuthType>(cast<uint8>($$.application_id));
       self.authData = $$.application_data;
     }
-    if ((self.authType == BindAuthType::BIND_AUTH_SIMPLE) && (|self.authData| > 0)) {
-      self.simpleCreds = self.authData.decode();
-      if (|self.simpleCreds| > 0) {
-        message.arg = self.simpleCreds;
-      }
-    }
-  }
-  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;
   }
 
+  if ( |self.authData| > 0 ) {
+    switch ( self.authType ) {
+      BindAuthType::BIND_AUTH_SIMPLE ->
+        : void {
+          self.simpleCreds = self.authData.decode();
+          message.arg = self.simpleCreds;
+        }
 
-  sicilyMessage: SicilyMessage(self.authType) &parse-from=self.authData if ((self.authType == BindAuthType::SICILY_NEGOTIATE
-                                                                             || self.authType == BindAuthType::SICILY_RESPONSE)
-                                                                            && (|self.authData| > 0)) {
-    message.arg = self.sicilyMessage.signature.decode();
-  }
+      BindAuthType::BIND_AUTH_SASL ->
+        saslCreds: SaslCredentials {
+          message.arg = self.saslCreds.mechanism;
+          ctx.saslMechanism = self.saslCreds.mechanism;
+        }
 
+      BindAuthType::SICILY_NEGOTIATE, BindAuthType::SICILY_RESPONSE ->
+        sicilyMessage: SicilyMessage {
+          message.arg = self.sicilyMessage.signature_decoded;
+        }
+
+      * -> : void;
+    } &parse-from=self.authData;
+  };
 } &requires=(self?.authType && (self.authType != BindAuthType::Undef));
 
 type ServerSaslCreds = unit {