From: Eloy Pérez González <zer1t0ps@protonmail.com>
Date: Fri, 22 Oct 2021 13:32:41 +0000 (+0200)
Subject: dcerpc: avoid delete the rpc state interface context
X-Git-Tag: suricata-7.0.0-beta1~988
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1aca2676a6965056582c3da85e4bb2058aca43b0;p=thirdparty%2Fsuricata.git

dcerpc: avoid delete the rpc state interface context

The bug:
The dcerpc dce_iface keyword just match the packet following the bind. Only the
next request after the rpc is sent will match. However the expected behaviour it
that all the rpc requests/responses sent under the context of the given
interface would match.

In the Open Group c706 the following is indicated:

In 2.2.1 Binding-related Operations, indicates that one category of binding
operations are those that "operations that establish internal call routing
information for the server." (The other are to establish the protocol which is
not relevant here). And the following statement can be found:

Operations in the second category establish a set of mappings that the server
can use to route calls internally to the appropriate manager routine. This
routing is based on the interface and version, operation and any object
requested by the call.

It indicates that server routes (to call methods) are based on the operation,
interface and object.

- Operation: To indicate the method to call, and operation number is
             specified as indicated in the second step of 2.3.3.2 (Client
             Binding Steps).
- Interface: An interface is a set of remotely callable operations offered by a
             server and invokable by clients. (2.1.1.1)
- Object: Is the manager that implements the interface, as stated in section
          Interface and Manager Selection of 2.3.3.3. It is not mandatory, can
          be nil.

To call a method, a client must send a request message as defined in 2.6.4.9,
that contains these identifiers:

- opnum: The opnum field identifies the operation being invoked within the
         interface.
- p_cont_id (Context ID in Wireshark): The p_cont_id field holds a presentation
                                       context identifier that identifies the
                                       data representation and interface, as
                                       defined in 12.6.3.4 (Context Identifiers).
- object: The object field is contained if the PFC_OBJECT_UUID is set. (Could be
          interesting to create a keyword dce_object for matching this UUID)

Therefore, to get the correct method to invoke, the server must map the context
to the correct interface. This is negotiated by the bind request

Interfaces are first negotiated using the bind message (12.6.4.3), contained in
the p_context_elem array. Then they are accepted or rejected using the bind_ack
message (12.6.4.4).

Once these contexts are established, both client and server can use the context
id, which is the index of the p_context_elem array, to refer the interface they
are using.

Moreover, in the middle of the connection, the context can be changed with the
alter_context message.

This is way suricata shouldn't delete the bindack attribute, that contains
the contexts, used by match_backuuid. This is the only way to know the interface
a request message is referring to.

ticket: 4769
https://redmine.openinfosecfoundation.org/issues/4769
---

diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs
index 3e2e3b635d..d532223697 100644
--- a/rust/src/dcerpc/dcerpc.rs
+++ b/rust/src/dcerpc/dcerpc.rs
@@ -526,19 +526,6 @@ impl DCERPCState {
         None
     }
 
-    pub fn handle_bind_cache(&mut self, call_id: u32, is_response: bool) {
-        if self.clear_bind_cache == true {
-            self.bind = None;
-            self.bindack = None;
-        }
-        if self.prev_tx_call_id == call_id && is_response == true {
-            self.clear_bind_cache = true;
-        } else {
-            self.clear_bind_cache = false;
-        }
-        self.prev_tx_call_id = call_id;
-    }
-
     pub fn parse_data_gap(&mut self, direction: Direction) -> AppLayerResult {
         match direction {
             Direction::ToServer => {
@@ -1013,7 +1000,6 @@ impl DCERPCState {
                     if retval == -1 {
                         return AppLayerResult::err();
                     }
-                    self.handle_bind_cache(current_call_id, false);
                 }
                 DCERPC_TYPE_BINDACK | DCERPC_TYPE_ALTER_CONTEXT_RESP => {
                     retval = self.process_bindack_pdu(&buffer[parsed as usize..]);
@@ -1034,7 +1020,6 @@ impl DCERPCState {
                     if let Some(flow) = self.flow {
                         sc_app_layer_parser_trigger_raw_stream_reassembly(flow, Direction::ToClient as i32);
                     }
-                    self.handle_bind_cache(current_call_id, false);
                 }
                 DCERPC_TYPE_REQUEST => {
                     retval = self.process_request_pdu(&buffer[parsed as usize..]);
@@ -1043,7 +1028,6 @@ impl DCERPCState {
                     }
                     // In case the response came first, the transaction would complete later when
                     // the corresponding request also comes through
-                    self.handle_bind_cache(current_call_id, false);
                 }
                 DCERPC_TYPE_RESPONSE => {
                     let transaction = self.get_tx_by_call_id(current_call_id, Direction::ToClient);
@@ -1065,7 +1049,6 @@ impl DCERPCState {
                     if retval < 0 {
                         return AppLayerResult::err();
                     }
-                    self.handle_bind_cache(current_call_id, true);
                 }
                 _ => {
                     SCLogDebug!("Unrecognized packet type: {:?}", x);