Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect: add ldap.request.option keyword - v1 #12321

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/userguide/rules/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ Suricata Rules
differences-from-snort
multi-buffer-matching
tag
ldap-keywords
44 changes: 44 additions & 0 deletions doc/userguide/rules/ldap-keywords.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
LDAP Keywords
=============

.. role:: example-rule-action
.. role:: example-rule-header
.. role:: example-rule-options
.. role:: example-rule-emphasis

ldap.request.operation
----------------------

Suricata has a ``ldap.request.operation`` keyword that can be used in signatures to identify
and filter network packets based on Lightweight Directory Access Protocol request operations.

Syntax::

ldap.request.operation: operation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good from a brief overview! Would like other opinions on the name of the keyword. This looks good to me but perhaps too long?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For info, this name comes from json schema


ldap.request.operation uses :ref:`unsigned 8-bit integer <rules-integer-keywords>`.

.. table:: **Operation values for ldap.request.operation keyword**

==== ================================================
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you implement ldap.response.operation, the same table should be used for both

Code Request Operation
==== ================================================
0 BindRequest
2 UnbindRequest
3 SearchRequest
6 ModifyRequest
8 AddRequest
10 DelRequest
12 ModDnRequest
14 CompareRequest
23 ExtendedRequest
==== ================================================

Example
^^^^^^^^

Example of a signature that would alert if the packet has an LDAP bind request operation:

.. container:: example-rule

alert tcp any any -> any any (msg:"Test LDAP bind request"; :example-rule-emphasis:`ldap.request.operation:0;` sid:1;)
94 changes: 94 additions & 0 deletions rust/src/ldap/detect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/* Copyright (C) 2024 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
* Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* version 2 along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
* 02110-1301, USA.
*/

use super::ldap::{LdapTransaction, ALPROTO_LDAP};
use crate::detect::uint::{
rs_detect_u8_free, rs_detect_u8_match, rs_detect_u8_parse, DetectUintData,
};
use crate::detect::{
DetectHelperBufferRegister, DetectHelperKeywordRegister, DetectSignatureSetAppProto,
SCSigTableElmt, SigMatchAppendSMToList,
};

use std::os::raw::{c_int, c_void};

static mut G_LDAP_REQUEST_OPERATION_KW_ID: c_int = 0;
static mut G_LDAP_REQUEST_OPERATION_BUFFER_ID: c_int = 0;

unsafe extern "C" fn ldap_detect_request_operation_setup(
de: *mut c_void, s: *mut c_void, raw: *const libc::c_char,
) -> c_int {
if DetectSignatureSetAppProto(s, ALPROTO_LDAP) != 0 {
return -1;
}
let ctx = rs_detect_u8_parse(raw) as *mut c_void;
if ctx.is_null() {
return -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can implement a protocol_op from_str and call it there if rs_detect_u8_parse fails

}
if SigMatchAppendSMToList(
de,
s,
G_LDAP_REQUEST_OPERATION_KW_ID,
ctx,
G_LDAP_REQUEST_OPERATION_BUFFER_ID,
)
.is_null()
{
ldap_detect_request_operation_free(std::ptr::null_mut(), ctx);
return -1;
}
return 0;
}

unsafe extern "C" fn ldap_detect_request_operation_match(
_de: *mut c_void, _f: *mut c_void, _flags: u8, _state: *mut c_void, tx: *mut c_void,
_sig: *const c_void, ctx: *const c_void,
) -> c_int {
let tx = cast_pointer!(tx, LdapTransaction);
let ctx = cast_pointer!(ctx, DetectUintData<u8>);
let option: u8 = match &tx.request {
Some(request) => request.protocol_op.to_u8(),
None => 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should return 0, instead of having option=0

Otherwise, we match on ldap.request.operation: 0 for responses

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better call rs_detect_u8_match(option, ctx) only if let Some(request) = &tx.request{ and return 0 by default at the end of function

};
return rs_detect_u8_match(option, ctx);
}

unsafe extern "C" fn ldap_detect_request_operation_free(_de: *mut c_void, ctx: *mut c_void) {
// Just unbox...
let ctx = cast_pointer!(ctx, DetectUintData<u8>);
rs_detect_u8_free(ctx);
}

#[no_mangle]
pub unsafe extern "C" fn ScDetectLdapRegister() {
let kw = SCSigTableElmt {
name: b"ldap.request.operation\0".as_ptr() as *const libc::c_char,
desc: b"match LDAP request operation\0".as_ptr() as *const libc::c_char,
url: b"/rules/ldap-keywords.html#ldap.request.operation\0".as_ptr() as *const libc::c_char,
AppLayerTxMatch: Some(ldap_detect_request_operation_match),
Setup: ldap_detect_request_operation_setup,
Free: Some(ldap_detect_request_operation_free),
flags: 0,
};
G_LDAP_REQUEST_OPERATION_KW_ID = DetectHelperKeywordRegister(&kw);
G_LDAP_REQUEST_OPERATION_BUFFER_ID = DetectHelperBufferRegister(
b"ldap.request.operation\0".as_ptr() as *const libc::c_char,
ALPROTO_LDAP,
false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment meaning of these true/false values

true,
);
}
2 changes: 1 addition & 1 deletion rust/src/ldap/ldap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static LDAP_MAX_TX_DEFAULT: usize = 256;

static mut LDAP_MAX_TX: usize = LDAP_MAX_TX_DEFAULT;

static mut ALPROTO_LDAP: AppProto = ALPROTO_UNKNOWN;
pub(super) static mut ALPROTO_LDAP: AppProto = ALPROTO_UNKNOWN;

const STARTTLS_OID: &str = "1.3.6.1.4.1.1466.20037";

Expand Down
1 change: 1 addition & 0 deletions rust/src/ldap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ pub mod filters;
pub mod ldap;
pub mod logger;
pub mod types;
pub mod detect;
28 changes: 28 additions & 0 deletions rust/src/ldap/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,34 @@ pub enum ProtocolOp {
Unknown,
}

impl ProtocolOp {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasonish what do you think about this rust code that works ?

pub fn to_u8(&self) -> u8 {
match self {
ProtocolOp::BindRequest(_) => 0,
ProtocolOp::BindResponse(_) => 1,
ProtocolOp::UnbindRequest => 2,
ProtocolOp::SearchRequest(_) => 3,
ProtocolOp::SearchResultEntry(_) => 4,
ProtocolOp::SearchResultDone(_) => 5,
ProtocolOp::SearchResultReference(_) => 19,
ProtocolOp::ModifyRequest(_) => 6,
ProtocolOp::ModifyResponse(_) => 7,
ProtocolOp::AddRequest(_) => 8,
ProtocolOp::AddResponse(_) => 9,
ProtocolOp::DelRequest(_) => 10,
ProtocolOp::DelResponse(_) => 11,
ProtocolOp::ModDnRequest(_) => 12,
ProtocolOp::ModDnResponse(_) => 13,
ProtocolOp::CompareRequest(_) => 14,
ProtocolOp::CompareResponse(_) => 15,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why is there a hole for 16, 17... ?

Maybe something that was implement in LDAP v1 and is obsolete in LDAP v3 but we should still recognize ?

ProtocolOp::ExtendedRequest(_) => 23,
ProtocolOp::ExtendedResponse(_) => 24,
ProtocolOp::IntermediateResponse(_) => 25,
ProtocolOp::Unknown => 255,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not ideal
There should be some documentation at least
Or even better, we should have ProtocolOp::Unknown(u) => u.opcode, but this may require big changes...

}
}
}

impl Display for ProtocolOp {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Expand Down
1 change: 1 addition & 0 deletions src/detect-engine-register.c
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ void SigTableSetup(void)
ScDetectRfbRegister();
ScDetectSipRegister();
ScDetectTemplateRegister();
ScDetectLdapRegister();

/* close keyword registration */
DetectBufferTypeCloseRegistration();
Expand Down
Loading