ldap: Harden parsing a bit

ASN1Message(True) may go off parsing arbitrary input data as
"something ASN.1" This could be GBs of octet strings or just very
long sequences. Avoid this by open-coding some top-level types expected.

This also tries to avoid some of the &parse-from usages that result
in unnecessary copies of data.

Adds a locally generated PCAP with addRequest/addResponse that we
don't currently handle.
This commit is contained in:
Arne Welzel 2024-07-16 21:53:05 +02:00
parent 31122f335f
commit 0cab87c185
5 changed files with 63 additions and 24 deletions

View file

@ -303,25 +303,30 @@ public type MessageWrapper = unit(ctx: Ctx&) {
public type Message = unit(ctx: Ctx&) {
var messageID: int64;
var opcode: ProtocolOpcode = ProtocolOpcode::Undef;
var applicationBytes: bytes;
var unsetResultDefault: Result;
var result_: Result& = self.unsetResultDefault;
var obj: string = "";
var arg: string = "";
var success: bool = False;
var seqHeaderLen: uint64;
var msgLen: uint64;
: ASN1::ASN1Message(True) {
if (($$.head.tag.type_ == ASN1::ASN1Type::Sequence) &&
($$.body?.seq) &&
(|$$.body.seq.submessages| >= 2)) {
if ($$.body.seq.submessages[0].body?.num_value) {
self.messageID = $$.body.seq.submessages[0].body.num_value;
seqHeader: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::Universal && $$.tag.type_ == ASN1::ASN1Type::Sequence) {
self.msgLen = $$.len.len;
}
if ($$.body.seq.submessages[1]?.application_id) {
self.opcode = cast<ProtocolOpcode>(cast<uint8>($$.body.seq.submessages[1].application_id));
self.applicationBytes = $$.body.seq.submessages[1].application_data;
# Use offset() to determine how many bytes the seqHeader took. This
# needs to be done after the seqHeader field hook.
: void {
self.seqHeaderLen = self.offset();
}
messageID_header: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::Universal && $$.tag.type_ == ASN1::ASN1Type::Integer);
: ASN1::ASN1Body(self.messageID_header, False) {
self.messageID = $$.num_value;
}
protocolOp: ASN1::ASN1Header &requires=($$.tag.class == ASN1::ASN1Class::Application) {
self.opcode = cast<ProtocolOpcode>(cast<uint8>($$.tag.type_));
}
switch ( self.opcode ) {
@ -349,17 +354,15 @@ public type Message = unit(ctx: Ctx&) {
ProtocolOpcode::INTERMEDIATE_RESPONSE -> INTERMEDIATE_RESPONSE: NotImplemented(self);
ProtocolOpcode::MOD_DN_REQUEST -> MOD_DN_REQUEST: NotImplemented(self);
ProtocolOpcode::SEARCH_RESULT_REFERENCE -> SEARCH_RESULT_REFERENCE: NotImplemented(self);
} &parse-from=self.applicationBytes if ( self.opcode );
} &size=self.protocolOp.len.len;
on %error {
self.backtrack();
}
# Ensure some invariants hold after parsing the command.
: void &requires=(self.offset() >= self.seqHeaderLen);
: void &requires=(self.msgLen >= (self.offset() - self.seqHeaderLen));
on %done {
self.success = True;
}
} &requires=((self?.messageID) && (self?.opcode) && (self.opcode != ProtocolOpcode::Undef));
# Eat the controls field if it exists.
: skip bytes &size=self.msgLen - (self.offset() - self.seqHeaderLen);
};
#-----------------------------------------------------------------------------
# Bind Operation
@ -462,7 +465,7 @@ type ServerSaslCreds = unit {
# 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) {
# Do nothing
: skip bytes &eod;
};
type BindRequest = unit(inout message: Message) {

View file

@ -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 46160 127.0.1.1 389 tcp ldap_tcp 3.537413 536 42 SF 0 ShADadFf 11 1116 6 362 -
#close XXXX-XX-XX-XX-XX-XX

View file

@ -0,0 +1,14 @@
### 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 46160 127.0.1.1 389 1 3 bind simple success - cn=admin,dc=example,dc=com REDACTED
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46160 127.0.1.1 389 2 - add success - - -
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46160 127.0.1.1 389 3 - add success - - -
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46160 127.0.1.1 389 4 - unbind - - - -
#close XXXX-XX-XX-XX-XX-XX

Binary file not shown.

View file

@ -0,0 +1,11 @@
# Copyright (c) 2024 by the Zeek Project. See LICENSE for details.
# @TEST-REQUIRES: have-spicy
# @TEST-EXEC: zeek -C -r ${TRACES}/ldap/ldap-add.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: ! test -f dpd.log
# @TEST-EXEC: ! test -f analyzer.log
#
# @TEST-DOC: The addRequest/addResponse operation is not implemented, yet we process it.