From 72ea7fc06c8d8a8f9116822df682a55dc70a85a6 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 20 Feb 2025 09:36:18 +0100 Subject: [PATCH] RDP: cookie is optional Fixes GH-4237 --- NEWS | 3 +++ src/analyzer/protocol/rdp/events.bif | 2 +- src/analyzer/protocol/rdp/rdp-analyzer.pac | 8 ++++++- src/analyzer/protocol/rdp/rdp-protocol.pac | 21 +++++++++++++----- .../rdp.log | 11 +++++++++ .../ssl.log | 11 +++++++++ .../Traces/rdp/rdp-no-cookie-mstshash.pcap | Bin 0 -> 4348 bytes .../protocols/rdp/rdp-no-cookie-msthash.zeek | 12 ++++++++++ 8 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/rdp.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/ssl.log create mode 100644 testing/btest/Traces/rdp/rdp-no-cookie-mstshash.pcap create mode 100644 testing/btest/scripts/base/protocols/rdp/rdp-no-cookie-msthash.zeek diff --git a/NEWS b/NEWS index 6e5d14385a..7dce0c9e10 100644 --- a/NEWS +++ b/NEWS @@ -86,6 +86,9 @@ Changed Functionality warnings instead of builtin errors when hitting trouble. This allows Zeek to continue gracefully in case of such problems, particularly during zeek_init(). +- The RDP analyzer now also parses connections that do not contain the cookie + field, which were previously rejected. + Removed Functionality --------------------- diff --git a/src/analyzer/protocol/rdp/events.bif b/src/analyzer/protocol/rdp/events.bif index bed0322698..59d73f8f4d 100644 --- a/src/analyzer/protocol/rdp/events.bif +++ b/src/analyzer/protocol/rdp/events.bif @@ -39,7 +39,7 @@ event rdp_native_encrypted_data%(c: connection, orig: bool, len: count%); ## ## c: The connection record for the underlying transport-layer session/flow. ## -## cookie: The cookie included in the request. +## cookie: The cookie included in the request; empty if no cookie was provided. ## ## flags: The flags set by the client. event rdp_connect_request%(c: connection, cookie: string, flags: count%); diff --git a/src/analyzer/protocol/rdp/rdp-analyzer.pac b/src/analyzer/protocol/rdp/rdp-analyzer.pac index 59f33a625b..184fe668c0 100644 --- a/src/analyzer/protocol/rdp/rdp-analyzer.pac +++ b/src/analyzer/protocol/rdp/rdp-analyzer.pac @@ -10,9 +10,15 @@ refine flow RDP_Flow += { %{ if ( rdp_connect_request ) { + zeek::StringValPtr cookie_value; + if ( ${cr.cookie} ) + cookie_value = to_stringval(${cr.cookie.cookie_value}); + else + cookie_value = zeek::val_mgr->EmptyString(); + zeek::BifEvent::enqueue_rdp_connect_request(connection()->zeek_analyzer(), connection()->zeek_analyzer()->Conn(), - to_stringval(${cr.cookie_value}), + cookie_value, ${cr.rdp_neg_req} ? ${cr.rdp_neg_req.flags} : 0); } diff --git a/src/analyzer/protocol/rdp/rdp-protocol.pac b/src/analyzer/protocol/rdp/rdp-protocol.pac index 3e2f6fde16..c067071f74 100644 --- a/src/analyzer/protocol/rdp/rdp-protocol.pac +++ b/src/analyzer/protocol/rdp/rdp-protocol.pac @@ -74,18 +74,27 @@ type Data_Block = record { # Client X.224 ###################################################################### +type RDP_Connect_Request_cookie = record { + cookie_value: RE/[^\x0d]*/; + cookie_terminator: RE/\x0d\x0a/; +} &byteorder=littleendian; + type Connect_Request(cotp: COTP) = record { destination_reference: uint16; source_reference: uint16; flow_control: uint8; - cookie_mstshash: RE/Cookie: mstshash\=/; - cookie_value: RE/[^\x0d]*/; - cookie_terminator: RE/\x0d\x0a/; + cookie_mstshash: RE/(Cookie: mstshash\=)?/; + # Cookie is optional as per + # https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/18a27ef9-6f9a-4501-b000-94b1fe3c2c10 + switch1: case (sizeof(cookie_mstshash) > 0 ) of { + 0 -> none1: empty; + default -> cookie: RDP_Connect_Request_cookie; + }; # Terrifying little case statement to figure out if there # is any data left in the COTP structure. - switch1: case (offsetof(switch1) + 2 - cotp.cotp_len - 1) of { - 0 -> none: empty; - default -> rdp_neg_req: RDP_Negotiation_Request; + switch2: case (offsetof(switch2) + 2 - cotp.cotp_len - 1) of { + 0 -> none2: empty; + default -> rdp_neg_req: RDP_Negotiation_Request; }; } &byteorder=littleendian; diff --git a/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/rdp.log b/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/rdp.log new file mode 100644 index 0000000000..c03230b258 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/rdp.log @@ -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 rdp +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p cookie result security_protocol client_channels keyboard_layout client_build client_name client_dig_product_id desktop_width desktop_height requested_color_depth cert_type cert_count cert_permanent encryption_level encryption_method +#types time string addr port addr port string string string vector[string] string string string string count count string string count bool string string +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.128.36.245 50204 10.132.153.76 3389 (empty) encrypted HYBRID_EX - - - - - - - - - 0 - - - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/ssl.log b/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/ssl.log new file mode 100644 index 0000000000..39c1d80d14 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.rdp.rdp-no-cookie-msthash/ssl.log @@ -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 ssl +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p version cipher curve server_name resumed last_alert next_protocol established ssl_history cert_chain_fps client_cert_chain_fps subject issuer sni_matches_cert +#types time string addr port addr port string string string string bool string string bool string vector[string] vector[string] string string bool +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.128.36.245 50204 10.132.153.76 3389 TLSv12 TLS_RSA_WITH_AES_256_GCM_SHA384 - - F - - T CsxnGIi 2f24fa5efe1c62447b32aa41af38c4176d03a62ca9c3c8d491946ed283b86cca (empty) CN=CLK-456040-D.one.phoenix.gov CN=CLK-456040-D.one.phoenix.gov - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/rdp/rdp-no-cookie-mstshash.pcap b/testing/btest/Traces/rdp/rdp-no-cookie-mstshash.pcap new file mode 100644 index 0000000000000000000000000000000000000000..2c29203f737743d5a4615be256a4f8a69fce479b GIT binary patch literal 4348 zcmeHKZBSEJ8b0UVB$Dv`C8_cafz$}xU_eoWv_R#fnCL<(h;$JFfj~e+pvXuA){1;| zT?oY`AQcb+ab+mzwA6@EmeICM6fC$!LIo8IA}a-{)jc=1t4%vQ`@>FWJD!<0bI+aJ z=RW6o-uK-1Ts_}bivbFFHXwj#657%pA2h22UPsqt0HC+11p=Yy)3Dt-z#juV-VGm+ zN1Kr|@*HOw1(k0qE%o_oRht$0-@NXH>D7GzU=(UQ#Bm)6DHw?;c;Se*7M|m%VF!l4 z&r$UtjsWnAR``H-Xtlv4Ka!_DUul_`CK*CJt@BJ4z_uI>$L)wC_=)3-N4$+Vu0^Vl zp`{}LY~F}Q$0jWutr5@bLL*za=R3_~=!aDB&I`nvL|B0cmyBQVTEkOoc*%a17t$w^Ed=LKo?z+B&U8W;CpuTXNP8=pjzRma8R%n&I*Qt zt}s)W0y(Gxe3XVBzzr}E z#;1@C^L8l(nvU2(}DEa1A2V$_*#ujO*v)F1$)*>;|l$&Ut(oolFq>33!` z&nBfi%w{M*abes-o&oxQgx3W)4WN9$0U^a65mI>b2#!GvGs@ar(c09k=nCptm(rHe z??dR*=*!*^Y%E|u)J3beGGP4f|)%83b#?XTO^+o9gw(!29j zL*J?SK$ozT?Q4?LVmm`8N>A8uTD;;eG`P&sc)9i+@0)LD=|vYX-g4zv`yIz{R7po2 z346j6`C`SQ=n)K>4u=M*)I~TXbjh2B6bg<}i05@gW+ELSkmMWfb8J)lRBts`1qno} z#1~zbN<2GMUR8cM5u2=;he8aonBnQyxIyiw_HMj0BW^Gc|7&|aw|wrZe~kXp&VjLM zPMPFScY<5vTcAj)_DF7z+vd!-JjEP6@k7*~8{Be(rThSO^ue--B-Q24jZR|6dnv8g zl1_;obx-c=5o|3k7k;&aIs5hatlX!3w0hm@@3RLTifg)?(>aa%+%k*s%2$-<%NL67 zZucr|7<}APB%ENrP5Y;7kd1O6C-G!N)iHr|+B2*F(8_PS#@wzL8#%>g2q!<%&F;U8 zJ58EAep1igvG%Q764>~TU*v(V_iGExP3OhRp$1A|{P4vS2BC+sEpy-am2XI8W1gBfZur4NGtHreY!BI_ zn&dcHQkD@aH}XHt66Ow$-&}hBep0Q@7<-7FDOQ$zyyqdgV-~;k;CJ`n7?%zbkY8kI z_n2Tyk*DSyWB2nOGhL)swPQl&yk!(RuK$Ypq6hh6Regf5^M{7RhMBeFj|GJ#QQ6IW z+jGYE0>arG(){1;#a(E{|K$ukra1@I|Ge_sF;w{ngk}I#M3X|1gIvVf7lie1Xd>?N literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/rdp/rdp-no-cookie-msthash.zeek b/testing/btest/scripts/base/protocols/rdp/rdp-no-cookie-msthash.zeek new file mode 100644 index 0000000000..8773396d01 --- /dev/null +++ b/testing/btest/scripts/base/protocols/rdp/rdp-no-cookie-msthash.zeek @@ -0,0 +1,12 @@ +# Test a trace that does not have a cookie field. + +# @TEST-EXEC: zeek -b -r $TRACES/rdp/rdp-no-cookie-mstshash.pcap %INPUT +# @TEST-EXEC: btest-diff rdp.log +# @TEST-EXEC: btest-diff ssl.log +# @TEST-EXEC: test ! -f analyzer.log +# @TEST-EXEC: test ! -f dpd.log + +@load base/protocols/rdp +@load base/protocols/ssl + +redef SSL::log_include_server_certificate_subject_issuer=T;