Anticipation of future needs and other musings from the crystal ball

[Limbic~Region]

On July 11th, 2002 my eyes were opened. I discovered PM and Perl that day. As I am sure a lot of people do, I have tried to become good at Perl by solving problems at work that needed solving.

One thing I needed to solve was a way of monitoring system health and reporting only what was relavent. Without discussing the merits of other programs that do the same thing, I needed it to be incredibly specific to my environment, I needed it to be fast, and I needed it to be expandable.

The last part is the one that intrigued me - was it possible to write a program that could grow and expand without modifying the code? The answer is yes if you standardize the "types" of things you are looking for. Now the question you may slam me for asking - is this the right approach? Here is a sample of some code to give you an idea of what I am talking about.

#!/usr/bin/perl -W
use strict;
use Config::IniFiles;
delete @ENV{qw(IFS PATH CDPATH ENV BASH_ENV)};
my $cfg = new Config::IniFiles( -file => "default.ini" );
&processcheck if $cfg->val('PROCESS','ENABLED');
&displayresults;
exit;
sub processcheck {
 my @ptable = `/usr/bin/ps -ef`;
 my @alert = $cfg->val('PROCESS','ALERT');
 my @message = $cfg->val('PROCESS','MESSAGE');
 my @process = $cfg->val('PROCESS','PROCESS');
 for ( my $i = 0; $i <= $#alert; $i++ ) {
  unless(grep /$process[$i]/ , @ptable) {
   $alerts{"$alert[$i]"} .= "\n$message[$i]";
  }
 }
}
sub displayresults {
 print "REPORT TAKEN AT " . localtime(time) . "\n";
 for my $alerttype ( sort keys %alerts ) {
  print "\n\U$alerttype\E ALERTS";
  print "$alerts{$alerttype}\n";
 }
}

#default.ini
[PROCESS]
ENABLED = 1
PROCESS = ldapd -p 389
PROCESS = sendmail: accepting
PROCESS = /xntpd
PROCESS = apache/bin/httpd
MESSAGE = LDAP is not running
MESSAGE = Sendmail daemon is not accepting connections
MESSAGE = NTP daemon is not running
MESSAGE = Apache web server is not running
ALERT = medium
ALERT = major
ALERT = minor
ALERT = minor

Again, most of the code is tailored for the environment I work in and I couldn't disclose it here if I wanted to. There is a way to set thresholds for disk space and the appropriate alert, ways to check if log files are being updated or are stagnant, etc.

My question is - is this a bad idea, are the security risks too great? The above section of code should work with Taint turned on for those of you who would like to use it. For me, it is a lot easier to go into the config file to adjust a parameter or add a new check then it is to go into the code - especially when what people want changes every day - I wanted the code to stay small and clean.

ok, fire away - my job is secure even if my XPs aren't.

[sauoq]

Using a config file in the manner that you are is a common way to achieve extensibility. Although I agree with the suggestion that you impose a more readable order on the lines, your basic approach is very reasonable.

As far as security goes, you should remember that not every script needs to be written like Fort Knox depends on it. It's a little difficult to explain what's appropriate and what isn't because it all depends so heavily on your particular situation and environment. In a lot of cases, file permissions are pretty much all the security that you need. Sometimes you need to allow users to run a script as a different user and need to take some more precautions. Sometimes you need to allow people who aren't users to run a script (such as a CGI script executed by a webserver) and that might require more or different precautions. It is pretty unlikely that this script would require any security other than properly set permissions.

Just as extensibility and security are important, so is robustness. One suggestion that I do have is to examine how you are checking for a process's existence and consider how such a check might fail. As an example of what I mean, if your process check was for /sshd and someone happened to be running vi /etc/sshd_config it might look like sshd was alive even when it wasn't.

-sauoq
"My two cents aren't worth a dime.";

[FoxtrotUniform]

This is a clever approach.

That said, why are you so concerned about extending your program's behaviour without modifying the code? (That's a question, not a criticism.) It seems to me that being able to extend a program without adding code is most useful when the program takes effort to re-package (for instance, the familiar compile-link step with C), or when the code itself is difficult to modify. Perl avoids the first issue -- just edit and go. The second will become a problem no matter how you're extending the program.

If you have a block that looks like:

&check_proc('ldap -p 389', 'LDAP is not running', 'medium');
&check_proc('sendmail: accepting',
            'sendmail is not accepting new connections',
            'major');
&check_proc('/xntpd', 'NTP daemon is not running', 'minor');
&check_proc('apache/bin/httpd',
            'Apache web server is not running',
            'minor');

it doesn't take much more effort to add another call to check_proc than it does to modify a config file, and if you ever want to do something different (like monitor system load), you're set: you won't have to add syntax to the config file for a different kind of check, just add another sub. Never mind that you've eliminated a goodly chunk of code (the config file parser) and another feature (reading config files), which won't exactly make your code more difficult to maintain.

The only disadvantages I can see are:

1. Depending on how your monitoring script is running, it may be more difficult to refresh the code than a config file.
2. It'll probably be more difficult for other people to change the code than the config file, especially if the code has appropriately strict permissions.

but these may not be of concern to you.

Keep the code loosely coupled, write solid unit tests, and code becomes easy to change. (At least, that's what the eXtreme Programming zealots would have you believe... and in my experience, they're on to something.)

Good luck, and let us know how it turns out. I wouldn't mind seeing this appear in the Code Catacombs.

--
F o x t r o t U n i f o r m
Found a typo in this node? /msg me
The hell with paco, vote for Erudil!

[Zaxo]

Looks good.

I can add a few suggestions. You can reduce the number of lines to search by refining the ps call. It appears that you are checking daemon status, so `ps x` to show only processes with no controlling tty will cut the search space a lot.

You could improve the process check loop like this:

    @regexen = @process;
    for (@regexen) {
        quotemeta;
        $_ = qr/$_/;
    }
    for ( my $i = 0; $i <= $#alert; $i++ ) {
        unless(grep /$regexen[$i]/ , @ptable) {
            $alerts{"$alert[$i]"} .= "\n$message[$i]";
        }
    }

The quotemeta in the regex will save some hard to diagnose errors. and precompiling will likely save time since they are used to scan all the process entries.

A third improvement would be to reformulate your search to stop scanning when a regex matches, and to not check that regex again afterwards. See merlyn's reply to Ovid's (OT) Interview questions -- your response?, question 1.

After Compline,
Zaxo

[Aristotle]

Yes, this is called "data driven programming" and is generally agreed on as a Good Thing. :-)

There is room for improvement though. You are using functions that take no parameters, a single time each. There's no need to put that code in functions.

As suggested, a refined ps call will help a lot. If you have a GNU ps available, try ps xo command.

A suggestion for your configuration needs: use Config::General, makes for much nicer files, something like:

Enabled 1

<Process ldap>
    Name "ldapd -p 389"
    Message "LDAP is not running"
    Level medium
</Process>

<Process mta>
    Name "sendmail: accepting"
    Message "Sendmail daemon is not accepting connections"
    Level critical
</Process>

<Process ntp>
    Name /xntpd
    Message "NTP daemon is not running"
    Level minor
</Process>

<Process httpd>
    Name apache/bin/httpd
    Message "Apache web server is not running"
    Level minor
</Process>

Overall it then looks like this:

#!/usr/bin/perl -w
use strict;
use Config::General;

delete @ENV{qw(IFS PATH CDPATH ENV BASH_ENV)};

my %config = Config::General->new(
    -ConfigFile => ".processcheckrc",
    -LowerCaseNames => 1,
)->getall;

exit unless $config{enabled};

my $ptable = `/usr/bin/ps xo command`;

my %alert;

while(my($name,$option) = each %{$config{process}}) {
    my $re = quotemeta $option->{name};
    push @{ $alert{ $option->{level} } }, $option->{message}
        unless $ptable =~ /$re/;
}

print "REPORT TAKEN AT " . localtime . "\n";

print map "$_\n", "\U$_\E ALERTS", @{%alert{$_}}
    for sort keys %alert;

Makeshifts last the longest.

[fglock]

I'd order it as below, but it is a matter of taste. I think it secure enough - it won't do anything the user himself can't do.

PROCESS = ...
MESSAGE = ...
ALERT = ...

PROCESS = ...
MESSAGE = ...
ALERT = ...

I got here at July 12th, 2002 :)

[Limbic~Region]

My actual config file looks a tad bit different, and I agree with you. I was just trying to give an example and I forgot to include what it would take to add a new check - that was my intention, so here it is:

PROCESS = /sshd
MESSAGE = The SSH Daemon has terminated
ALERT = major

In just a few key strokes, the program now performs one more check and no one even had to open up the code. Of course, I realize that if you want to define a new "type" of check that it has to be coded. One of the other side effects of doing this is having multiple configuration files. Instead of hard coding to "default.ini" - you could use getargs to specify an alternate - as in changing disk space threshold. Each admin could tailor the file to their liking.

[theorbtwo]

for ( my $i = 0; $i <= $#alert; $i++ ) {
  unless(grep /$process[$i]/ , @ptable) {
   $alerts{"$alert[$i]"­} .= "\n$message[$i]";
  }
}

One thing about this I'd change: C-style for loops are often fencepost errors waiting to happen. When they aren't, they're often easer ways to write it. For example, in this case:

for (0..@alert) {
  next if (grep /$process[$_]/, @ptable);
  $alerts{$alert[$_]} .= "\n$message[$_]";
}

While I was at it, I changed from $i to $_ for your index var, and from an unless block to a next if. Those are both more matters of taste then perlishness. Slightly more major: I removed a redundant pair of double-quotes, on "$alert$i". Doing that in this case does nothing but slow you down slightly. In some cases, though, it'll force stringifaction, which is rarely what you want. For example, "$obj"->foo() won't call foo on $obj. Instead, it'll try to call foo on somthing like "Class=HASH(0x089742)", and fail.

---

Confession: It does an Immortal Body good.

[fglock]

Note: scalar @a != $#a

scalar @a is the number of elements

$#a is the last element index (scalar @a - 1)

[theorbtwo]

Bah! I am many things, but a careful coder isn't one of them nearly as often as it should be. You are, of course, right, fglock.

---

Confession: It does an Immortal Body good.