Merge pull request #916 from grahamc/argon2-nested
Rehash existing sha1 passwords with Argon2
This commit is contained in:
commit
1bb1ba6928
|
@ -238,6 +238,12 @@ sub check_password {
|
|||
$self->setPassword($password);
|
||||
}
|
||||
|
||||
return 1;
|
||||
} elsif ($authenticator->verify_password(sha1_hex($password), $self->password)) {
|
||||
# The user's database record has their old password as sha1, re-hashed as Argon2.
|
||||
# Store their password hashed only with Argon2.
|
||||
$self->setPassword($password);
|
||||
|
||||
return 1;
|
||||
} else {
|
||||
return 0;
|
||||
|
@ -252,4 +258,15 @@ sub setPassword {
|
|||
});
|
||||
}
|
||||
|
||||
sub setPasswordHash {
|
||||
my ($self, $passwordHash) = @_;;
|
||||
|
||||
if ($passwordHash =~ /^[a-f0-9]{40}$/) {
|
||||
# This is (probably) a sha1 password, re-hash it and we'll check for a hashed sha1 in Users.pm
|
||||
$self->setPassword($passwordHash);
|
||||
} else {
|
||||
$self->update({ password => $passwordHash });
|
||||
}
|
||||
}
|
||||
|
||||
1;
|
||||
|
|
|
@ -109,7 +109,10 @@ $db->txn_do(sub {
|
|||
if (defined $password && !(defined $passwordHash)) {
|
||||
$user->setPassword($password);
|
||||
}
|
||||
$user->update({ password => $passwordHash }) if defined $passwordHash;
|
||||
|
||||
if (defined $passwordHash) {
|
||||
$user->setPasswordHash($passwordHash);
|
||||
}
|
||||
}
|
||||
|
||||
$user->userroles->delete if $wipeRoles;
|
||||
|
|
|
@ -72,3 +72,12 @@ for (my $n = $schemaVersion; $n < $maxSchemaVersion; $n++) {
|
|||
};
|
||||
die "schema upgrade failed: $@\n" if $@;
|
||||
}
|
||||
|
||||
my @usersWithSha1s = $db->resultset('Users')->search(\['LENGTH(password) = 40 AND password ~ \'^[0-9a-f]{40}$\'']);
|
||||
if (scalar(@usersWithSha1s) > 0) {
|
||||
print STDERR "upgrading user passwords from sha1\n";
|
||||
for my $user (@usersWithSha1s) {
|
||||
print STDERR " * " . $user->username . "\n";
|
||||
$user->setPassword($user->password);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -39,4 +39,34 @@ ok($user->check_password("foobar"), "Checking the password, foobar, is right");
|
|||
isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The user has had their password rehashed.");
|
||||
ok($user->check_password("foobar"), "Checking the password, foobar, is still right");
|
||||
|
||||
# All sha1 passwords will be upgraded when `hydra-init` is run, by passing the sha1 through
|
||||
# Argon2. Verify a rehashed sha1 validates too. This removes very weak password hashes
|
||||
# from the database without requiring users to log in.
|
||||
subtest "Hashing their sha1 as Argon2 still lets them log in with their password" => sub {
|
||||
$user->setPassword("8843d7f92416211de9ebb963ff4ce28125932878"); # SHA1 of "foobar"
|
||||
my $hashedHashPassword = $user->password;
|
||||
isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The user has had their password's hash rehashed.");
|
||||
ok($user->check_password("foobar"), "Checking the password, foobar, is still right");
|
||||
isnt($user->password, $hashedHashPassword, "The user's hashed hash was replaced with just Argon2.");
|
||||
};
|
||||
|
||||
|
||||
subtest "Setting the user's passwordHash to a sha1 stores the password as a hashed sha1" => sub {
|
||||
$user->setPasswordHash("8843d7f92416211de9ebb963ff4ce28125932878");
|
||||
isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text.");
|
||||
|
||||
my $storedPassword = $user->password;
|
||||
ok($user->check_password("foobar"), "Their password validates");
|
||||
isnt($storedPassword, $user->password, "The password was upgraded.");
|
||||
};
|
||||
|
||||
subtest "Setting the user's passwordHash to an argon2 password stores the password as given" => sub {
|
||||
$user->setPasswordHash('$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ');
|
||||
isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text.");
|
||||
is($user->password, '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ', "The password was saved as-is.");
|
||||
|
||||
my $storedPassword = $user->password;
|
||||
ok($user->check_password("foobar"), "Their password validates");
|
||||
is($storedPassword, $user->password, "The password was not upgraded.");
|
||||
};
|
||||
done_testing;
|
||||
|
|
56
t/scripts/hydra-create-user.t
Normal file
56
t/scripts/hydra-create-user.t
Normal file
|
@ -0,0 +1,56 @@
|
|||
use feature 'unicode_strings';
|
||||
use strict;
|
||||
use Setup;
|
||||
|
||||
my %ctx = test_init();
|
||||
|
||||
require Hydra::Schema;
|
||||
require Hydra::Model::DB;
|
||||
|
||||
use Test2::V0;
|
||||
|
||||
my $db = Hydra::Model::DB->new;
|
||||
hydra_setup($db);
|
||||
|
||||
subtest "Handling password and password hash creation" => sub {
|
||||
subtest "Creating a user with a plain text password (insecure) stores the password securely" => sub {
|
||||
my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password", "foobar"));
|
||||
is($res, 0, "hydra-create-user should exit zero");
|
||||
|
||||
my $user = $db->resultset('Users')->find({ username => "plain-text-user" });
|
||||
isnt($user, undef, "The user exists");
|
||||
isnt($user->password, "foobar", "The password was not saved in plain text.");
|
||||
|
||||
my $storedPassword = $user->password;
|
||||
ok($user->check_password("foobar"), "Their password validates");
|
||||
is($storedPassword, $user->password, "The password was not upgraded.");
|
||||
};
|
||||
|
||||
subtest "Creating a user with a sha1 password (still insecure) stores the password as a hashed sha1" => sub {
|
||||
my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password-hash", "8843d7f92416211de9ebb963ff4ce28125932878"));
|
||||
is($res, 0, "hydra-create-user should exit zero");
|
||||
|
||||
my $user = $db->resultset('Users')->find({ username => "plain-text-user" });
|
||||
isnt($user, undef, "The user exists");
|
||||
isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text.");
|
||||
|
||||
my $storedPassword = $user->password;
|
||||
ok($user->check_password("foobar"), "Their password validates");
|
||||
isnt($storedPassword, $user->password, "The password was upgraded.");
|
||||
};
|
||||
|
||||
subtest "Creating a user with an argon2 password stores the password as given" => sub {
|
||||
my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password-hash", '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ'));
|
||||
is($res, 0, "hydra-create-user should exit zero");
|
||||
|
||||
my $user = $db->resultset('Users')->find({ username => "plain-text-user" });
|
||||
isnt($user, undef, "The user exists");
|
||||
is($user->password, '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ', "The password was saved as-is.");
|
||||
|
||||
my $storedPassword = $user->password;
|
||||
ok($user->check_password("foobar"), "Their password validates");
|
||||
is($storedPassword, $user->password, "The password was not upgraded.");
|
||||
};
|
||||
};
|
||||
|
||||
done_testing;
|
68
t/scripts/hydra-init.t
Normal file
68
t/scripts/hydra-init.t
Normal file
|
@ -0,0 +1,68 @@
|
|||
use feature 'unicode_strings';
|
||||
use strict;
|
||||
use Setup;
|
||||
|
||||
my %ctx = test_init();
|
||||
|
||||
require Hydra::Schema;
|
||||
require Hydra::Model::DB;
|
||||
|
||||
use Test2::V0;
|
||||
|
||||
my $db = Hydra::Model::DB->new;
|
||||
hydra_setup($db);
|
||||
|
||||
subtest "hydra-init upgrades user's password hashes from sha1 to sha1 inside Argon2" => sub {
|
||||
my $alice = $db->resultset('Users')->create({
|
||||
"username" => "alice",
|
||||
"emailaddress" => 'alice@nixos.org',
|
||||
"password" => "8843d7f92416211de9ebb963ff4ce28125932878" # SHA1 of "foobar"
|
||||
});
|
||||
my $janet = $db->resultset('Users')->create({
|
||||
"username" => "janet",
|
||||
"emailaddress" => 'janet@nixos.org',
|
||||
"password" => "!"
|
||||
});
|
||||
$janet->setPassword("foobar");
|
||||
|
||||
is($alice->password, "8843d7f92416211de9ebb963ff4ce28125932878", "Alices's sha1 is stored in the database");
|
||||
my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-init"));
|
||||
if ($res != 0) {
|
||||
is($stdout, "");
|
||||
is($stderr, "");
|
||||
}
|
||||
is($res, 0, "hydra-init should exit zero");
|
||||
|
||||
subtest "Alice had their password updated in place" => sub {
|
||||
my $updatedAlice = $db->resultset('Users')->find({ username => "alice" });
|
||||
isnt($updatedAlice, undef);
|
||||
isnt($updatedAlice->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was updated in place.");
|
||||
|
||||
my $storedPassword = $updatedAlice->password;
|
||||
ok($updatedAlice->check_password("foobar"), "Their password validates");
|
||||
isnt($storedPassword, $updatedAlice->password, "The password is upgraded in place.");
|
||||
};
|
||||
|
||||
subtest "Janet did not have their password change" => sub {
|
||||
my $updatedJanet = $db->resultset('Users')->find({ username => "janet" });
|
||||
isnt($updatedJanet, undef);
|
||||
is($updatedJanet->password, $janet->password, "The password was not updated in place.");
|
||||
|
||||
ok($updatedJanet->check_password("foobar"), "Their password validates");
|
||||
is($updatedJanet->password, $janet->password, "The password is not upgraded in place.");
|
||||
};
|
||||
|
||||
subtest "Running hydra-init don't break Alice or Janet's passwords" => sub {
|
||||
my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-init"));
|
||||
is($res, 0, "hydra-init should exit zero");
|
||||
|
||||
my $updatedAlice = $db->resultset('Users')->find({ username => "alice" });
|
||||
ok($updatedAlice->check_password("foobar"), "Alice's password validates");
|
||||
|
||||
my $updatedJanet = $db->resultset('Users')->find({ username => "janet" });
|
||||
ok($updatedJanet->check_password("foobar"), "Janet's password validates");
|
||||
};
|
||||
|
||||
};
|
||||
|
||||
done_testing;
|
Loading…
Reference in a new issue