From 8f3ded5e2d16d5fd2609a04aa05d1a9cd2664fb4 Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Tue, 4 Aug 2015 15:46:24 +0200 Subject: [PATCH 1/2] Refactoring of Base64 functions. Base64Converter now uses a connection directly, instead of an analyzer redirecting to the underlying connection for reporting to Weird. The new built-in functions en-/decode_base64_intern make use of this to send encoding-errors to Weird instead of Reporter. According to the documentation, using the empty string as alphabet in the built-in functions, will use the default alphabet. Therefore the built-in functions can now use default arguments and en-/decode_base64_custom is deprecated. The tests have been updated accordingly. --- src/Base64.cc | 16 ++-- src/Base64.h | 19 ++-- src/analyzer/protocol/mime/MIME.cc | 10 +- src/bro.bif | 94 +++++++++++++++++-- testing/btest/Baseline/bifs.decode_base64/out | 8 ++ testing/btest/Baseline/bifs.encode_base64/out | 4 + testing/btest/bifs/decode_base64.bro | 8 ++ testing/btest/bifs/encode_base64.bro | 5 + 8 files changed, 136 insertions(+), 28 deletions(-) diff --git a/src/Base64.cc b/src/Base64.cc index 2ff858cad5..3644740c7e 100644 --- a/src/Base64.cc +++ b/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); diff --git a/src/Base64.h b/src/Base64.h index d7e4384ac5..7214ba6f29 100644 --- a/src/Base64.h +++ b/src/Base64.h @@ -8,15 +8,16 @@ #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: - // 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(). + // is used for error reporting. If it is set to zero, 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(analyzer::Analyzer* analyzer, const string& alphabet = ""); + Base64Converter(Connection* conn, const string& alphabet = ""); ~Base64Converter(); // A note on Decode(): @@ -42,8 +43,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 +64,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 */ diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index be10681266..f40e931299 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -1116,13 +1116,21 @@ void MIME_Entity::DecodeBase64(int len, const char* data) void MIME_Entity::StartDecodeBase64() { + analyzer::Analyzer* analyzer = message->GetAnalyzer(); + Connection* conn = 0; + if ( base64_decoder ) { reporter->InternalWarning("previous MIME Base64 decoder not released"); delete base64_decoder; } - base64_decoder = new Base64Converter(message->GetAnalyzer()); + if( analyzer ) + conn = analyzer->Conn(); + else + reporter->InternalWarning("no analyzer associated with MIME message"); + + base64_decoder = new Base64Converter(conn); } void MIME_Entity::FinishDecodeBase64() diff --git a/src/bro.bif b/src/bro.bif index 629abe7735..5a3a3ba759 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -2723,14 +2723,51 @@ function hexstr_to_bytestring%(hexstr: string%): string ## Encodes a Base64-encoded string. ## -## 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 +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. ## ## Returns: The encoded version of *s*. ## -## .. bro:see:: encode_base64_custom decode_base64 -function encode_base64%(s: string%): string +## .. bro:see:: encode_base64_intern decode_base64 +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 + { + reporter->Error("error in encoding string %s", s->CheckString()); + return new StringVal(""); + } + %} + +## Encodes a Base64-encoded string. +## +## cid: The connection identifier, identifiying the connection which is used to +## handle encoding-errors (errors will go to Weird). +## +## 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 +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. +## +## Returns: The encoded version of *s*. +## +## .. bro:see:: encode_base64 decode_base64_intern +function encode_base64_intern%(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 = encode_base64(s->AsString(), a->AsString(), conn); if ( t ) return new StringVal(t); else @@ -2742,7 +2779,7 @@ 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 @@ -2751,7 +2788,7 @@ function encode_base64%(s: string%): string ## Returns: The encoded version of *s*. ## ## .. bro:see:: encode_base64 decode_base64_custom -function encode_base64_custom%(s: string, a: string%): string +function encode_base64_custom%(s: string, a: string%): string &deprecated %{ BroString* t = encode_base64(s->AsString(), a->AsString()); if ( t ) @@ -2767,12 +2804,49 @@ function encode_base64_custom%(s: string, a: string%): string ## ## s: The Base64-encoded string. ## +## 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 +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~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. +## +## cid: The connection identifier, identifiying the connection which is used to +## handle encoding-errors (errors will go to Weird). +## +## s: The Base64-encoded string. +## +## 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 +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. +## +## Returns: The decoded version of *s*. +## +## .. bro:see:: decode_base64 encode_base64_intern +function decode_base64_intern%(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 @@ -2793,7 +2867,7 @@ 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 +function decode_base64_custom%(s: string, a: string%): string &deprecated %{ BroString* t = decode_base64(s->AsString(), a->AsString()); if ( t ) diff --git a/testing/btest/Baseline/bifs.decode_base64/out b/testing/btest/Baseline/bifs.decode_base64/out index af0d32fbb8..aa265d2148 100644 --- a/testing/btest/Baseline/bifs.decode_base64/out +++ b/testing/btest/Baseline/bifs.decode_base64/out @@ -4,3 +4,11 @@ bro bro bro bro +bro +bro +bro +bro +bro +bro +bro +bro diff --git a/testing/btest/Baseline/bifs.encode_base64/out b/testing/btest/Baseline/bifs.encode_base64/out index 84c2c98264..3008115853 100644 --- a/testing/btest/Baseline/bifs.encode_base64/out +++ b/testing/btest/Baseline/bifs.encode_base64/out @@ -1,5 +1,9 @@ YnJv YnJv +YnJv +}n-v +YnJv +YnJv }n-v cGFkZGluZw== cGFkZGluZzE= diff --git a/testing/btest/bifs/decode_base64.bro b/testing/btest/bifs/decode_base64.bro index d4cbd2f37d..2d552a2523 100644 --- a/testing/btest/bifs/decode_base64.bro +++ b/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); diff --git a/testing/btest/bifs/encode_base64.bro b/testing/btest/bifs/encode_base64.bro index a351392bb5..bbad715ecc 100644 --- a/testing/btest/bifs/encode_base64.bro +++ b/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"); From 55dc982a332130dbc6dbb7e0527eeb272b3d4875 Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Wed, 5 Aug 2015 11:33:57 +0200 Subject: [PATCH 2/2] Update calls of Base64 functions. Base64 encoding-errors during authentication in POP3 analyzer, authentication in FTP analyzer (using GSI) and basic authentication on HTTP will be logged to Weird. --- scripts/base/protocols/http/main.bro | 2 +- src/analyzer/protocol/ftp/FTP.cc | 4 ++-- src/analyzer/protocol/pop3/POP3.cc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/base/protocols/http/main.bro b/scripts/base/protocols/http/main.bro index 916723ebcb..4d9969fa7d 100644 --- a/scripts/base/protocols/http/main.bro +++ b/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_intern(c$id, sub(value, /[bB][aA][sS][iI][cC][[:blank:]]/, "")); local up = split_string(userpass, /:/); if ( |up| >= 2 ) { diff --git a/src/analyzer/protocol/ftp/FTP.cc b/src/analyzer/protocol/ftp/FTP.cc index fd38ee8f29..402532eff1 100644 --- a/src/analyzer/protocol/ftp/FTP.cc +++ b/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, this->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, this->Conn()); } break; diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index 05ff3c317d..69740eb71d 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/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, this->Conn()); if ( ! decoded ) {