Forum teuk.org

Snape's Potion Class: Ditching MariaDB PASSWORD(), Retiring the Last getNickInfo Ghost, and Cleaning Up the Noise

in Mediabot · started by TeuK · 2w ago

TeuK · 2w ago

Snape didn’t teach potions by calling a house elf to do the brewing for him. Similarly, asking MariaDB to hash your passwords for you when you can perfectly well do it in Perl — with the same algorithm, without the round-trip — is an unnecessary dependency. Class is in session.


The Problem With PASSWORD()

MariaDB’s PASSWORD() function is a double SHA1 — '*' . uc(sha1_hex(sha1($plain))) — built into the server. It works, it’s consistent, and it’s what every Mediabot password has been stored as since the beginning.

The problem is that it ties password logic to the database engine. Every password check and every password write required a round-trip to MariaDB, or at minimum the knowledge that MariaDB was on the other end computing it. It’s also deprecated in newer MariaDB versions, which means it’s a ticking portability problem.

Auth.pm had already been migrated in a previous session: verify_credentials() calls _password_matches(), which computes the double SHA1 in Perl using Digest::SHA. But five other code paths were still calling PASSWORD(?) directly — either in SQL WHERE clauses or in UPDATE SET password=PASSWORD(?).

What Was Still Using It

Function File Usage
checkAuth() LoginCommands.pm WHERE password = PASSWORD(?)
checkAuthByUser() LoginCommands.pm WHERE password = PASSWORD(?)
userLogin_ctx() LoginCommands.pm SELECT PASSWORD(?) to compute hash
userPass_ctx() LoginCommands.pm UPDATE SET password=PASSWORD(?)
userAdd() Helpers.pm INSERT password=PASSWORD(?)

The Fix

A new function make_password_hash($plain) has been added to Helpers.pm and exported:

sub make_password_hash {
    my ($plain) = @_;
    return undef unless defined $plain && length $plain;
    return '*' . uc(sha1_hex(sha1($plain)));
}

This produces byte-for-byte identical output to PASSWORD(). All existing stored hashes remain valid — no migration, no password resets. The function is called before any SQL statement, so the query just binds a pre-computed string:

# Before
"UPDATE USER SET password=PASSWORD(?) WHERE id_user=?"
$sth->execute($plain, $id);

# After
my $hashed = make_password_hash($plain);
"UPDATE USER SET password=? WHERE id_user=?"
$sth->execute($hashed, $id);

All five call sites have been updated. Digest::SHA is imported in Helpers.pm and available to everything that already does use Mediabot::Helpers. LoginCommands.pm no longer needs its own SHA import.


The Last getNickInfo Ghost in the Codebase

getNickInfo() is a legacy function that returns eight positional variables from a database lookup by IRC hostmask. It was the standard auth pattern before the Context/User object system. It has been progressively replaced throughout the codebase over several sessions.

One occurrence remained: the on_message_INVITE handler in mediabot.pl.

# Before — eight variables, six unused, joinChannel commented out
my ($iMatchingUserId, $iMatchingUserLevel, $iMatchingUserLevelDesc,
    $iMatchingUserAuth, $sMatchingUserHandle, $sMatchingUserPasswd,
    $sMatchingUserInfo1, $sMatchingUserInfo2) = $mediabot->getNickInfo($message);

if (defined($iMatchingUserId)) {
    if (defined($iMatchingUserAuth) && $iMatchingUserAuth) {
        $mediabot->{logger}->log(1, "... (authentified user)");
        #$mediabot->joinChannel($target_name);
    } else {
        # ... log
        #$mediabot->joinChannel($target_name);
    }
}

The actual auto-join logic has been commented out for a long time. The auth check was only there to produce a slightly different log message. The replacement is straightforward:

# After
my $inviter_user = $mediabot->get_user_from_message($message);
my $is_auth      = $inviter_user && $inviter_user->is_authenticated ? 1 : 0;
my $auth_label   = $is_auth ? 'authenticated' : 'not authenticated';
$mediabot->{logger}->log(1, "$invited_nick has been invited to join "
    . "$target_name by $inviter_nick ($auth_label)");
# Auto-join disabled — uncomment to re-enable:
# $mediabot->joinChannel($target_name) if $is_auth;

getNickInfo() is now called zero times anywhere outside of its own definition and the three getNickInfoWhois() calls in the WHOIS handlers. Those three require a dedicated get_user_from_hostmask() method (WHOIS provides ident@host without a nick) — that’s a separate session.


44 Prototypes Cleaned From Install Scripts

The Mediabot core modules were already cleaned of sub foo(@) prototypes in earlier sessions. The install and contrib scripts hadn’t been touched yet:

File Prototypes removed
install/conf_servers.pl 26
install/configure.pl 10
contrib/icecast2/getIcecastListeners.pl 4
contrib/icecast2/getIcecastTitle.pl 4

These prototypes (sub foo(@)) serve no purpose in modern Perl with use strict — they neither enforce types nor affect argument passing. They’re just noise that misleads anyone reading the code into thinking something special is happening.


Log Level Reclassification

Four log messages were incorrectly at level 0 (always visible, equivalent to print STDERR):

Helpers.pm — Two [IGNORED] messages emitted when a badword or responder lookup matches but the bot decides not to act (flood protection, channel exclusion, etc.). These are fine-grained debug traces, not operational alerts. Moved to level 1.

DBCommands.pm"Checking timers to set at startup" — informational startup trace. Moved to level 1.

LoginCommands.pm"Register attempt ignored (users already exist)" — expected behaviour in production, not an error. Moved to level 1.


Summary

Area Change
Helpers.pm + LoginCommands.pm PASSWORD()make_password_hash() in Perl, 5 call sites
mediabot.pl Last getNickInfo call removed from INVITE handler
install/ + contrib/ 44 (@) prototypes removed
Helpers.pm, DBCommands.pm, LoginCommands.pm log(0)log(1) reclassification

Pending (next session): getNickInfoWhois in WHOIS handlers requires a new get_user_from_hostmask($ident_at_host) method before those three call sites can be migrated.

You must be logged in to reply.