Merge branch 'topic/jgras/base64-logging' of https://github.com/J-Gras/bro

* 'topic/jgras/base64-logging' of https://github.com/J-Gras/bro:
  Update calls of Base64 functions.
  Refactoring of Base64 functions.

I've removed the additional bif for encoding with a connection, as I'm
not sure there's much of a use case for it; we can always add it back
later if it turns out there is. I've also renamed
decode_base64_intern() to decode_base64_conn() to be a bit more
explicit about the difference.

CHANGES

@@ -1,4 +1,26 @@
 
+2.4-108 | 2015-08-30 20:14:31 -0700
+
+   * Update Base64 decoding.  (Jan Grashoefer)
+     
+        - A new built-in function, decode_base64_conn() for Base64
+          decoding. It works like decode_base64() but receives an
+          additional connection argument that will be used for
+          reporting decoding errors into weird.log (instead of
+          reporter.log).
+
+        - FTP, POP3, and HTTP analyzers now likewise log Base64
+          decoding errors to weird.log.
+
+        - The built-in functions decode_base64_custom() and
+          encode_base64_custom() are now deprecated. Their
+          functionality is provided directly by decode_base64() and
+          encode_base64(), which take an optional parameter to change
+          the Base64 alphabet.
+
+  * Fix potential crash if TCP header was captured incompletely.
+    (Robin Sommer)
+    
 2.4-103 | 2015-08-29 10:51:55 -0700
 
   * Make ASN.1 date/time parsing more robust. (Johanna Amann)

NEWS

@@ -28,11 +28,25 @@ New Functionality
   information. Use with care, generating events per packet is
   expensive.
 
+- A new built-in function, decode_base64_conn() for Base64 decoding.
+  It works like decode_base64() but receives an additional connection
+  argument that will be used for decoding errors into weird.log
+  (instead of reporter.log).
+
 - New Bro plugins in aux/plugins:
 
     - pf_ring: Native PF_RING support.
     - redis:   An experimental log writer for Redis.
 
+Deprecated Functionality
+------------------------
+
+    - The built-in functions decode_base64_custom() and
+      encode_base64_custom() are no longer needed and will be removed
+      in the future. Their functionality is now provided directly by
+      decode_base64() and encode_base64(), which take an optional
+      parameter to change the Base64 alphabet.
+
 Bro 2.4
 =======
 

VERSION

@@ -1 +1 @@
-2.4-103
+2.4-108

scripts/base/protocols/http/main.bro

@@ -270,7 +270,7 @@ event http_header(c: connection, is_orig: bool, name: string, value: string) &pr
 			{
 			if ( /^[bB][aA][sS][iI][cC] / in value )
 				{
-				local userpass = decode_base64(sub(value, /[bB][aA][sS][iI][cC][[:blank:]]/, ""));
+				local userpass = decode_base64_conn(c$id, sub(value, /[bB][aA][sS][iI][cC][[:blank:]]/, ""));
 				local up = split_string(userpass, /:/);
 				if ( |up| >= 2 )
 					{

src/Base64.cc

@@ -82,7 +82,7 @@ int* Base64Converter::InitBase64Table(const string& alphabet)
 	return base64_table;
 	}
 
-Base64Converter::Base64Converter(analyzer::Analyzer* arg_analyzer, const string& arg_alphabet)
+Base64Converter::Base64Converter(Connection* arg_conn, const string& arg_alphabet)
 	{
 	if ( arg_alphabet.size() > 0 )
 		{
@@ -98,7 +98,7 @@ Base64Converter::Base64Converter(analyzer::Analyzer* arg_analyzer, const string&
 	base64_group_next = 0;
 	base64_padding = base64_after_padding = 0;
 	errored = 0;
-	analyzer = arg_analyzer;
+	conn = arg_conn;
 	}
 
 Base64Converter::~Base64Converter()
@@ -216,9 +216,9 @@ int Base64Converter::Done(int* pblen, char** pbuf)
 	}
 
 
-BroString* decode_base64(const BroString* s, const BroString* a)
+BroString* decode_base64(const BroString* s, const BroString* a, Connection* conn)
 	{
-	if ( a && a->Len() != 64 )
+	if ( a && a->Len() != 0 && a->Len() != 64 )
 		{
 		reporter->Error("base64 decoding alphabet is not 64 characters: %s",
 		                a->CheckString());
@@ -229,7 +229,7 @@ BroString* decode_base64(const BroString* s, const BroString* a)
 	int rlen2, rlen = buf_len;
 	char* rbuf2, *rbuf = new char[rlen];
 
-	Base64Converter dec(0, a ? a->CheckString() : "");
+	Base64Converter dec(conn, a ? a->CheckString() : "");
 	if ( dec.Decode(s->Len(), (const char*) s->Bytes(), &rlen, &rbuf) == -1 )
 		goto err;
 
@@ -248,9 +248,9 @@ err:
 	return 0;
 	}
 
-BroString* encode_base64(const BroString* s, const BroString* a)
+BroString* encode_base64(const BroString* s, const BroString* a, Connection* conn)
 	{
-	if ( a && a->Len() != 64 )
+	if ( a && a->Len() != 0 && a->Len() != 64 )
 		{
 		reporter->Error("base64 alphabet is not 64 characters: %s",
 		                a->CheckString());
@@ -259,7 +259,7 @@ BroString* encode_base64(const BroString* s, const BroString* a)
 
 	char* outbuf = 0;
 	int outlen = 0;
-	Base64Converter enc(0, a ? a->CheckString() : "");
+	Base64Converter enc(conn, a ? a->CheckString() : "");
 	enc.Encode(s->Len(), (const unsigned char*) s->Bytes(), &outlen, &outbuf);
 
 	return new BroString(1, (u_char*)outbuf, outlen);

src/Base64.h

@@ -8,15 +8,17 @@
 #include "util.h"
 #include "BroString.h"
 #include "Reporter.h"
-#include "analyzer/Analyzer.h"
+#include "Conn.h"
 
 // Maybe we should have a base class for generic decoders?
 class Base64Converter {
 public:
-	// <analyzer> is used for error reporting, and it should be zero when
-	// the decoder is called by the built-in function decode_base64() or encode_base64().
-	// Empty alphabet indicates the default base64 alphabet.
-	Base64Converter(analyzer::Analyzer* analyzer, const string& alphabet = "");
+	// <conn> is used for error reporting. If it is set to zero (as,
+	// e.g., done by the built-in functions decode_base64() and
+	// encode_base64()), encoding-errors will go to Reporter instead of
+	// Weird. Usage errors go to Reporter in any case. Empty alphabet
+	// indicates the default base64 alphabet.
+	Base64Converter(Connection* conn, const string& alphabet = "");
 	~Base64Converter();
 
 	// A note on Decode():
@@ -42,8 +44,8 @@ public:
 	void IllegalEncoding(const char* msg)
 		{
 		// strncpy(error_msg, msg, sizeof(error_msg));
-		if ( analyzer )
-			analyzer->Weird("base64_illegal_encoding", msg);
+		if ( conn )
+			conn->Weird("base64_illegal_encoding", msg);
 		else
 			reporter->Error("%s", msg);
 		}
@@ -63,11 +65,11 @@ protected:
 	int base64_after_padding;
 	int* base64_table;
 	int errored;	// if true, we encountered an error - skip further processing
-	analyzer::Analyzer* analyzer;
+	Connection* conn;
 
 };
 
-BroString* decode_base64(const BroString* s, const BroString* a = 0);
-BroString* encode_base64(const BroString* s, const BroString* a = 0);
+BroString* decode_base64(const BroString* s, const BroString* a = 0, Connection* conn = 0);
+BroString* encode_base64(const BroString* s, const BroString* a = 0, Connection* conn = 0);
 
 #endif /* base64_h */

src/analyzer/protocol/ftp/FTP.cc

@@ -206,7 +206,7 @@ void FTP_ADAT_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 			{
 			line = skip_whitespace(line + cmd_len, end_of_line);
 			StringVal encoded(end_of_line - line, line);
-			decoded_adat = decode_base64(encoded.AsString());
+			decoded_adat = decode_base64(encoded.AsString(), 0, Conn());
 
 			if ( first_token )
 				{
@@ -273,7 +273,7 @@ void FTP_ADAT_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 				{
 				line += 5;
 				StringVal encoded(end_of_line - line, line);
-				decoded_adat = decode_base64(encoded.AsString());
+				decoded_adat = decode_base64(encoded.AsString(), 0, Conn());
 				}
 
 			break;

src/analyzer/protocol/mime/MIME.cc

@@ -1134,7 +1134,15 @@ void MIME_Entity::StartDecodeBase64()
 		delete base64_decoder;
 		}
 
-	base64_decoder = new Base64Converter(message->GetAnalyzer());
+	analyzer::Analyzer* analyzer = message->GetAnalyzer();
+
+	if ( ! analyzer )
+		{
+		reporter->InternalWarning("no analyzer associated with MIME message");
+		return;
+		}
+
+	base64_decoder = new Base64Converter(analyzer->Conn());
 	}
 
 void MIME_Entity::FinishDecodeBase64()

src/analyzer/protocol/pop3/POP3.cc

@@ -137,7 +137,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line)
 		++authLines;
 
 		BroString encoded(line);
-		BroString* decoded = decode_base64(&encoded);
+		BroString* decoded = decode_base64(&encoded, 0, Conn());
 
 		if ( ! decoded )
 			{

src/bro.bif

@@ -2723,14 +2723,18 @@ function hexstr_to_bytestring%(hexstr: string%): string
 
 ## Encodes a Base64-encoded string.
 ##
-## s: The string to encode
+## s: The string to encode.
+##
+## a: An optional custom alphabet. The empty string indicates the default alphabet.
+##    If given, the length of *a* must be 64. For example, a custom alphabet could be
+##    ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``.
 ##
 ## Returns: The encoded version of *s*.
 ##
-## .. bro:see:: encode_base64_custom decode_base64
-function encode_base64%(s: string%): string
+## .. bro:see:: decode_base64 decode_base64_conn
+function encode_base64%(s: string, a: string &default=""%): string
 	%{
-	BroString* t = encode_base64(s->AsString());
+	BroString* t = encode_base64(s->AsString(), a->AsString());
 	if ( t )
 		return new StringVal(t);
 	else
@@ -2740,18 +2744,19 @@ function encode_base64%(s: string%): string
 		}
 	%}
 
+
 ## Encodes a Base64-encoded string with a custom alphabet.
 ##
-## s: The string to encode
+## s: The string to encode.
 ##
-## a: The custom alphabet. The empty string indicates the default alphabet. The
-##    length of *a* must be 64. For example, a custom alphabet could be
+## a: An optional custom alphabet. The empty string indicates the default alphabet.
+##    If given, the length of *a* must be 64. For example, a custom alphabet could be
 ##    ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``.
 ##
 ## Returns: The encoded version of *s*.
 ##
-## .. bro:see:: encode_base64 decode_base64_custom
-function encode_base64_custom%(s: string, a: string%): string
+## .. bro:see:: encode_base64 decode_base64 decode_base64_conn
+function encode_base64_custom%(s: string, a: string%): string &deprecated
 	%{
 	BroString* t = encode_base64(s->AsString(), a->AsString());
 	if ( t )
@@ -2767,12 +2772,50 @@ function encode_base64_custom%(s: string, a: string%): string
 ##
 ## s: The Base64-encoded string.
 ##
+## a: An optional custom alphabet. The empty string indicates the default alphabet.
+##    If given, the length of *a* must be 64. For example, a custom alphabet could be
+##    ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``.
+##
 ## Returns: The decoded version of *s*.
 ##
-## .. bro:see:: decode_base64_custom encode_base64
-function decode_base64%(s: string%): string
+## .. bro:see:: decode_base64_intern encode_base64
+function decode_base64%(s: string, a: string &default=""%): string
 	%{
-	BroString* t = decode_base64(s->AsString());
+	BroString* t = decode_base64(s->AsString(), a->AsString());
+	if ( t )
+		return new StringVal(t);
+	else
+		{
+		reporter->Error("error in decoding string %s", s->CheckString());
+		return new StringVal("");
+		}
+	%}
+
+## Decodes a Base64-encoded string that was derived from processing a connection.
+## If an error is encountered decoding the string, that will be logged to
+## ``weird.log`` with the associated connection,
+##
+## cid: The identifier of the connection that the encoding originates from.
+##
+## s: The Base64-encoded string.
+##
+## a: An optional custom alphabet. The empty string indicates the default alphabet.
+## If given, the length of *a* must be 64. For example, a custom alphabet could be
+## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``.
+##
+## Returns: The decoded version of *s*.
+##
+## .. bro:see:: decode_base64 encode_base64_intern
+function decode_base64_conn%(cid: conn_id, s: string, a: string &default=""%): string
+	%{
+	Connection* conn = sessions->FindConnection(cid);
+	if ( ! conn )
+		{
+		builtin_error("connection ID not a known connection", cid);
+		return new StringVal("");
+		}
+
+	BroString* t = decode_base64(s->AsString(), a->AsString(), conn);
 	if ( t )
 		return new StringVal(t);
 	else
@@ -2792,8 +2835,8 @@ function decode_base64%(s: string%): string
 ##
 ## Returns: The decoded version of *s*.
 ##
-## .. bro:see:: decode_base64 encode_base64_custom
-function decode_base64_custom%(s: string, a: string%): string
+## .. bro:see:: decode_base64 decode_base64_conn encode_base64
+function decode_base64_custom%(s: string, a: string%): string &deprecated
 	%{
 	BroString* t = decode_base64(s->AsString(), a->AsString());
 	if ( t )

testing/btest/Baseline/bifs.decode_base64/out

@@ -4,3 +4,11 @@ bro
 bro
 bro
 bro
+bro
+bro
+bro
+bro
+bro
+bro
+bro
+bro

testing/btest/Baseline/bifs.decode_base64_conn/weird.log

@@ -0,0 +1,12 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	weird
+#open	2015-08-31-03-09-20
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	name	addl	notice	peer
+#types	time	string	addr	port	addr	port	string	string	bool	string
+1254722767.875996	CjhGID4nQcgTWjvg4c	10.10.1.4	1470	74.53.140.153	25	base64_illegal_encoding	incomplete base64 group, padding with 12 bits of 0	F	bro
+1437831787.861602	CPbrpk1qSsw6ESzHV4	192.168.133.100	49648	192.168.133.102	25	base64_illegal_encoding	incomplete base64 group, padding with 12 bits of 0	F	bro
+1437831799.610433	C7XEbhP654jzLoe3a	192.168.133.100	49655	17.167.150.73	443	base64_illegal_encoding	incomplete base64 group, padding with 12 bits of 0	F	bro
+#close	2015-08-31-03-09-20

testing/btest/Baseline/bifs.encode_base64/out

@@ -1,5 +1,9 @@
 YnJv
 YnJv
+YnJv
+}n-v
+YnJv
+YnJv
 }n-v
 cGFkZGluZw==
 cGFkZGluZzE=

testing/btest/bifs/decode_base64.bro

@@ -6,9 +6,17 @@ global default_alphabet: string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrs
 global my_alphabet: string = "!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?";
 
 print decode_base64("YnJv");
+print decode_base64("YnJv", default_alphabet);
+print decode_base64("YnJv", ""); # should use default alpabet
+print decode_base64("}n-v", my_alphabet);
 print decode_base64_custom("YnJv", default_alphabet);
+print decode_base64_custom("YnJv", ""); # should use default alpabet
 print decode_base64_custom("}n-v", my_alphabet);
 
 print decode_base64("YnJv");
+print decode_base64("YnJv", default_alphabet);
+print decode_base64("YnJv", ""); # should use default alpabet
+print decode_base64("}n-v", my_alphabet);
 print decode_base64_custom("YnJv", default_alphabet);
+print decode_base64_custom("YnJv", ""); # should use default alpabet
 print decode_base64_custom("}n-v", my_alphabet);

testing/btest/bifs/decode_base64_conn.bro

@@ -0,0 +1,8 @@
+# @TEST-EXEC: bro -r $TRACES/smtp.trace %INPUT >out
+# @TEST-EXEC: btest-diff weird.log
+
+event connection_established(c: connection)
+	{
+	# This should be logged into weird.
+	print decode_base64_conn(c$id, "kaputt");
+	}

testing/btest/bifs/encode_base64.bro

@@ -6,7 +6,12 @@ global default_alphabet: string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrs
 global my_alphabet: string = "!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?";
 
 print encode_base64("bro");
+print encode_base64("bro", default_alphabet);
+print encode_base64("bro", ""); # should use default alpabet
+print encode_base64("bro", my_alphabet);
+
 print encode_base64_custom("bro", default_alphabet);
+print encode_base64_custom("bro", ""); # should use default alpabet
 print encode_base64_custom("bro", my_alphabet);
 
 print encode_base64("padding");