From 50370511615574ec09be0e1b6be465c0e3dc2b7f Mon Sep 17 00:00:00 2001
From: Pierre Chifflier <chifflier@wzdftpd.net>
Date: Thu, 19 Apr 2018 23:15:42 +0200
Subject: [PATCH] Kerberos 5: rename weak crypto to weak encryption, and log it

---
 rules/kerberos-events.rules |  2 +-
 rust/src/krb/krb5.rs        | 41 ++++++++++++++++++++-----------------
 rust/src/krb/log.rs         |  3 ++-
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/rules/kerberos-events.rules b/rules/kerberos-events.rules
index 5e23958cbe..523140d415 100644
--- a/rules/kerberos-events.rules
+++ b/rules/kerberos-events.rules
@@ -5,4 +5,4 @@
 # These sigs fire at most once per connection.
 #
 alert krb5 any any -> any any (msg:"SURICATA Kerberos 5 malformed request data"; flow:to_server; app-layer-event:krb5.malformed_data; classtype:protocol-command-decode; sid:2226000; rev:1;)
-alert krb5 any any -> any any (msg:"SURICATA Kerberos 5 weak cryptographic parameters"; flow:to_client; app-layer-event:krb5.weak_crypto; classtype:protocol-command-decode; sid:2226001; rev:1;)
+alert krb5 any any -> any any (msg:"SURICATA Kerberos 5 weak encryption parameters"; flow:to_client; app-layer-event:krb5.weak_encryption; classtype:protocol-command-decode; sid:2226001; rev:1;)
diff --git a/rust/src/krb/krb5.rs b/rust/src/krb/krb5.rs
index 7ea6c83076..92512b4481 100644
--- a/rust/src/krb/krb5.rs
+++ b/rust/src/krb/krb5.rs
@@ -34,7 +34,7 @@ use log::*;
 #[repr(u32)]
 pub enum KRB5Event {
     MalformedData = 0,
-    WeakCrypto,
+    WeakEncryption,
 }
 
 pub struct KRB5State {
@@ -115,7 +115,9 @@ impl KRB5State {
                             tx.sname = Some(kdc_rep.ticket.sname);
                             tx.etype = Some(kdc_rep.enc_part.etype);
                             self.transactions.push(tx);
-                            self.check_crypto(kdc_rep.enc_part.etype);
+                            if test_weak_encryption(kdc_rep.enc_part.etype) {
+                                self.set_event(KRB5Event::WeakEncryption);
+                            }
                         });
                         self.req_id = 0;
                     },
@@ -132,7 +134,9 @@ impl KRB5State {
                             tx.sname = Some(kdc_rep.ticket.sname);
                             tx.etype = Some(kdc_rep.enc_part.etype);
                             self.transactions.push(tx);
-                            self.check_crypto(kdc_rep.enc_part.etype);
+                            if test_weak_encryption(kdc_rep.enc_part.etype) {
+                                self.set_event(KRB5Event::WeakEncryption);
+                            }
                         });
                         self.req_id = 0;
                     },
@@ -172,21 +176,6 @@ impl KRB5State {
         }
     }
 
-    fn check_crypto(&mut self, alg:EncryptionType) {
-        match alg {
-            EncryptionType::AES128_CTS_HMAC_SHA1_96 |
-            EncryptionType::AES256_CTS_HMAC_SHA1_96 |
-            EncryptionType::AES128_CTS_HMAC_SHA256_128 |
-            EncryptionType::AES256_CTS_HMAC_SHA384_192 |
-            EncryptionType::CAMELLIA128_CTS_CMAC |
-            EncryptionType::CAMELLIA256_CTS_CMAC => (),
-            _ => { // all other ciphers are weak or deprecated
-                SCLogDebug!("Kerberos5: weak encryption {:?}", alg);
-                self.set_event(KRB5Event::WeakCrypto);
-            }
-        }
-    }
-
     pub fn free(&mut self) {
         // All transactions are freed when the `transactions` object is freed.
         // But let's be explicit
@@ -247,6 +236,20 @@ impl Drop for KRB5Transaction {
     }
 }
 
+/// Return true if Kerberos `EncryptionType` is weak
+pub fn test_weak_encryption(alg:EncryptionType) -> bool {
+    match alg {
+        EncryptionType::AES128_CTS_HMAC_SHA1_96 |
+        EncryptionType::AES256_CTS_HMAC_SHA1_96 |
+        EncryptionType::AES128_CTS_HMAC_SHA256_128 |
+        EncryptionType::AES256_CTS_HMAC_SHA384_192 |
+        EncryptionType::CAMELLIA128_CTS_CMAC |
+        EncryptionType::CAMELLIA256_CTS_CMAC => false,
+        _ => true, // all other ciphers are weak or deprecated
+    }
+}
+
+
 
 
 
@@ -377,7 +380,7 @@ pub extern "C" fn rs_krb5_state_get_event_info(event_name: *const libc::c_char,
         Ok(s) => {
             match s {
                 "malformed_data"     => KRB5Event::MalformedData as i32,
-                "weak_crypto"        => KRB5Event::WeakCrypto as i32,
+                "weak_encryption"    => KRB5Event::WeakEncryption as i32,
                 _                    => -1, // unknown event
             }
         },
diff --git a/rust/src/krb/log.rs b/rust/src/krb/log.rs
index c910d11ffc..07f2dfecfd 100644
--- a/rust/src/krb/log.rs
+++ b/rust/src/krb/log.rs
@@ -18,7 +18,7 @@
 // written by Pierre Chifflier  <chifflier@wzdftpd.net>
 
 use json::*;
-use krb::krb5::{KRB5State,KRB5Transaction};
+use krb::krb5::{KRB5State,KRB5Transaction,test_weak_encryption};
 
 #[no_mangle]
 pub extern "C" fn rs_krb5_log_json_response(_state: &mut KRB5State, tx: &mut KRB5Transaction) -> *mut JsonT
@@ -52,6 +52,7 @@ pub extern "C" fn rs_krb5_log_json_response(_state: &mut KRB5State, tx: &mut KRB
     js.set_string("realm", &realm);
     js.set_string("sname", &sname);
     js.set_string("encryption", &encryption);
+    js.set_boolean("weak_encryption", tx.etype.map_or(false,test_weak_encryption));
     return js.unwrap();
 }