Merge remote-tracking branch 'origin/master' into topic/vladg/bit-1838

CHANGES

@@ -1,4 +1,31 @@
 
+2.5-310 | 2017-09-21 09:10:21 -0700
+
+  * fix interaction of gridftp scripts with other thresholds. (Justin Azoff)
+
+2.5-307 | 2017-09-20 10:51:09 -0500
+
+  * BIT-1846: Updating broctl submodule to include fix for symlinking
+    issue (Jon Siwek)
+
+2.5-306 | 2017-09-18 14:43:42 -0700
+
+  * Make strerror_r portable, supporting XSI/gnu versions. (Thomas Petersen)
+
+  * Prevent crash when calling bro -U. (Thomas Petersen)
+
+  * Remove annoying error message from connsize bifs. (Johanna Amann)
+
+  * Add test to verify that log rotation works with gzipped logs (Daniel Thayer)
+
+  * Fix ascii writer to not discard a ".gz" file extension. (Daniel Thayer)
+
+    When Bro writes a compressed log, it uses a file extension of ".gz".
+    However, upon log rotation the ascii writer script function
+    "default_rotation_postprocessor_func" was discarding the ".gz"
+    file extension.  Fixed so that the correct file extension is
+    preserved after rotation. (Daniel Thayer)
+
 2.5-297 | 2017-09-11 09:26:33 -0700
 
   * Fix small OCSP parser bug; serial numbers were not passed to events

VERSION

@@ -1 +1 @@
-2.5-297
+2.5-310

aux/broctl

@@ -1 +1 @@
-Subproject commit 1ab5ed3d3b0f2a3ff231de77816a697d55abccb8
+Subproject commit e960be2c192a02f1244ebca3ec31ca57d64e23dc

scripts/base/frameworks/logging/writers/ascii.bro

@@ -79,9 +79,12 @@ export {
 # runs the writer's default postprocessor command on it.
 function default_rotation_postprocessor_func(info: Log::RotationInfo) : bool
 	{
+	# If the filename has a ".gz" extension, then keep it.
+	local gz = info$fname[-3:] == ".gz" ? ".gz" : "";
+
 	# Move file to name including both opening and closing time.
-	local dst = fmt("%s.%s.log", info$path,
-			strftime(Log::default_rotation_date_format, info$open));
+	local dst = fmt("%s.%s.log%s", info$path,
+			strftime(Log::default_rotation_date_format, info$open), gz);
 
 	system(fmt("/bin/mv %s %s", info$fname, dst));
 

scripts/base/protocols/ftp/gridftp.bro

@@ -75,6 +75,9 @@ event ConnThreshold::bytes_threshold_crossed(c: connection, threshold: count, is
 	if ( threshold < size_threshold || "gridftp-data" in c$service || c$duration > max_time )
 		return;
 
+	if ( ! data_channel_initial_criteria(c) )
+		return;
+
 	add c$service["gridftp-data"];
 	event GridFTP::data_channel_detected(c);
 

src/Expr.cc

@@ -4351,9 +4351,8 @@ Val* InExpr::Fold(Val* v1, Val* v2) const
 		const BroString* s1 = v1->AsString();
 		const BroString* s2 = v2->AsString();
 
-		// Could do better here - either roll our own, to deal with
-		// NULs, and/or Boyer-Moore if done repeatedly.
-		return new Val(strstr(s2->CheckString(), s1->CheckString()) != 0, TYPE_BOOL);
+		// Could do better here e.g. Boyer-Moore if done repeatedly.
+		return new Val(strstr_n(s2->Len(), s2->Bytes(), s1->Len(), reinterpret_cast<const unsigned char*>(s1->CheckString())) != -1, TYPE_BOOL);
 		}
 
 	if ( v1->Type()->Tag() == TYPE_ADDR &&

src/File.cc

@@ -302,7 +302,7 @@ FILE* BroFile::BringIntoCache()
 
 	if ( ! f )
 		{
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("can't open %s: %s", name, buf);
 
 		f = fopen("/dev/null", "w");
@@ -313,7 +313,7 @@ FILE* BroFile::BringIntoCache()
 			return f;
 			}
 
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("can't open /dev/null: %s", buf);
 		return 0;
 		}
@@ -323,7 +323,7 @@ FILE* BroFile::BringIntoCache()
 
 	if ( fseek(f, position, SEEK_SET) < 0 )
 		{
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("reopen seek failed: %s", buf);
 		}
 
@@ -413,7 +413,7 @@ void BroFile::Suspend()
 	if ( (position = ftell(f)) < 0 )
 		{
 		char buf[256];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("ftell failed: %s", buf);
 		position = 0;
 		}

src/Flare.cc

@@ -16,7 +16,7 @@ Flare::Flare()
 static void bad_pipe_op(const char* which)
 	{
 	char buf[256];
-	strerror_r(errno, buf, sizeof(buf));
+	bro_strerror_r(errno, buf, sizeof(buf));
 	reporter->FatalErrorWithCore("unexpected pipe %s failure: %s", which, buf);
 	}
 

src/Pipe.cc

@@ -12,7 +12,7 @@ using namespace bro;
 static void pipe_fail(int eno)
 	{
 	char tmp[256];
-	strerror_r(eno, tmp, sizeof(tmp));
+	bro_strerror_r(eno, tmp, sizeof(tmp));
 	reporter->FatalError("Pipe failure: %s", tmp);
 	}
 

src/PolicyFile.cc

@@ -84,7 +84,7 @@ bool LoadPolicyFileText(const char* policy_filename)
 	if ( fstat(fileno(f), &st) != 0 )
 		{
 		char buf[256];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("fstat failed on %s: %s", policy_filename, buf);
 		fclose(f);
 		return false;

src/analyzer/protocol/conn-size/functions.bif

@@ -5,10 +5,7 @@ static analyzer::Analyzer* GetConnsizeAnalyzer(Val* cid)
 	{
 	Connection* c = sessions->FindConnection(cid);
 	if ( ! c )
-		{
-		reporter->Error("cannot find connection");
 		return 0;
-		}
 
 	analyzer::Analyzer* a = c->FindAnalyzer("CONNSIZE");
 	if ( ! a )

src/analyzer/protocol/ssl/tls-handshake-protocol.pac

@@ -487,7 +487,7 @@ type SSLExtension(rec: HandshakeRecord) = record {
 		EXT_SIGNATURE_ALGORITHMS -> signature_algorithm: SignatureAlgorithm(rec)[] &until($element == 0 || $element != 0);
 		EXT_SIGNED_CERTIFICATE_TIMESTAMP -> certificate_timestamp: SignedCertificateTimestampList(rec)[] &until($element == 0 || $element != 0);
 		EXT_KEY_SHARE -> key_share: KeyShare(rec)[] &until($element == 0 || $element != 0);
-		EXT_SUPPORTED_VERSIONS -> supported_versions: SupportedVersions(rec)[] &until($element == 0 || $element != 0);
+ 		EXT_SUPPORTED_VERSIONS -> supported_versions_selector: SupportedVersionsSelector(rec, data_len)[] &until($element == 0 || $element != 0);
 		EXT_PSK_KEY_EXCHANGE_MODES -> psk_key_exchange_modes: PSKKeyExchangeModes(rec)[] &until($element == 0 || $element != 0);
 		default -> data: bytestring &restofdata;
 	};
@@ -495,6 +495,11 @@ type SSLExtension(rec: HandshakeRecord) = record {
 
 %include tls-handshake-signed_certificate_timestamp.pac
 
+type SupportedVersionsSelector(rec: HandshakeRecord, data_len: uint16) = case rec.is_orig of {
+	true -> a: SupportedVersions(rec);
+	false -> b: bytestring &length=data_len &transient;
+}
+
 type SupportedVersions(rec: HandshakeRecord) = record {
 	length: uint8;
 	versions: uint16[] &until($input.length() == 0);

src/analyzer/protocol/tcp/TCP_Endpoint.cc

@@ -229,7 +229,7 @@ int TCP_Endpoint::DataSent(double t, uint64 seq, int len, int caplen,
 		if ( fwrite(data, 1, len, f) < unsigned(len) )
 			{
 			char buf[256];
-			strerror_r(errno, buf, sizeof(buf));
+			bro_strerror_r(errno, buf, sizeof(buf));
 			reporter->Error("TCP contents write failed: %s", buf);
 
 			if ( contents_file_write_failure )

src/file_analysis/analyzer/extract/Extract.cc

@@ -20,7 +20,7 @@ Extract::Extract(RecordVal* args, File* file, const string& arg_filename,
 		{
 		fd = 0;
 		char buf[128];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("cannot open %s: %s", filename.c_str(), buf);
 		}
 	}

src/input/readers/raw/Raw.cc

@@ -90,7 +90,7 @@ bool Raw::SetFDFlags(int fd, int cmd, int flags)
 		return true;
 
 	char buf[256];
-	strerror_r(errno, buf, sizeof(buf));
+	bro_strerror_r(errno, buf, sizeof(buf));
 	Error(Fmt("failed to set fd flags: %s", buf));
 	return false;
 	}
@@ -197,7 +197,7 @@ bool Raw::Execute()
 			else
 				{
 				char buf[256];
-				strerror_r(errno, buf, sizeof(buf));
+				bro_strerror_r(errno, buf, sizeof(buf));
 				Warning(Fmt("Could not set child process group: %s", buf));
 				}
 			}
@@ -293,7 +293,7 @@ bool Raw::OpenInput()
 			if ( fseek(file.get(), pos, whence) < 0 )
 				{
 				char buf[256];
-				strerror_r(errno, buf, sizeof(buf));
+				bro_strerror_r(errno, buf, sizeof(buf));
 				Error(Fmt("Seek failed in init: %s", buf));
 				}
 			}

src/logging/writers/ascii/Ascii.cc

@@ -414,7 +414,7 @@ bool Ascii::DoRotate(const char* rotated_path, double open, double close, bool t
 	if ( rename(fname.c_str(), nname.c_str()) != 0 )
 		{
 		char buf[256];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		Error(Fmt("failed to rename %s to %s: %s", fname.c_str(),
 		          nname.c_str(), buf));
 		FinishedRotation();

src/threading/BasicThread.cc

@@ -98,7 +98,7 @@ const char* BasicThread::Strerror(int err)
 	if ( ! strerr_buffer )
 		strerr_buffer = new char[256];
 
-	strerror_r(err, strerr_buffer, 256);
+	bro_strerror_r(err, strerr_buffer, 256);
 	return strerr_buffer;
 	}
 

src/util.cc

@@ -1012,7 +1012,7 @@ FILE* open_file(const string& path, const string& mode)
 	if ( ! rval )
 		{
 		char buf[256];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		reporter->Error("Failed to open file %s: %s", filename, buf);
 		}
 
@@ -1396,9 +1396,13 @@ void _set_processing_status(const char* status)
 	if ( fd < 0 )
 		{
 		char buf[256];
-		strerror_r(errno, buf, sizeof(buf));
-		reporter->Error("Failed to open process status file '%s': %s",
-		                proc_status_file, buf);
+		bro_strerror_r(errno, buf, sizeof(buf));
+		if ( reporter )
+			reporter->Error("Failed to open process status file '%s': %s",
+			                proc_status_file, buf);
+		else
+			fprintf(stderr, "Failed to open process status file '%s': %s\n",
+			                proc_status_file, buf);
 		errno = old_errno;
 		return;
 		}
@@ -1612,7 +1616,7 @@ void safe_close(int fd)
 	if ( close(fd) < 0 && errno != EINTR )
 		{
 		char buf[128];
-		strerror_r(errno, buf, sizeof(buf));
+		bro_strerror_r(errno, buf, sizeof(buf));
 		fprintf(stderr, "safe_close error %d: %s\n", errno, buf);
 		abort();
 		}
@@ -1745,3 +1749,24 @@ std::string canonify_name(const std::string& name)
 
 	return nname;
 	}
+
+static void strerror_r_helper(char* result, char* buf, size_t buflen)
+	{
+	// Seems the GNU flavor of strerror_r may return a pointer to a static
+	// string. So try to copy as much as possible into desired buffer.
+	auto len = strlen(result);
+	strncpy(buf, result, buflen);
+
+	if ( len >= buflen )
+		buf[buflen - 1] = 0;
+	}
+
+static void strerror_r_helper(int result, char* buf, size_t buflen)
+	{ /* XSI flavor of strerror_r, no-op. */ }
+
+void bro_strerror_r(int bro_errno, char* buf, size_t buflen)
+	{
+	auto res = strerror_r(bro_errno, buf, buflen);
+	// GNU vs. XSI flavors make it harder to use strerror_r.
+	strerror_r_helper(res, buf, buflen);
+	}

src/util.h

@@ -516,4 +516,10 @@ struct CompareString
  */
 std::string canonify_name(const std::string& name);
 
+/**
+ * Reentrant version of strerror(). Takes care of the difference between the
+ * XSI-compliant and the GNU-specific version of strerror_r().
+ */
+void bro_strerror_r(int bro_errno, char* buf, size_t buflen);
+
 #endif

testing/btest/Baseline/scripts.base.files.data_event.basic/.stdout

@@ -0,0 +1,3 @@
+Found
+Found
+Found

testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/.stdout

@@ -0,0 +1 @@
+7e01

testing/btest/Baseline/scripts.base.protocols.ssl.tls13-experiment/ssl.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open	2017-09-10-05-23-15
+#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

testing/btest/scripts/base/files/data_event/basic.bro

@@ -0,0 +1,20 @@
+# Just a very basic test to check if ANALYZER_DATA_EVENT works.
+# Also check if "in" works with binary data.
+# @TEST-EXEC: bro -r $TRACES/pe/pe.trace %INPUT
+# @TEST-EXEC: btest-diff .stdout
+# @TEST-EXEC: btest-diff .stderr
+
+event stream_data(f: fa_file, data: string)
+	{
+	if ( "Windows" in data )
+		{
+		print "Found";
+		}
+	}
+
+event file_new (f: fa_file)
+	{
+	Files::add_analyzer(f, Files::ANALYZER_DATA_EVENT,
+		[$stream_event=stream_data]);
+	}
+

testing/btest/scripts/base/frameworks/logging/ascii-gz-rotate.bro

@@ -0,0 +1,25 @@
+# Test that log rotation works with compressed logs.
+#
+# @TEST-EXEC: bro -b %INPUT
+# @TEST-EXEC: gunzip test.*.log.gz
+#
+
+module Test;
+
+export {
+	redef enum Log::ID += { LOG };
+
+	type Log: record {
+		s: string;
+	} &log;
+}
+
+redef Log::default_rotation_interval = 1hr;
+redef LogAscii::gzip_level = 1;
+
+event bro_init()
+{
+	Log::create_stream(Test::LOG, [$columns=Log]);
+
+	Log::write(Test::LOG, [$s="testing"]);
+}

testing/btest/scripts/base/protocols/ssl/tls13-experiment.test

@@ -0,0 +1,16 @@
+# @TEST-EXEC: bro -C -r $TRACES/tls/chrome-63.0.3211.0-canary-tls_experiment.pcap %INPUT
+# @TEST-EXEC: btest-diff ssl.log
+# @TEST-EXEC: btest-diff .stdout
+
+# This is a trace that uses a completely non-standard way of establishing TLS 1.3; this seems
+# to be an undocumented extension where the TLS version is negotiated via the server sending back
+# an supported_versions extension (which, according to the RFC is strictly prohibited).
+#
+# 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.
+
+event ssl_extension(c: connection, is_orig: bool, code: count, val: string)
+	{
+	if ( ! is_orig && code == 43 )
+		print bytestring_to_hexstr(val);
+	}