From 21888a145a86bbc04d227261cfbcf12b04dba85c Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 3 May 2023 11:16:14 +0100 Subject: [PATCH 1/2] SSL: do not try to disable failed analyzer Currently, if a TLS/DTLS analyzer fails with a protocol violation, we will still try to remove the analyzer later, which results in the following error message: error: connection does not have analyzer specified to disable Now, instead we don't try removing the analyzer anymore, after a violation occurred. --- scripts/base/protocols/ssl/main.zeek | 9 +++++++++ .../.stderr | 1 + .../Traces/tls/tls1.2-protocol-violation.pcap | Bin 0 -> 8601 bytes .../protocols/ssl/tls-protocol-violation.test | 5 +++++ 4 files changed, 15 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.ssl.tls-protocol-violation/.stderr create mode 100644 testing/btest/Traces/tls/tls1.2-protocol-violation.pcap create mode 100644 testing/btest/scripts/base/protocols/ssl/tls-protocol-violation.test diff --git a/scripts/base/protocols/ssl/main.zeek b/scripts/base/protocols/ssl/main.zeek index 3e74950951..f4c2eb4e18 100644 --- a/scripts/base/protocols/ssl/main.zeek +++ b/scripts/base/protocols/ssl/main.zeek @@ -499,6 +499,15 @@ event analyzer_confirmation_info(atype: AllAnalyzers::Tag, info: AnalyzerConfirm } } +event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationInfo) &priority=5 + { + if ( atype == Analyzer::ANALYZER_SSL || atype == Analyzer::ANALYZER_DTLS ) + { + # analyzer errored out; prevent us from trying to remove it later + delete info$c$ssl$analyzer_id; + } + } + event ssl_plaintext_data(c: connection, is_client: bool, record_version: count, content_type: count, length: count) &priority=5 { set_session(c); diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.tls-protocol-violation/.stderr b/testing/btest/Baseline/scripts.base.protocols.ssl.tls-protocol-violation/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.tls-protocol-violation/.stderr @@ -0,0 +1 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. diff --git a/testing/btest/Traces/tls/tls1.2-protocol-violation.pcap b/testing/btest/Traces/tls/tls1.2-protocol-violation.pcap new file mode 100644 index 0000000000000000000000000000000000000000..76a51c00c9c8622b9e52bd02094b8da16f185076 GIT binary patch literal 8601 zcmd6sc{o+w+sF4Aam@3O;baV98>14(kRh2xd%utbN@b1l{U-bN*-+R4(yzhIiYhCtUd+qPNhWq~9YcJ0q-OWV;Xkh)fvH}1I z=!PmkX1cSJ1~7)M;nD{>>pv;4{CtOt)rS_)1pv*ZP)$G+Td<4+08^a_BZdKWM9qQe zYBN}=6uGj(Wdi_6G^PxJLZJ}|S{$hA`T>51My{y`IMV)9U(;Q)v8JBa4FDgpahG9b z*aD(vKpo$I)C96%UdqNi_Z?D!)J-9E09L2ra}R|rtg9h)h6lsbCMTEjmp`!><1c9qS4saOI1QY=UAOpArEvpVio+3k$q=-?NC}#PM4^xfBoYZt9Mm3xfY&4hNe9GWfYq|uhcKb-v9J#vyt~PVOb<-gq(rhX&xypX ztqHWkxuB{WCl};PW{_X)N1@Cg&d(Mi=Ox53_QzaK?lUfBQrY_;yoM>Po;lk8stD%|+)^6aUpKdgExn?Xh_om7l|%pxNKfx!SEIZmD)q{E`D z&{%e)o|gO;kQct8WoOePx;c^Di1__ZzAheafzYKq3&;dNq+zGAaPoHdBf84-fjn>x zj-6H8WWR}y30|9kH}drHB)PfD%YicRGZee{U(fVC4|~WE0t17Hkfa;l!p$X!2>&4l zLuRxIpddG+ygaB3DuJs%Tj*8=+yN>nC@QNdtJ#9&wqN>cWg*W5GQb8{*)eKL$||bs z4{6zv+5~xKXr!xs#p(n(dwcrIvw)5aeI0<5ZGY0`Y4t@mhXC z-fqE8(D2wnR`@xN9cSs}?dFF!@O6=uX8>_y#S4Q_S zkuz!E>dsB?v-A^ZWlf7N?88M=`3lz`5osT*_Ih|2SxtDJ-uHRt3rSj1&s=~pGi^|> zLC27q5$x~6|BdGt~^U6{&LBqk@ha$m-P}UCwg<92k_2ltBs^0?`ZU8^~|J2{hmOv z2q5yJ5EN)eYC$1r1_eaWJRnyjhel61wO1$1)Q~$bGr{n6WvQ^M7bpNXU_x`DIj#oN z;fplC=iY3dP_AOV=CqQqlDF!*0?Y`+3L1?+EjsQD+j|73Wm&MRrAP1IK8ViL6 z4GQt|3JLCowee^TP#xTbWq=x>5qISgNCXNEN`sQ?4Uq^g0S^+%KX9j3H$SL118$gD-oTEJ)%WTAIJ za^$*k1OiP0iJ;;7J&628ob*2sCxN0rwGc5pe|6N)CD4C^*sA{vu%aOpME&aCQFQwq8Ohx|_m8qF9m8~~r_Q;fVDv#-Gr1Fo5dJ(C@!OzY2rp9X2iBd9| zGQA}3A~zpLhUpJh&%>;{nWOkw{ydeF*ljU3p;q6ddqgxjO@`p>Qn&Y$cKJb`r-~Ki z=fy#M?#mvY^Ig_Q#Qert6)OFsu?GjW%XmaJ&atas@{2Ik`=jhp&)AU;=d1Br1Hy#Y zep8<;_Hx(r3J5%UY*Y^3x09?+h^W!#P!gGLJi?@u>;HX^3S~ZZ=~%xF`8DkkIz7JO zXHnZfxum#p(q(X|3q?+oBdw4p>RH~9S>~SB%H|z|QXWM9Z(tN^WR3nTTuF~4?}uV( z%!DwdO8R?Sex(fzf4_BPLhQHDU8fX|ou1(8YDo+VB;g5K^5T#itm01KXRU51lns2H zyz!Qnrg&pFQivbXOI{en!|hP)yuY`DvVysvA8D2FAgBakeVyp@njKN;vEWWD&y$n_0ua&zJez13e^_1m}36z472-p=y|a)yn1paJ;LVIj^0}n z*t!=*Zl88kHTkrMG>H$rAPHSd=c-}wYJLzxowX^Hyi!%^iBV~}+i#OcA5@|qFC2y6 z;~#U0zsjT|B%g*)#HCa9z^V83iBF8@dD1fZ#xJ}-dz&*$bk26yICE8zEu#A_zvI&^ zbM(OZ!XFGbhjq`QoNB{{LS#=9cH20#EIbMMXw$rZ8#QD3Qy`tF2dNfs)VIB~c>JZY z&&LGO?hE$G>vRr*=p49#&Lwr};YwxVi_4G2EoV`6L!vE~e8157Z{ZDZEPm1%fnCj) zYjmasu@I!8E#PV{h9Hasp(-pI;xPc~tnn10GN^WC#}(xm#b0B`y|)fJ2%_X`AUO}zSg(OF`Y%nA?0-wse}@aG_KSeb&rbb20V_*m3q9>s zrjtOipRXcfcz?z22IHH8c@?Vc(L&~tJWnkaf5`IPZhc9=yP-Q8!o#++Rhkjoa}s-1 zVsJ(rGQAhI&l7le>`Tj;x(-=E(vd*J;ttayl)4q%0L-*EYD_l=@oggMV&wQ=_y-M7kl9_Bgz zuNygkAj48JdO^}Sa#t$#s6`sC1&{>a-6lT;&Ke?ZIck>YO_;37pB>< zO@Hil0GBZ-IjBl1;e-E8@d0#|W<&HF{tKGaR^LaN#j}oXgO0*;wgur=1YDRE{VRVCc#kbk=2j^=-%31u=h#*JM5Ur!YO3BDDJe2SwV0kn+Tkg?sgP^~v}Tluoc{q{GRtcn-& zC5?_>xLjvgdtn;I_b##HPajj;-iwkcwvnX<;jd&o~ zVs8ab+UBTShCQE6NY8qC zDvbP6(2j#fM8z$!v@fLQCc#_J@uXSll7jl!x6k72O_5v$^|s$mcZ7z;ZOs@qqQS)| z*M59s_P8sp;&p`ajQ@9jwJ8Rq8HdcdIJ$Jkyqs@3t-M@Yi7zhC%ffsLi))TA^8tFMvdq|gJ4JIrZ08e zkzTSBtwv0?8DY$LoNP~yi@2cG`INpWZrh{O+NMO~7&Lr_fM$XYGGIV{fB*>}K*446 zkrANe?+GJUj_Y^io+*Todv8nHPvmN>Be#Qr4hTg6t7Wqy;z)=+tSJ8eZxwm)Kz!9t zMNL={>MR1w0v*{P0KBv;O6%gcC)afL33u7{y*=xzT;wHj_65r#pw5o8~ocaKV9ZhiRTS(ZL`py&{n67;m$;0Mhnf){MJ%bLOG=DMvwW}!W z>V)XG4DR&lQJ_TYa79;3VSWGoschBTPE^HK*f>MZKe<)8HS7eF5*wYM_8GqaOc&%< z-+BKs4jFcZoZwkEq{!Mw3%El=fy-vM0&K1Y!-`)<{#FrkD{FAwt^8lYirmmF3f&C2 zcfKIF^BnqauFYPTtvLU;0s-fQBQr=(Q%}9gwu7yXPT+oq$8VFUBsc|6B2&tyN$i3q zaVC<5ErHo{eG-YipsN2%Sg~{x2SCTptL3+G7_NxI|VB{%|qni163Q+fo(wy`mtJ0#M3V< zS#+%u626w##yu_UQ0-jFZL%|1ZursI&Js7-nJ(L^&MsOpZ||Y&0*bt_~TGK%jM_+UF=*x<{Jy(RZNU`>JLl19DObB!t*#VlRKk~aBbSI^>y>q*1kwt zds(aC&)q0p(-IcW+N~IZJ5}*ijEN2i7~2u@)#KgY)s_jI(?54-duyx74lZ9XO;LT> zsQ&XktGP3AI;GUaEmsBIXP;H8_kLdLrXwip3Gdg+PT>SiSjBiOIJji&?T+>C;ny5L z*~W1^CE-H;M9jbkF*66Sro^tO$}_DoI<$y*m{v@1F|_BNgllMkHcq%?l55J$Idh8A za%(s~%MBHqB+OLKp+d!`KlQ8)H!(ggM*U_EWc8O=Oj7x_?!L&NxKl2)Fk1Dn!SMkb zwWx97At#ive?f=T7m;2=e39vusZvqng<(Xkh>DXvFL~wFWgf8xBOut6Ul^NFD%8yg zHk^AUJ_t}RJ|Ftiz_~W)sm#KtfO^$TcYK_v)U(GY!SL8O#&_h{tPCG{1t`Csu_NK= zE;72Nh>ppN3mEIpFYz~5q<`A}xv|khvJ|0aY!ilcY<+C{njFn{{Iae?5m_YnSS-TB zXxwe0cK+(0RtL{KxiMkQEX#uu&vqt08K8c6aggT|Dk968i{Ij5)#%H}tqHMMoY+v_ zF||)Y1q#95#UpId#G2c7>@j){e;o2N)04_8cdpK-)Be&zR~~%Dz$PK-4DZsb!In>& z`-UW|Y+j|gJ|-AWaV2M7qKa#8JLJ$U-fy1iAQLu~DiRuf`rS6mD>#inrkuTAD<$vs z17dN0zB7k|dafN=Jk)BbuRz>(-9V}^+^NQi@vHmZ3hW(|uMXmFO)b|`CnJ~D51rLf zNIjAL@^sY69CC@>yIT(rEeNU22=aASUFC19b|Bau!%L7w;yJmgwGI}uqq~wTa(35h z6>!ac?+kA99drL9h_~`uJLN@f#(vPTDx7@ubZ?97qh>Mwa*-I>l~B*1vNxTXZ)M`< zdb4xHRjC&)9+^xlH_O>>P|x)4SqttJk}l;^nicEZ{r!|Ahk!h6H`(jv(^A22zXo?_VnogNk@q3=c247-#}@fKPq$UsckC&6kR0JUH{!5z zBl+n2ab*^JUF0#*V_4N@3R~|sG@eJHm-U3JwKzhZuiPUcHK2w?~WZB;QZXB=S zj(b;2$+9zDjy~vLS9`-^ho4aGOiUinkww4I06Ftd%#5ITKGf5)p$?a-Y`6=`bNfM@AA{R zo@t!WNU5klHQu9m^ZwwVFW#x}<dkDiKuWlnO&i@I)f*;z>R$)ed!a^R}6sG Date: Wed, 3 May 2023 13:41:36 +0100 Subject: [PATCH 2/2] SSL: failing analyzer handling - address review feedback Fold the two analyzer_violation_info events into one. See GH-3012 --- scripts/base/protocols/ssl/main.zeek | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/scripts/base/protocols/ssl/main.zeek b/scripts/base/protocols/ssl/main.zeek index f4c2eb4e18..1389b711db 100644 --- a/scripts/base/protocols/ssl/main.zeek +++ b/scripts/base/protocols/ssl/main.zeek @@ -501,11 +501,6 @@ event analyzer_confirmation_info(atype: AllAnalyzers::Tag, info: AnalyzerConfirm event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationInfo) &priority=5 { - if ( atype == Analyzer::ANALYZER_SSL || atype == Analyzer::ANALYZER_DTLS ) - { - # analyzer errored out; prevent us from trying to remove it later - delete info$c$ssl$analyzer_id; - } } event ssl_plaintext_data(c: connection, is_client: bool, record_version: count, content_type: count, length: count) &priority=5 @@ -523,5 +518,9 @@ event analyzer_violation_info(atype: AllAnalyzers::Tag, info: AnalyzerViolationI { if ( atype == Analyzer::ANALYZER_SSL || atype == Analyzer::ANALYZER_DTLS ) if ( info$c?$ssl ) + { + # analyzer errored out; prevent us from trying to remove it later + delete info$c$ssl$analyzer_id; finish(info$c, T); + } }