in reply to SSH Failed Attempt Monitor

Just a few random comments from another set of eyes:

#!/usr/bin/perl
# This script watches the /var/log/secure

[download]

So it runs as root, but you're not using taint mode. No bugs at first sight, but you never know.

my @safe = qw/ 192.168.0.1 /;
[Some code]
my @rejects;

[download]

Why not define @rejects just under my @safe since it serves a similar purpose?

next unless $line =~ /Failed password/ig;

If I were you, I'd tight that regex a little more to avoid false positives, althought that has the drawback that maybe in a future version of the sshd daemon, the message about failed login attempts changes and suddenly you don't seem to be attacked anymore ;^). Also, why /ig?

if ( !defined( $db{$ip} ) ) {
    push ( @{ $db{$ip} }, $epoch );
}
else {
    push ( @{ $db{$ip} }, $epoch );

[download]

The push could be moved outside the if. Plus, is there more information stored in $db{$ip}? You seem to store only the IP, so there's no need to have a hash and use push and undef below. Just $db{$ip} = $epoch.

my @safe = @_;
foreach (@safe) {
    return 1 if $ip eq $_;
}
return 0;

[download]

If you like compact code, try: return !!grep { $ip eq $_ } @_ ## tested

sub isRejected {

Either rename this to rejected, or safe to isSafe.

foreach (@rejects) {

Same grep as before.

--
David Serrano

Replies are listed 'Best First'.
Re^2: SSH Failed Attempt Monitor
by Anonymous Monk on Jul 06, 2006 at 21:06 UTC
    >So it runs as root, but you're not using taint mode. No bugs at first sight, but you never know.

    True, however it should run as is in taint mode. The only input per say is the entry in the log file. The IP address is pulled out with a regex and $1 is reassigned to $ip.. Therefor when system() is called, its taint free and shouldn't ever be an issue. By all means, turn it on if its a concern.

    >If I were you, I'd tight that regex a little more to avoid false positives, althought that has the drawback that maybe in a future version of the sshd daemon, the message about failed login attempts changes and suddenly you don't seem to be attacked anymore ;^). Also, why /ig?

    /g -- the g isn't needed, just a habbit
    /i -- same reason as you mentioned above. a little bit of a loose regular expression, but hopefully will work if the log format changes a little bit.
    @safe and @reject are two entities, kept apart for readability purposes, you are correct that other ways exist, perhaps even better ways.

    the $db{$ip} stores more than IP.. it also stores the count, and epoch times of the event. The count being the number of array elements, and the times are the element stores as an epoch time:

           if ( $db{$ip}[ $#{ $db{$ip} } ] - $db{$ip}[ $#{ $db{$ip} } - 1 
+] > $thresHold ) {

    [download]


    Here I say .. if the last array item (which holds the highest epoch time) minus the previous attempts epoch time is greater than the threshold (ie. 10 minute), then clear the current attack and treat as a new attack. I'm basically just clearing the entire record of previous attack. The else of that is the handler for ongoing attack -- increase count or reject the attacker.


    I like your grep suggestion as well, although I opted foreach because its slightly more readable. That's the only real reason for that one.

    And yes, the subroutine names are inconsistent, good point. That will be fixed.

    Matt
[reply]
[d/l]

In Section Code Catacombs