KISS with a bit of luck

## connect via ssh
$Expect::Log_Stdout = 0;
$ssh = Expect->new("ssh $login\@localhost");
#$ssh->debug(1);
$ssh->expect(2,qq{$login\@localhost's password:});
$ssh->send($passwd . "\n");
unless(scalar $ssh->expect(2,'$') || $ssh->expect(2,"Changing password for")) {#$ssh->expect(2,"Old password:")) {
return 0; #unable to login
}
else {
return $ssh; # logged in
}

This choice also allowed one more feature in the panel. Changing passwords.
Typical password forms want an old password and the new password repeated twice.

If we have the old password we can always authenticate the user with ssh and issue the passwd command changing the password using the existing *nix tool.

sub change_pass {
my ($ip,$login,$old_pass,$new_pass,$repeat_pass) = @_;

return 0 if $new_pass ne $repeat_pass;

my $ssh = login($login,$old_pass,$ip);

unless(check_shell($login)) {
$ssh->send("passwd\n");
}

$ssh->send("$old_pass\n") if (scalar $ssh->expect(2,"Old password:"));
$ssh->send("$new_pass\n") if (scalar $ssh->expect(2,"New password:"));
$ssh->send("$repeat_pass\n") if (scalar $ssh->expect(2,"Re-enter new password:"));



if($ssh->expect(2,"Password changed.")) {
$ssh->close;
return 1;
}

return -1;
}

KISS by accident

The most surprising thing is that this script was actually alive and in use.
We never got a 'bug report' against it from any of the users or the admin himself. Though now I see that he did tweak some stuff here & there (like translating error messages from English to Polish, or changing vhosts when he switched domains).

Here are some stats grabbed from the database this script used:

• registered 228 config updates (ka_config.updates) this denotes the number of times the kewlup.pl script runs that resulted in vhost changes.
• ka_domains has 139 entries from 105 users.
• 907 entries in ka_users (unique username + ip login attempts)
• 417 unique usernames trying to log in
• 524 failed login attempts

There are only 114 users on the server itself, based on the number of unique users and some usernames I saw the site was subject to brute force attacks. The one that stands out the most is for the 'java' account:

|  59 | X.Y.Z.N     | java                           |        13 |  1153683487|

There's of course no account on the server with that username.

The ugly

Now the ugly parts. The most prominent offender is the way we decided to generate the session key.

sub genkey {
my $key ='';
my @char = ('a' .. 'z','A' .. 'Z', 0 .. 9);
$key  .= $char[rand@char] while length $key != 32;
return $key;
}

Please never do this. Nowadays I would use an UUID generator making sure it's cryptographically secure before touching it.

Though genkey is awful - the way it's used is even uglier.

sub create_cookie {
 # ...
$sth = $dbh->prepare("SELECT id FROM ka_keys WHERE cookie = ?");

##
# generate until unique key
while (1) {
$key = genkey();
$sth->execute($key);
last unless $sth->rows;
}
 # ...
}

We were hitting the DB to check on each call to make sure that the token we generated is unique in the DB. With a proper UUID you should never do that. Just generate the token and handle a potential error case on insert/update with lot's of logging.

Fortunately even with a predicable session key the only thing a hijacked session could do was to alter/add/remove a vhost or view the current quota for an account. That's still far more than I'm comfortable with now but at least there's no obvious way (to me) that an attacker could gain shell access this way.

Second thing I'm not proud of is picking MySQL. This one is a bit personal as MySQL is used a lot by a lot of people but I just don't trust it any data any more. When this code was written, I knew nothing about databases except trivial basics. After 7 years with Oracle & around 3 years with PostgreSQL I'm not touching MySQL with a 10 foot pole.

The last bit that made me uneasy is actually the shell outs to the commands.

Let me list them here:

        # ....
if ($_login =~ /^([A-Za-z][-_A-Za-z0-9]{0,29})$/) {
$login = $1;
        # ....

my $uinfo = `finger -mp $login`;
# ....
my $space_used = `sudo du -sm ~$login | awk '{print \$1}'`;
# ....

Yes, $login was untained before usage but I'm still uneasy about calling any shell command with string interpolation.

Final thoughts