Merge remote-tracking branch 'origin/topic/robin/bit-1641'

* origin/topic/robin/bit-1641:
  Fixing duplicate SSH authentication failure events.

I changed the test slightly; the output of uniq is not stable between
operating systems (on OS-X, it emits a space, on Linux it apparently
emits a tab). I removed the call to uniq - sort by itself is enough to
create a difference if there are duplicate entries.

Addresses BIT-1641
This commit is contained in:
Johanna Amann 2016-08-02 15:09:18 -07:00
commit bac1bd5bdf
4 changed files with 31 additions and 7 deletions

View file

@ -16,7 +16,7 @@ SSH_Analyzer::SSH_Analyzer(Connection* c)
{ {
interp = new binpac::SSH::SSH_Conn(this); interp = new binpac::SSH::SSH_Conn(this);
had_gap = false; had_gap = false;
auth_decision_made = false; auth_decision = AUTH_UNKNOWN;
skipped_banner = false; skipped_banner = false;
service_accept_size = 0; service_accept_size = 0;
userauth_failure_size = 0; userauth_failure_size = 0;
@ -60,7 +60,7 @@ void SSH_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
BifEvent::generate_ssh_encrypted_packet(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), BifEvent::generate_ssh_encrypted_packet(interp->bro_analyzer(), interp->bro_analyzer()->Conn(),
orig, len); orig, len);
if ( ! auth_decision_made ) if ( auth_decision != AUTH_SUCCESS )
ProcessEncrypted(len, orig); ProcessEncrypted(len, orig);
return; return;
@ -105,9 +105,10 @@ void SSH_Analyzer::ProcessEncrypted(int len, bool orig)
// -16. // -16.
if ( ! userauth_failure_size && (len + 16 == service_accept_size) ) if ( ! userauth_failure_size && (len + 16 == service_accept_size) )
{ {
auth_decision_made = true;
if ( ssh_auth_successful ) if ( ssh_auth_successful )
BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true); BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true);
auth_decision = AUTH_SUCCESS;
return; return;
} }
@ -131,17 +132,20 @@ void SSH_Analyzer::ProcessEncrypted(int len, bool orig)
// another packet of the same size. // another packet of the same size.
if ( len == userauth_failure_size ) if ( len == userauth_failure_size )
{ {
if ( ssh_auth_failed ) if ( ssh_auth_failed && auth_decision != AUTH_FAILURE )
BifEvent::generate_ssh_auth_failed(interp->bro_analyzer(), interp->bro_analyzer()->Conn()); BifEvent::generate_ssh_auth_failed(interp->bro_analyzer(), interp->bro_analyzer()->Conn());
auth_decision = AUTH_FAILURE;
return; return;
} }
// ...or a success packet. // ...or a success packet.
if ( len - service_accept_size == -16 ) if ( len - service_accept_size == -16 )
{ {
auth_decision_made = true;
if ( ssh_auth_successful ) if ( ssh_auth_successful )
BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), false); BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), false);
auth_decision = AUTH_SUCCESS;
return; return;
} }
} }

View file

@ -35,12 +35,14 @@ namespace analyzer {
bool had_gap; bool had_gap;
// Packet analysis stuff // Packet analysis stuff
bool auth_decision_made;
bool skipped_banner; bool skipped_banner;
int service_accept_size; int service_accept_size;
int userauth_failure_size; int userauth_failure_size;
enum AuthDecision {
AUTH_UNKNOWN, AUTH_FAILURE, AUTH_SUCCESS
} auth_decision;
}; };
} }

View file

@ -0,0 +1,11 @@
C0LAHyvtKSQHyJxIl
C37jN32gN3y3AZzyf6
C3eiCBGOLw3VtHfOj
C4J4Th3PJpwUYZZ6gc
CHhAvVGS1DHFjwGM9
CP5puj4I8PtEU4qzYg
CUM0KZ3MLUfNB0cl11
ClEkJM2Vm5giqnMf4h
CmES5u32sYpV7JYN
CtPZjS20MLrsMUOJi2
CwjjYJ2WqgTbAqiHl6

View file

@ -0,0 +1,7 @@
# @TEST-EXEC: bro -C -r $TRACES/ssh/sshguess.pcap %INPUT | sort >output
# @TEST-EXEC: btest-diff output
event ssh_auth_failed(c: connection)
{
print c$uid;
}