From f39efd03172369b373b1f2b1d90d1de76ffa37f5 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Tue, 27 Mar 2018 14:58:06 -0700 Subject: [PATCH] Recognize TLS 1.3 negotiation correctly. The way in which TLS 1.3 is negotiated was changed slightly in later revisions of the standard. The final version is only sent in an extension - while the version field in the server hello still shows TLS 1.2. This patch makes ssl.log show the correct version again. --- scripts/base/protocols/ssl/main.bro | 19 ++++++++++++++++-- .../protocol/ssl/tls-handshake-analyzer.pac | 15 ++++++++++++++ .../protocol/ssl/tls-handshake-protocol.pac | 6 +++++- .../ssl.log | 6 +++--- .../ssl.log | 10 +++++++++ ...tls13draft23-chrome67.0.3368.0-canary.pcap | Bin 0 -> 5816 bytes .../base/protocols/ssl/tls13-experiment.test | 3 +++ .../base/protocols/ssl/tls13-version.test | 4 ++++ 8 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.ssl.tls13-version/ssl.log create mode 100644 testing/btest/Traces/tls/tls13draft23-chrome67.0.3368.0-canary.pcap create mode 100644 testing/btest/scripts/base/protocols/ssl/tls13-version.test diff --git a/scripts/base/protocols/ssl/main.bro b/scripts/base/protocols/ssl/main.bro index 5a50d4a4c3..6ac4260537 100644 --- a/scripts/base/protocols/ssl/main.bro +++ b/scripts/base/protocols/ssl/main.bro @@ -216,14 +216,29 @@ event ssl_server_hello(c: connection, version: count, possible_ts: time, server_ { set_session(c); - c$ssl$version_num = version; - c$ssl$version = version_strings[version]; + # If it is already filled, we saw a supported_versions extensions which overrides this. + if ( ! c$ssl?$version_num ) + { + c$ssl$version_num = version; + c$ssl$version = version_strings[version]; + } c$ssl$cipher = cipher_desc[cipher]; if ( c$ssl?$session_id && c$ssl$session_id == bytestring_to_hexstr(session_id) ) c$ssl$resumed = T; } +event ssl_extension_supported_versions(c: connection, is_orig: bool, versions: index_vec) + { + if ( is_orig || |versions| != 1 ) + return; + + set_session(c); + + c$ssl$version_num = versions[0]; + c$ssl$version = version_strings[versions[0]]; + } + event ssl_ecdh_server_params(c: connection, curve: count, point: string) &priority=5 { set_session(c); diff --git a/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac b/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac index b45fb7d2a9..fde5ee9cbc 100644 --- a/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac +++ b/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac @@ -205,6 +205,17 @@ refine connection Handshake_Conn += { return true; %} + function proc_one_supported_version(rec: HandshakeRecord, version: uint16) : bool + %{ + VectorVal* versions = new VectorVal(internal_type("index_vec")->AsVectorType()); + versions->Assign(0u, new Val(version, TYPE_COUNT)); + + BifEvent::generate_ssl_extension_supported_versions(bro_analyzer(), bro_analyzer()->Conn(), + ${rec.is_orig}, versions); + + return true; + %} + function proc_psk_key_exchange_modes(rec: HandshakeRecord, mode_list: uint8[]) : bool %{ VectorVal* modes = new VectorVal(internal_type("index_vec")->AsVectorType()); @@ -501,6 +512,10 @@ refine typeattr SupportedVersions += &let { proc : bool = $context.connection.proc_supported_versions(rec, versions); }; +refine typeattr OneSupportedVersion += &let { + proc : bool = $context.connection.proc_one_supported_version(rec, version); +}; + refine typeattr PSKKeyExchangeModes += &let { proc : bool = $context.connection.proc_psk_key_exchange_modes(rec, modes); }; diff --git a/src/analyzer/protocol/ssl/tls-handshake-protocol.pac b/src/analyzer/protocol/ssl/tls-handshake-protocol.pac index e6b3754d07..33c75294a8 100644 --- a/src/analyzer/protocol/ssl/tls-handshake-protocol.pac +++ b/src/analyzer/protocol/ssl/tls-handshake-protocol.pac @@ -786,7 +786,7 @@ type SSLExtension(rec: HandshakeRecord) = record { type SupportedVersionsSelector(rec: HandshakeRecord, data_len: uint16) = case rec.is_orig of { true -> a: SupportedVersions(rec); - false -> b: bytestring &length=data_len &transient; + false -> b: OneSupportedVersion(rec); } type SupportedVersions(rec: HandshakeRecord) = record { @@ -794,6 +794,10 @@ type SupportedVersions(rec: HandshakeRecord) = record { versions: uint16[] &until($input.length() == 0); } &length=length+1; +type OneSupportedVersion(rec: HandshakeRecord) = record { + version: uint16; +}; + type PSKKeyExchangeModes(rec: HandshakeRecord) = record { length: uint8; modes: uint8[] &until($input.length() == 0); diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log b/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log index c88237dd18..e7e6bc5f54 100644 --- a/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log @@ -3,8 +3,8 @@ #empty_field (empty) #unset_field - #path ssl -#open 2017-09-10-05-23-15 +#open 2018-03-27-21-54-13 #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 cert_chain_fuids client_cert_chain_fuids subject issuer client_subject client_issuer #types time string addr port addr port string string string string bool string string bool vector[string] vector[string] string string string string -1505019126.007778 CHhAvVGS1DHFjwGM9 192.168.0.2 62873 104.196.219.53 443 TLSv12 TLS_AES_128_GCM_SHA256 x25519 tls.ctf.network T - - T - - - - - - -#close 2017-09-10-05-23-16 +1505019126.007778 CHhAvVGS1DHFjwGM9 192.168.0.2 62873 104.196.219.53 443 unknown-32257 TLS_AES_128_GCM_SHA256 x25519 tls.ctf.network T - - T - - - - - - +#close 2018-03-27-21-54-13 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-version/ssl.log b/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-version/ssl.log new file mode 100644 index 0000000000..b5106d5830 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.tls13-version/ssl.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path ssl +#open 2018-03-27-21-57-37 +#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 cert_chain_fuids client_cert_chain_fuids subject issuer client_subject client_issuer +#types time string addr port addr port string string string string bool string string bool vector[string] vector[string] string string string string +1520820013.621497 CHhAvVGS1DHFjwGM9 192.168.86.23 63449 52.32.149.186 443 TLSv13-draft23 TLS_AES_128_GCM_SHA256 - tls13.crypto.mozilla.org T - - T - - - - - - +#close 2018-03-27-21-57-37 diff --git a/testing/btest/Traces/tls/tls13draft23-chrome67.0.3368.0-canary.pcap b/testing/btest/Traces/tls/tls13draft23-chrome67.0.3368.0-canary.pcap new file mode 100644 index 0000000000000000000000000000000000000000..15c80ef84988c254f02a2dad1afb2cb262109ac6 GIT binary patch literal 5816 zcmdUzc|4R2*T=6hW{f4qzKbOLP9$q(U!r7>vXkt~AX|3H&K3u)`$q;X zCU}4b_#A7F60a2La#4aU9Wt#5Kpp^Ou@70e0aJ6n4z&n#V%_#A1Wo!-qYOM4D8)HA zsAB>E99&o>1PX;gAP53N^RxTd&v0>$=)3cXe?lLFpc}M4EdQW;w;!jAnf{gDP;_ z;CbL-Re^cq0~bIl)_zk(tb6>5Nw>pzK?157$DdZ4x9CBzgcQb!8 zPqRQ*L05h=c{2s@5J&^~0a1VebQP-t%6tGm0VM$#0eS$SakX<16y`H?_H=Z0;JfYM zWn*V|lh47~3ZMoLVFm33Kmm~9LJ_#Q5Ev8zgFz5*I0Od*zyR1G1|S0uLIT795stMG z!rIly!~-9yMI*Vvb!~2;s}_T`yi;h_DFt+f2xm zeW`*$Wy|Xd0=(dtAmCRB3C70!sG(3O4io|qfOiDQU3;xHL$I;7fy6l&aA?Pgu(!ZPEYbGl|3;k9`$3!o zutX{-6z~EI${h;5Fn%rTiO=OFcP(Y_Jx`v>voQgKnDIvtyi4?c-F^WIe_I0NU@q{1 z$#xo>WxH>N3yFPvJ1u0l@F)05z7P}Z?=lOd!J}6F)>bWSu7ml7<3~*ax`#l(93v!K zzTDFzSd-%VW_WvCR#NjW*PnefNrfETJLFUMc)tbyT5rZ{~x>nwJUD>{C3Zp-c8dsm*GoRU8pSlfowUf|3b?Y2%*RmjBxg^wwj|B#BIC zvSbZv-krul`#vcrAFiwH>EQ14c+QOIucasG>$ibNhJ2^4%KV`I7KbT=Ri=lEDAk1a zgpG%nfk7l@k`VD6F7&l5J7AO+6&dkej~LNVx;R=iGAppBOm#Ek`nEQvUBM)=g*yEV zeM05w$f?XCLXHsC!hA!oyO(A-xYW*b51V^gme`j!JLP!wiBucQ^qF}|l4zT{U-

)3eR38TUE-<-2A zK6S|(FWa@G=fuW>Ava@nSIAB*5LRX?Y$_GFl>13_e&H3=&NsK^EpIVnN;Pw^PP*HvzzB^Xw5RvPS-H4V040EIY)Q~<|Vs9gz5*saBqk z0lL3C+wy)R@7Ij?`>-%VGBFKXyjaii{J^2N+JRs4pyb{Z+5(Rk&uoXN`M^3USSP0P zQmz^u`tjzt1HQdF5lr9@CxxdiL(l3RBqS~Hlnf*$C>$sC1+&ygniD+Hvf3F{s%2A8 zY-t6SF)2%asWTKY*@8k6Gg7gr)N2GhDaM)eI||{&6OUzg1*Nk@`I=xa;NNYE&@*NG z!b9ZjS1oj1rC>9K-vy1+C0xel#%>=rZQ2lr714T95>n6OBEd;02K5El$Nfu5FFf2x?Cl2*> zX#62!g1-{>WT5xvG@kh?_h(t7(>Vn|tezxNnXXf}qST%w_zedu35Q!^c3t*Ja=@BK zIr%LcH%tB7-t!cZXZg>~*y|^S#g5n-g`jVRwwbgj5-Y;^RU44~)uVQU5CC{4V8<}R^x9YBZilE!>%;>3%JcwzoxH$q$hU&M(_c9IG-en9 zTKP$z@=va6Hp+&Q!Bu(wGp2ezPN#B6XC>td{4qgcc&GOIjVhwJs?^4pk`hBFW;&?M z4VsMc3H)Szt0IuBDy~|}N!siY{^(xF;=O~sd4}N-iQt&U96H2Q`I5WP9pS!K26QTl zFqDc|jvzE~{slg-<6Ba0i`6=Q%4Z`V^P8tVJ`HSYXoV86#2eS9)}9OwZ{svtA#1Ev z%MY@gzEUVgd5zGmVXo(~NwJg)VaptcTNA7#OY__H@J*ruA{MdyTJ}qE6=yVXX?gWE zn_2Fo7Fw-0xGw9eTMEKkJWwo(dCqmu_t;xH&4rRo;<+}d8!PiSHzxE`DK?*3ehGcU z$f{~2$v4zRb~B1%sSNz(ORVpy8D{1%eqUTUc)7Y^ep|cY75Ri znCrXI$%=gVBuw2j_(BRPffAh4Ua{Zh%=?#VFKmK4d3+=J>m$Y=wZS>6gshUJs%1N4 zNz5KV+&{lu9kJ~&c{EG1yMckE&{SV`s>)IBqNVmXFyPnZgL1`^=A;BZLqVgLD5v{g zFX&>p63!A8P`=%%{60dKG#N*$3ZESMmPAE$$7+$|)IrvX7?TVYqH`~hylT5e0T#vO zVkFb0Mq?(dcT_7HGtx|Z^|dRs^kiZpBt#dYp%1^B4U#UCNVoB8O%fGEZw(8=`OogB zTzooWDM+L6flO&Q*4+tV!2-QaT^Hm#Tw@j1(ag5zDjM#G>DqD} zi}w1eOOcS=qEhc(+Ow$K3LF&Ddbf4GjEYyE4!lVH4sRZxGznALq6Xw1;Dyw@lUHLf z&txhnS#?aX94&>Ax*;7`=O%iM8}4v;dyW_5Z)~B=P0EBHxHqzX-0tjr<#NKe{d;aZ z{#yAsv8|25mAWq)%E|iR0eKq+YOChag~{4*3lRs`^i3y)5ZMpKN@B+9<2H?Ru&Lct z(I!W(Ev~b4?|o@l$zpiL8xr>fPGD`Q``C9qe(|caWovtd-hkb^ z6DgjnVqqUVKC?w2Znm{|R0hUbZ>Ts;*4dn;4}I2Wj*RFzz)1_2@)?q3eYBW)#p_f@ z>#{P}zSd1*H@$Shv2CehzN&9SEzO$uirshL%?|bU7nHGTaoz~W+{^2!toNcwG;6n_ zd!uH|NRn9IChv{GRde^L`YgQ~diAxH+oby79XdxVn@hr_rjt`&jX9QO8~r^B=p~SB zb7!bYo3+j3(nv^Oe!yIbiGBN&%=s0Da_Di%&J|SrG+vO4t0JS5X43xTlcx8Y1R<$Y z{n`Uu8>)d$&l=$h23K=4Y&T5gPKItf4`|cz#&=3ZIktP4P3aU4=YOP;j?D4EcK&wo zKYtWBU}5d}e}2FpkUyLvJb&%{np2jQXv$ zQOlHVTx@hux<6-}<3l;{`Uo>&1Vs1fX#@2`3k9ih3nAUNmjVTt1ZjCi_+BT?~zCS4vpfUOxk+eMWNVN~8#DFL`)nV;;-a*&_ z1#I5g_aDnUeK7B~{lUEJaXKP`G*0Z*1;;#KDRw>Ju6bApZLGUs!9p#0jAuW$q@d$+ zZ;X&shkfboj8ZpgVP4_rS3SaW{iE9$^ZhOBFm}K!ROxyyn z6M&>K;Tz)YcOypVJd>uXA8!#Kh&T2vyLKytGvdi+*Fw{c?i_4)Pw0lfbP)3qmDEt8 zaz~MRN>#8im@2K)zPl@KWjcsf0b&UT+6*dt;BDT!RIb$bcNc<9;;c+D2|)$%2NONS z*X-o%cYDR%?6Z1z)#uu7BMM}>C~;W~AMt6R+VJex%9$7zPR2<2CmKI=*6!(|IWy8^ z_)4^&gP-IK_Og5gMKl1D1Z&4d1k?b zNnK5_(V$^9b?QlD9}oR4tfx<)rwgDLto`Om1W-Ek)OyU5`JpFLVEhcYnL1iF%pH2d z61mEVfwLeDYsZqD8Q}Q$29N4kl7kyT0MG;%r7qxJ^S`Ei{Yhc`zoPJiXXK#({i)Hh zGXnmmS&L${g48$IhKlm|5u>z`JaRtwUxw=|oB#j- literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/ssl/tls13-experiment.test b/testing/btest/scripts/base/protocols/ssl/tls13-experiment.test index d49cd928f6..e074535692 100644 --- a/testing/btest/scripts/base/protocols/ssl/tls13-experiment.test +++ b/testing/btest/scripts/base/protocols/ssl/tls13-experiment.test @@ -8,6 +8,9 @@ # # This only seems to happen with Chrome talking to google servers. We do not recognize this as # TLS 1.3, but we do not abort when encountering traffic like this. +# +# In the meantime this way of establishing TLS 1.3 was standardized. Still keeping the test even +# though we parse this correctly now. event ssl_extension(c: connection, is_orig: bool, code: count, val: string) { diff --git a/testing/btest/scripts/base/protocols/ssl/tls13-version.test b/testing/btest/scripts/base/protocols/ssl/tls13-version.test new file mode 100644 index 0000000000..9194c861e1 --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssl/tls13-version.test @@ -0,0 +1,4 @@ +# @TEST-EXEC: bro -C -r $TRACES/tls/tls13draft23-chrome67.0.3368.0-canary.pcap %INPUT +# @TEST-EXEC: btest-diff ssl.log + +# Test that we correctly parse the version out of the extension in an 1.3 connection