diff --git a/CHANGES b/CHANGES index 21f1b9b19c..e1398e8b71 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,22 @@ +2.0-319 | 2012-05-03 13:24:44 -0700 + + * SSL bugfixes and cleanup. (Seth Hall) + + - SSL related files and classes renamed to remove the "binpac" term. + + - A small fix for DPD scripts to make the DPD log more helpful if + there are multiple continued failures. + + - Fixed the SSL analyzer to make it stop doing repeated violation + messages for some handshake failures. + + - Added a $issuer_subject to the SSL log. + + - Created a basic test for SSL. + + - Fixed parsing of TLS server extensions. (Seth Hall) + 2.0-315 | 2012-05-03 11:44:17 -0700 * Add two more TLS extension values that we see in live traffic. diff --git a/VERSION b/VERSION index f2f473e46d..76ad1154f3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.0-315 +2.0-319 diff --git a/scripts/base/frameworks/dpd/main.bro b/scripts/base/frameworks/dpd/main.bro index e8488c3ec1..9eb0b467f8 100644 --- a/scripts/base/frameworks/dpd/main.bro +++ b/scripts/base/frameworks/dpd/main.bro @@ -105,5 +105,8 @@ event protocol_violation(c: connection, atype: count, aid: count, reason: string) &priority=-5 { if ( c?$dpd ) + { Log::write(DPD::LOG, c$dpd); + delete c$dpd; + } } diff --git a/scripts/base/protocols/ssl/main.bro b/scripts/base/protocols/ssl/main.bro index 0b280a6bcf..b5f74d5122 100644 --- a/scripts/base/protocols/ssl/main.bro +++ b/scripts/base/protocols/ssl/main.bro @@ -24,6 +24,8 @@ export { session_id: string &log &optional; ## Subject of the X.509 certificate offered by the server. subject: string &log &optional; + ## Subject of the signer of the X.509 certificate offered by the server. + issuer_subject: string &log &optional; ## NotValidBefore field value from the server certificate. not_valid_before: time &log &optional; ## NotValidAfter field value from the serve certificate. @@ -146,6 +148,7 @@ event x509_certificate(c: connection, is_orig: bool, cert: X509, chain_idx: coun # Also save other certificate information about the primary cert. c$ssl$subject = cert$subject; + c$ssl$issuer_subject = cert$issuer; c$ssl$not_valid_before = cert$not_valid_before; c$ssl$not_valid_after = cert$not_valid_after; } diff --git a/src/Analyzer.cc b/src/Analyzer.cc index 92ca3ecc50..a2a35490e8 100644 --- a/src/Analyzer.cc +++ b/src/Analyzer.cc @@ -34,7 +34,7 @@ #include "Portmap.h" #include "POP3.h" #include "SSH.h" -#include "SSL-binpac.h" +#include "SSL.h" #include "Syslog-binpac.h" #include "ConnSizeAnalyzer.h" @@ -121,8 +121,8 @@ const Analyzer::Config Analyzer::analyzer_configs[] = { HTTP_Analyzer_binpac::InstantiateAnalyzer, HTTP_Analyzer_binpac::Available, 0, false }, { AnalyzerTag::SSL, "SSL", - SSL_Analyzer_binpac::InstantiateAnalyzer, - SSL_Analyzer_binpac::Available, 0, false }, + SSL_Analyzer::InstantiateAnalyzer, + SSL_Analyzer::Available, 0, false }, { AnalyzerTag::SYSLOG_BINPAC, "SYSLOG_BINPAC", Syslog_Analyzer_binpac::InstantiateAnalyzer, Syslog_Analyzer_binpac::Available, 0, false }, diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ce1b25dd42..9f9eb8a60f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -376,7 +376,7 @@ set(bro_SRCS SMB.cc SMTP.cc SSH.cc - SSL-binpac.cc + SSL.cc Scope.cc SerializationFormat.cc SerialObj.cc diff --git a/src/SSL-binpac.cc b/src/SSL.cc similarity index 66% rename from src/SSL-binpac.cc rename to src/SSL.cc index db9a7004d6..218b17080b 100644 --- a/src/SSL-binpac.cc +++ b/src/SSL.cc @@ -1,21 +1,21 @@ -#include "SSL-binpac.h" +#include "SSL.h" #include "TCP_Reassembler.h" #include "Reporter.h" #include "util.h" -SSL_Analyzer_binpac::SSL_Analyzer_binpac(Connection* c) +SSL_Analyzer::SSL_Analyzer(Connection* c) : TCP_ApplicationAnalyzer(AnalyzerTag::SSL, c) { interp = new binpac::SSL::SSL_Conn(this); had_gap = false; } -SSL_Analyzer_binpac::~SSL_Analyzer_binpac() +SSL_Analyzer::~SSL_Analyzer() { delete interp; } -void SSL_Analyzer_binpac::Done() +void SSL_Analyzer::Done() { TCP_ApplicationAnalyzer::Done(); @@ -23,23 +23,22 @@ void SSL_Analyzer_binpac::Done() interp->FlowEOF(false); } -void SSL_Analyzer_binpac::EndpointEOF(TCP_Reassembler* endp) +void SSL_Analyzer::EndpointEOF(TCP_Reassembler* endp) { TCP_ApplicationAnalyzer::EndpointEOF(endp); interp->FlowEOF(endp->IsOrig()); } -void SSL_Analyzer_binpac::DeliverStream(int len, const u_char* data, bool orig) +void SSL_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { TCP_ApplicationAnalyzer::DeliverStream(len, data, orig); assert(TCP()); - if ( TCP()->IsPartial() ) return; if ( had_gap ) - // XXX: If only one side had a content gap, we could still try to + // If only one side had a content gap, we could still try to // deliver data to the other side if the script layer can handle this. return; @@ -53,7 +52,7 @@ void SSL_Analyzer_binpac::DeliverStream(int len, const u_char* data, bool orig) } } -void SSL_Analyzer_binpac::Undelivered(int seq, int len, bool orig) +void SSL_Analyzer::Undelivered(int seq, int len, bool orig) { TCP_ApplicationAnalyzer::Undelivered(seq, len, orig); had_gap = true; diff --git a/src/SSL-binpac.h b/src/SSL.h similarity index 74% rename from src/SSL-binpac.h rename to src/SSL.h index 8dab19d00c..c9f8d9be91 100644 --- a/src/SSL-binpac.h +++ b/src/SSL.h @@ -1,14 +1,13 @@ -#ifndef ssl_binpac_h -#define ssl_binpac_h +#ifndef ssl_h +#define ssl_h #include "TCP.h" - #include "ssl_pac.h" -class SSL_Analyzer_binpac : public TCP_ApplicationAnalyzer { +class SSL_Analyzer : public TCP_ApplicationAnalyzer { public: - SSL_Analyzer_binpac(Connection* conn); - virtual ~SSL_Analyzer_binpac(); + SSL_Analyzer(Connection* conn); + virtual ~SSL_Analyzer(); // Overriden from Analyzer. virtual void Done(); @@ -19,7 +18,7 @@ public: virtual void EndpointEOF(TCP_Reassembler* endp); static Analyzer* InstantiateAnalyzer(Connection* conn) - { return new SSL_Analyzer_binpac(conn); } + { return new SSL_Analyzer(conn); } static bool Available() { diff --git a/src/ssl-analyzer.pac b/src/ssl-analyzer.pac index f41fb8639b..bf9cf1e0ba 100644 --- a/src/ssl-analyzer.pac +++ b/src/ssl-analyzer.pac @@ -25,6 +25,7 @@ string orig_label(bool is_orig); void free_X509(void *); X509* d2i_X509_binpac(X509** px, const uint8** in, int len); + string handshake_type_label(int type); %} %code{ @@ -46,6 +47,27 @@ string orig_label(bool is_orig) return d2i_X509(px, (u_char**) in, len); #endif } + + string handshake_type_label(int type) + { + switch ( type ) { + case HELLO_REQUEST: return string("HELLO_REQUEST"); + case CLIENT_HELLO: return string("CLIENT_HELLO"); + case SERVER_HELLO: return string("SERVER_HELLO"); + case SESSION_TICKET: return string("SESSION_TICKET"); + case CERTIFICATE: return string("CERTIFICATE"); + case SERVER_KEY_EXCHANGE: return string("SERVER_KEY_EXCHANGE"); + case CERTIFICATE_REQUEST: return string("CERTIFICATE_REQUEST"); + case SERVER_HELLO_DONE: return string("SERVER_HELLO_DONE"); + case CERTIFICATE_VERIFY: return string("CERTIFICATE_VERIFY"); + case CLIENT_KEY_EXCHANGE: return string("CLIENT_KEY_EXCHANGE"); + case FINISHED: return string("FINISHED"); + case CERTIFICATE_URL: return string("CERTIFICATE_URL"); + case CERTIFICATE_STATUS: return string("CERTIFICATE_STATUS"); + default: return string(fmt("UNKNOWN (%d)", type)); + } + } + %} @@ -88,15 +110,15 @@ refine connection SSL_Conn += { eof=0; %} - %eof{ - if ( ! eof && - state_ != STATE_CONN_ESTABLISHED && - state_ != STATE_TRACK_LOST && - state_ != STATE_INITIAL ) - bro_analyzer()->ProtocolViolation(fmt("unexpected end of connection in state %s", - state_label(state_).c_str())); - ++eof; - %} + #%eof{ + # if ( ! eof && + # state_ != STATE_CONN_ESTABLISHED && + # state_ != STATE_TRACK_LOST && + # state_ != STATE_INITIAL ) + # bro_analyzer()->ProtocolViolation(fmt("unexpected end of connection in state %s", + # state_label(state_).c_str())); + # ++eof; + #%} %cleanup{ %} @@ -133,11 +155,6 @@ refine connection SSL_Conn += { cipher_suites16 : uint16[], cipher_suites24 : uint24[]) : bool %{ - if ( state_ == STATE_TRACK_LOST ) - bro_analyzer()->ProtocolViolation(fmt("unexpected client hello message from %s in state %s", - orig_label(${rec.is_orig}).c_str(), - state_label(old_state_).c_str())); - if ( ! version_ok(version) ) bro_analyzer()->ProtocolViolation(fmt("unsupported client SSL version 0x%04x", version)); @@ -175,11 +192,6 @@ refine connection SSL_Conn += { cipher_suites24 : uint24[], comp_method : uint8) : bool %{ - if ( state_ == STATE_TRACK_LOST ) - bro_analyzer()->ProtocolViolation(fmt("unexpected server hello message from %s in state %s", - orig_label(${rec.is_orig}).c_str(), - state_label(old_state_).c_str())); - if ( ! version_ok(version) ) bro_analyzer()->ProtocolViolation(fmt("unsupported server SSL version 0x%04x", version)); else @@ -205,7 +217,7 @@ refine connection SSL_Conn += { return true; %} - + function proc_session_ticket_handshake(rec: SessionTicketHandshake, is_orig: bool): bool %{ if ( ssl_session_ticket_handshake ) @@ -229,11 +241,6 @@ refine connection SSL_Conn += { function proc_certificate(rec: SSLRecord, certificates : bytestring[]) : bool %{ - if ( state_ == STATE_TRACK_LOST ) - bro_analyzer()->ProtocolViolation(fmt("unexpected certificate message from %s in state %s", - orig_label(${rec.is_orig}).c_str(), - state_label(old_state_).c_str())); - if ( certificates->size() == 0 ) return true; @@ -362,6 +369,7 @@ refine connection SSL_Conn += { handshake_type_label(${hs.msg_type}).c_str(), orig_label(is_orig).c_str(), state_label(old_state_).c_str())); + return true; %} diff --git a/src/ssl-defs.pac b/src/ssl-defs.pac index 31d90338f5..b13b7c4881 100644 --- a/src/ssl-defs.pac +++ b/src/ssl-defs.pac @@ -17,35 +17,6 @@ enum ContentType { UNKNOWN_OR_V2_ENCRYPTED = 400 }; -%code{ - string* record_type_label(int type) - { - switch ( type ) { - case CHANGE_CIPHER_SPEC: - return new string("CHANGE_CIPHER_SPEC"); - case ALERT: - return new string("ALERT"); - case HANDSHAKE: - return new string("HANDSHAKE"); - case APPLICATION_DATA: - return new string("APPLICATION_DATA"); - case V2_ERROR: - return new string("V2_ERROR"); - case V2_CLIENT_HELLO: - return new string("V2_CLIENT_HELLO"); - case V2_CLIENT_MASTER_KEY: - return new string("V2_CLIENT_MASTER_KEY"); - case V2_SERVER_HELLO: - return new string("V2_SERVER_HELLO"); - case UNKNOWN_OR_V2_ENCRYPTED: - return new string("UNKNOWN_OR_V2_ENCRYPTED"); - - default: - return new string(fmt("UNEXPECTED (%d)", type)); - } - } -%} - enum SSLVersions { UNKNOWN_VERSION = 0x0000, SSLv20 = 0x0002, diff --git a/src/ssl-protocol.pac b/src/ssl-protocol.pac index 627645e4da..0019478518 100644 --- a/src/ssl-protocol.pac +++ b/src/ssl-protocol.pac @@ -23,7 +23,6 @@ type uint24 = record { string state_label(int state_nr); double get_time_from_asn1(const ASN1_TIME * atime); - string handshake_type_label(int type); %} extern type to_int; @@ -268,28 +267,6 @@ enum HandshakeType { CERTIFICATE_STATUS = 22, # RFC 3546 }; -%code{ - string handshake_type_label(int type) - { - switch ( type ) { - case HELLO_REQUEST: return string("HELLO_REQUEST"); - case CLIENT_HELLO: return string("CLIENT_HELLO"); - case SERVER_HELLO: return string("SERVER_HELLO"); - case SESSION_TICKET: return string("SESSION_TICKET"); - case CERTIFICATE: return string("CERTIFICATE"); - case SERVER_KEY_EXCHANGE: return string("SERVER_KEY_EXCHANGE"); - case CERTIFICATE_REQUEST: return string("CERTIFICATE_REQUEST"); - case SERVER_HELLO_DONE: return string("SERVER_HELLO_DONE"); - case CERTIFICATE_VERIFY: return string("CERTIFICATE_VERIFY"); - case CLIENT_KEY_EXCHANGE: return string("CLIENT_KEY_EXCHANGE"); - case FINISHED: return string("FINISHED"); - case CERTIFICATE_URL: return string("CERTIFICATE_URL"); - case CERTIFICATE_STATUS: return string("CERTIFICATE_STATUS"); - default: return string(fmt("UNKNOWN (%d)", type)); - } - } -%} - ###################################################################### # V3 Change Cipher Spec Protocol (7.1.) @@ -425,6 +402,10 @@ type ServerHello(rec: SSLRecord) = record { session_id : uint8[session_len]; cipher_suite : uint16[1]; compression_method : uint8; + # This weirdness is to deal with the possible existence or absence + # of the following fields. + ext_len: uint16[] &until($element == 0 || $element != 0); + extensions : SSLExtension(rec)[] &until($input.length() == 0); } &let { state_changed : bool = $context.connection.transition(STATE_CLIENT_HELLO_RCVD, diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.basic/ssl.log b/testing/btest/Baseline/scripts.base.protocols.ssl.basic/ssl.log new file mode 100644 index 0000000000..74156362e5 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.basic/ssl.log @@ -0,0 +1,8 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path ssl +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p version cipher server_name session_id subject issuer_subject not_valid_before not_valid_after last_alert +#types time string addr port addr port string string string string string string time time string +1335538392.319381 UWkUyAuUGXf 192.168.1.105 62045 74.125.224.79 443 TLSv10 TLS_ECDHE_RSA_WITH_RC4_128_SHA ssl.gstatic.com - CN=*.gstatic.com,O=Google Inc,L=Mountain View,ST=California,C=US CN=Google Internet Authority,O=Google Inc,C=US 1334102677.000000 1365639277.000000 - diff --git a/testing/btest/Traces/tls-conn-with-extensions.trace b/testing/btest/Traces/tls-conn-with-extensions.trace new file mode 100644 index 0000000000..a3b724b3a1 Binary files /dev/null and b/testing/btest/Traces/tls-conn-with-extensions.trace differ diff --git a/testing/btest/scripts/base/protocols/ssl/basic.test b/testing/btest/scripts/base/protocols/ssl/basic.test new file mode 100644 index 0000000000..94b0e87ec1 --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssl/basic.test @@ -0,0 +1,4 @@ +# This tests a normal SSL connection and the log it outputs. + +# @TEST-EXEC: bro -r $TRACES/tls-conn-with-extensions.trace %INPUT +# @TEST-EXEC: btest-diff ssl.log