From d10d8964f28e98e551d39aa5bee70d2c38386dfc Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 16 Apr 2021 09:58:46 -0400
Subject: [PATCH 1/3] Users: add a validation step which lets the user's
 password be a Argon2 hashed sha1 hash.

OWASP suggests expiring all passwords and requiring users to update their password.
However, we don't have a way to do this. They suggest this mechanism
as a good alternative:
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#upgrading-legacy-hashes
---
 src/lib/Hydra/Schema/Users.pm |  6 ++++++
 t/Schema/Users.t              | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm
index 55f0f1cb..72686367 100644
--- a/src/lib/Hydra/Schema/Users.pm
+++ b/src/lib/Hydra/Schema/Users.pm
@@ -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;
diff --git a/t/Schema/Users.t b/t/Schema/Users.t
index 5f31af76..d8cbaf8c 100644
--- a/t/Schema/Users.t
+++ b/t/Schema/Users.t
@@ -39,4 +39,15 @@ 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.");
+};
+
 done_testing;

From 79b0ddc27dec968dd7d7754320d3c57f69fe8344 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 16 Apr 2021 11:28:00 -0400
Subject: [PATCH 2/3] hydra-create-user: re-hash sha1 as Argon2

---
 src/lib/Hydra/Schema/Users.pm | 11 +++++++
 src/script/hydra-create-user  |  5 +++-
 t/Schema/Users.t              | 19 ++++++++++++
 t/scripts/hydra-create-user.t | 56 +++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 t/scripts/hydra-create-user.t

diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm
index 72686367..9f2e1bc0 100644
--- a/src/lib/Hydra/Schema/Users.pm
+++ b/src/lib/Hydra/Schema/Users.pm
@@ -258,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;
diff --git a/src/script/hydra-create-user b/src/script/hydra-create-user
index 6e837270..0ed51bd8 100755
--- a/src/script/hydra-create-user
+++ b/src/script/hydra-create-user
@@ -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;
diff --git a/t/Schema/Users.t b/t/Schema/Users.t
index d8cbaf8c..fd1a66e1 100644
--- a/t/Schema/Users.t
+++ b/t/Schema/Users.t
@@ -50,4 +50,23 @@ subtest "Hashing their sha1 as Argon2 still lets them log in with their password
     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;
diff --git a/t/scripts/hydra-create-user.t b/t/scripts/hydra-create-user.t
new file mode 100644
index 00000000..3706d911
--- /dev/null
+++ b/t/scripts/hydra-create-user.t
@@ -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;

From 05636de7d2e250dd31710b5d4dcde8c5c6724a0d Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 16 Apr 2021 12:09:30 -0400
Subject: [PATCH 3/3] hydra-init: upgrade passwords to Argon2 on startup

---
 src/script/hydra-init  |  9 ++++++
 t/scripts/hydra-init.t | 68 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 t/scripts/hydra-init.t

diff --git a/src/script/hydra-init b/src/script/hydra-init
index 74498186..ac9d4afe 100755
--- a/src/script/hydra-init
+++ b/src/script/hydra-init
@@ -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);
+    }
+}
diff --git a/t/scripts/hydra-init.t b/t/scripts/hydra-init.t
new file mode 100644
index 00000000..517a14d7
--- /dev/null
+++ b/t/scripts/hydra-init.t
@@ -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;