Hello,
Hello,
I have several locations with a hardware VPN device and I've set them up to log to a central logging server.
Some of the managers of these facilities would like to have a report showing who's using the VPN and when.
I've written a script that does that. It works, but as I'm pretty new at Perl (this is only my second real life application) I'm sure it could be improved (significantly).
Would some kind soul like to take a look at this script, make suggestions or otherwise completely destroy my ego? :)
Ok, you asked for it.
Thanks in advance to anyone who takes this up...
The script can be found at:
http://www.zeffert.com/vpn_report
And some fake (sanitized) data to run it against (you'll have to make changes to the script accordingly):
http://www.zeffert.com/fake_data (60k text file, some gunk has been pre-filtered)
Thank you very much for providing a working program and data to test it with.
#!/usr/bin/perl -w # # FIXME: change the tempfile to something sane (randomly generated) # FIXME: delete the tempfile when we're done
If you really need a temp file you should use the File::Temp module.
perldoc File::Temp
# use warnings;
You don't need to use both the '-w' switch and the 'warnings' pragma, remove the '-w' switch.
use strict; my $inputfile = '/var/log/messages'; my $tmp_file = '/tmp/vpntmp'; my $month = `date +%b`; my $day = `date +%-d`; #my $day =140; my $year = `date +%Y`;
There is no need to call an external program to get the date as perl provides functions for this:
my ( $month, $day, $year ) = localtime =~ /(\w{3})\s+(\d+)\s+\S+\s+(\d{4})/;my @pppd_pids; my $pppd_pid_index=0; chop ($month, $day, $year);
You should use chomp() instead of chop() (which you rarely need) and you won't need either if you use the code above.
open(FILE,$inputfile);
You should *ALWAYS* verify that the file opened correctly.
# generate temp file open (TEMPFILE,">$tmp_file");
You don't really need a temp file as one days data should fit easily into memory.
print "VPN usage report for $day $month $year\n\n";
# populate temp file and get pids
while (<FILE>) {
#because of the split()s I need to convert all
#multiple spaces to single spaces.
tr/ //s;
If you used split() correctly then you wouldn't need to do this.
# populate temp file, if it's a pppd line from the 550
if ((/$month $day/ && /$year/ && /SG550/ && /pppd/) || (/$month $day/ && /$year/
&& /SG550/ && /pptpd/)) {
You can simplify that a bit:
if ( /$month $day/ && /$year/ && /SG550/ && /ppt?pd/ ) { print TEMPFILE $_;
}
# if it's a connection note the pid for later processing
#if (/$month $day/ && /$year/ && /pppd/ && /peer authentication
succeeded/){
if (/$month $day/ && /$year/ && /pppd/ && /2.3.8 started by/){
You are including the pppd version number in the regular expression but that won't work if you upgrade pppd. Also, the period is special in regular expressions, it matches any character.
#my $converted_string = $_;
#$converted_string =~ tr/ //s;
my @fields = split(/ /,$_);
You are using split() to split on a single space character. If you used the default split with no arguments it will work better:
my @fields = split;
my $pppd_pid = get_pid($fields[5]);
$pppd_pids[$pppd_pid_index]=$pppd_pid;
$pppd_pid_index++;
Instead of keeping track of the array index just use push():
push @pppd_pids, $pppd_pid;
} } close ( TEMPFILE ); close ( FILE );
Instead of critiquing the rest of your code I'll show you how I would code it:
#!/usr/bin/perl use warnings; use strict;
my $inputfile = '/var/log/messages';
my ( $month, $day, $year ) = localtime =~ /(\w{3})\s+(\d+)\s+\S+\s+(\d+)/;open FILE, '<', $inputfile or die "Cannot open $inputfile: $!";
print "VPN usage report for $day $month $year\n\n";
my ( $remote, %pids );
while ( <FILE> ) {
next unless /^$month +$day +([\d:]+).+?SG550 +ppt?pd\[(\d+)].+?\($year/o;
my ( $time, $pid ) = ( $1, $2 ); if ( /CTRL: Client ([\d.]+) control connection started/ ) {
$remote = $1;
}
elsif ( /pppd [\d.]+ started by/ ) {
# Instead of guessing the PID difference between pppd and pptpd
# this assumes that the pppd 'started by' line comes right after
# the pptpd 'connection started' line. @{ $pids{ $pid } }{ qw/start remote/ } = ( $time, $remote );
}
elsif ( /peer authentication succeeded for (.+)/ ) {
$pids{ $pid }{ username } = $1;
}
elsif ( /remote IP address ([\d.]+)/ ) {
$pids{ $pid }{ assigned } = $1;
}
elsif ( /Sent (\d+) bytes, received (\d+) bytes/ ) {
@{ $pids{ $pid } }{ qw/sent rcvd/ } = ( $1, $2 );
}
elsif ( /Connection terminated/ ) {
$pids{ $pid }{ end } = $time;
}
elsif ( /Connect time (.+)/ ) {
$pids{ $pid }{ connect } = $1;
}
}
close FILE;my $counter;
for my $pid ( sort { $a <=> $b } keys %pids ) {
# set empty fields to the string 'n/a'
$_ ||= 'n/a' for @{ $pids{ $pid } }{ qw/ username remote assigned sent rcvd start end connect / };
$counter++;
print <<STATUS;Connection $counter
PID: $pid
Username: $pids{$pid}{username}
Remote IP Address: $pids{$pid}{remote}
Assigned IP Address: $pids{$pid}{assigned}
Bytes Sent: $pids{$pid}{sent}
Bytes Received: $pids{$pid}{rcvd}
Start Time: $pids{$pid}{start}
End Time: $pids{$pid}{end}
Duration: $pids{$pid}{connect}STATUS
}print <<'MESG';
Please note that any "n/a" entries in the bytes or time columns may mean that the connection is still up, or that the connection dropped with an error (such as if the vpn server is restarted.) If there is a "n/a" in the assigned address field then the connection probably failed. If you can ping the assigned address then the connection is still up.
Also note that an empty file means there have been no connections today, you may wish to run this report by hand and specify another date, or you may wish to view the logs directly. MESG
__END__
John -- use Perl; program fulfillment
-- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>
