From d4d8f1ba1bd6890a0aa1cc8004c6f41bd1292dc4 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 10:31:41 -0400
Subject: [PATCH 1/6] Plugin::Authentication config: modernize

Some time in the last decade the plugin switched to preferring
a flatter namespace for realm config.

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 src/lib/Hydra.pm | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/lib/Hydra.pm b/src/lib/Hydra.pm
index 6d204431..a90235b8 100644
--- a/src/lib/Hydra.pm
+++ b/src/lib/Hydra.pm
@@ -27,27 +27,26 @@ our $VERSION = '0.01';
 __PACKAGE__->config(
     name => 'Hydra',
     default_view => "TT",
-    authentication => {
+    'Plugin::Authentication' => {
         default_realm => "dbic",
-        realms => {
-            dbic => {
-                credential => {
-                    class => "Password",
-                    password_field => "password",
-                    password_type => "hashed",
-                    password_hash_type => "SHA-1",
-                },
-                store => {
-                    class => "DBIx::Class",
-                    user_class => "DB::Users",
-                    role_relation => "userroles",
-                    role_field => "role",
-                },
+
+        dbic => {
+            credential => {
+                class => "Password",
+                password_field => "password",
+                password_type => "hashed",
+                password_hash_type => "SHA-1",
+            },
+            store => {
+                class => "DBIx::Class",
+                user_class => "DB::Users",
+                role_relation => "userroles",
+                role_field => "role",
             },
-            ldap => $ENV{'HYDRA_LDAP_CONFIG'} ? LoadFile(
-                file($ENV{'HYDRA_LDAP_CONFIG'})
-            ) : undef
         },
+        ldap => $ENV{'HYDRA_LDAP_CONFIG'} ? LoadFile(
+            file($ENV{'HYDRA_LDAP_CONFIG'})
+        ) : undef
     },
     'Plugin::Static::Simple' => {
         send_etag => 1,

From 29620df85e72a199afb5e0d9d2c8b3f2e73c5b34 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 10:40:55 -0400
Subject: [PATCH 2/6] Passwords: check in constant time

The default password comparison logic does not use
constant time validation. Switching to constant time
offers a meager improvement by removing a timing
oracle.

A prepatory step in moving to Argon2id password storage, since we'll need this change anyway after
for validating existing passwords.

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 flake.nix                     | 14 ++++++++++++++
 src/lib/Hydra.pm              |  3 +--
 src/lib/Hydra/Schema/Users.pm |  9 +++++++++
 t/Schema/Users.t              | 29 +++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 t/Schema/Users.t

diff --git a/flake.nix b/flake.nix
index 6444af6c..8fac28dd 100644
--- a/flake.nix
+++ b/flake.nix
@@ -229,6 +229,19 @@
               license = with final.stdenv.lib.licenses; [ artistic1 ];
             };
           };
+
+          StringCompareConstantTime = final.buildPerlPackage {
+            pname = "String-Compare-ConstantTime";
+            version = "0.321";
+            src = final.fetchurl {
+              url = "mirror://cpan/authors/id/F/FR/FRACTAL/String-Compare-ConstantTime-0.321.tar.gz";
+              sha256 = "0b26ba2b121d8004425d4485d1d46f59001c83763aa26624dff6220d7735d7f7";
+            };
+            meta = {
+              description = "Timing side-channel protected string compare";
+              license = with final.lib.licenses; [ artistic1 gpl1Plus ];
+            };
+          };
         };
 
         hydra = with final; let
@@ -279,6 +292,7 @@
                 SQLSplitStatement
                 SetScalar
                 Starman
+                StringCompareConstantTime
                 SysHostnameLong
                 TermSizeAny
                 TestMore
diff --git a/src/lib/Hydra.pm b/src/lib/Hydra.pm
index a90235b8..0ff86f11 100644
--- a/src/lib/Hydra.pm
+++ b/src/lib/Hydra.pm
@@ -34,8 +34,7 @@ __PACKAGE__->config(
             credential => {
                 class => "Password",
                 password_field => "password",
-                password_type => "hashed",
-                password_hash_type => "SHA-1",
+                password_type => "self_check",
             },
             store => {
                 class => "DBIx::Class",
diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm
index 7789b42c..ca650d9e 100644
--- a/src/lib/Hydra/Schema/Users.pm
+++ b/src/lib/Hydra/Schema/Users.pm
@@ -195,6 +195,9 @@ __PACKAGE__->many_to_many("projects", "projectmembers", "project");
 # Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-02-06 12:22:36
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4/WZ95asbnGmK+nEHb4sLQ
 
+use Digest::SHA1 qw(sha1_hex);
+use String::Compare::ConstantTime;
+
 my %hint = (
     columns => [
         "fullname",
@@ -210,4 +213,10 @@ sub json_hint {
     return \%hint;
 }
 
+sub check_password {
+    my ($self, $password) = @_;
+
+    return String::Compare::ConstantTime::equals($self->password, sha1_hex($password));
+}
+
 1;
diff --git a/t/Schema/Users.t b/t/Schema/Users.t
new file mode 100644
index 00000000..edfb9002
--- /dev/null
+++ b/t/Schema/Users.t
@@ -0,0 +1,29 @@
+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);
+
+# Catalyst's default password checking is not constant time. To improve
+# the security of the system, we replaced the check password routine.
+# Verify comparing correct and incorrect passwords work.
+
+# Starting the user with a sha1 password
+my $user = $db->resultset('Users')->create({
+    "username" => "alice",
+    "emailaddress" => 'alice@nixos.org',
+    "password" => "8843d7f92416211de9ebb963ff4ce28125932878" # SHA1 of "foobar"
+});
+isnt($user, undef, "My user was created.");
+
+ok(!$user->check_password("barbaz"), "Checking the password, barbaz, is not right");
+ok($user->check_password("foobar"), "Checking the password, foobar, is right");
+
+done_testing;

From 1da70030b74a2f5a2cb5ff9a99f81819540daf12 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 10:55:40 -0400
Subject: [PATCH 3/6] Users: transparently upgrade passwords to Argon2

Passwords that are sha1 will be transparently upgraded to argon2,
and future comparisons will use Argon2

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 flake.nix                     | 43 +++++++++++++++++++++++++++++++++++
 src/lib/Hydra/Schema/Users.pm | 24 ++++++++++++++++++-
 t/Schema/Users.t              | 21 +++++++++++++----
 3 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/flake.nix b/flake.nix
index 8fac28dd..87acfa5f 100644
--- a/flake.nix
+++ b/flake.nix
@@ -70,6 +70,47 @@
             };
           };
 
+          CryptArgon2 = final.perlPackages.buildPerlModule {
+            pname = "Crypt-Argon2";
+            version = "0.010";
+            src = final.fetchurl {
+              url = "mirror://cpan/authors/id/L/LE/LEONT/Crypt-Argon2-0.010.tar.gz";
+              sha256 = "3ea1c006f10ef66fd417e502a569df15c4cc1c776b084e35639751c41ce6671a";
+            };
+            nativeBuildInputs = [ pkgs.ld-is-cc-hook ];
+            meta = {
+              description = "Perl interface to the Argon2 key derivation functions";
+              license = final.lib.licenses.cc0;
+            };
+          };
+
+          CryptPassphrase = final.buildPerlPackage {
+            pname = "Crypt-Passphrase";
+            version = "0.003";
+            src = final.fetchurl {
+              url = "mirror://cpan/authors/id/L/LE/LEONT/Crypt-Passphrase-0.003.tar.gz";
+              sha256 = "685aa090f8179a86d6896212ccf8ccfde7a79cce857199bb14e2277a10d240ad";
+            };
+            meta = {
+              description = "A module for managing passwords in a cryptographically agile manner";
+              license = with final.lib.licenses; [ artistic1 gpl1Plus ];
+            };
+          };
+
+          CryptPassphraseArgon2 = final.buildPerlPackage {
+            pname = "Crypt-Passphrase-Argon2";
+            version = "0.002";
+            src = final.fetchurl {
+              url = "mirror://cpan/authors/id/L/LE/LEONT/Crypt-Passphrase-Argon2-0.002.tar.gz";
+              sha256 = "3906ff81697d13804ee21bd5ab78ffb1c4408b4822ce020e92ecf4737ba1f3a8";
+            };
+            propagatedBuildInputs = with final.perlPackages; [ CryptArgon2 CryptPassphrase ];
+            meta = {
+              description = "An Argon2 encoder for Crypt::Passphrase";
+              license = with final.lib.licenses; [ artistic1 gpl1Plus ];
+            };
+          };
+
           DirSelf = final.buildPerlPackage {
             pname = "Dir-Self";
             version = "0.11";
@@ -267,6 +308,8 @@
                 CatalystViewTT
                 CatalystXScriptServerStarman
                 CatalystXRoleApplicator
+                CryptPassphrase
+                CryptPassphraseArgon2
                 CryptRandPasswd
                 DBDPg
                 DBDSQLite
diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm
index ca650d9e..e11e0354 100644
--- a/src/lib/Hydra/Schema/Users.pm
+++ b/src/lib/Hydra/Schema/Users.pm
@@ -195,6 +195,7 @@ __PACKAGE__->many_to_many("projects", "projectmembers", "project");
 # Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-02-06 12:22:36
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4/WZ95asbnGmK+nEHb4sLQ
 
+use Crypt::Passphrase;
 use Digest::SHA1 qw(sha1_hex);
 use String::Compare::ConstantTime;
 
@@ -216,7 +217,28 @@ sub json_hint {
 sub check_password {
     my ($self, $password) = @_;
 
-    return String::Compare::ConstantTime::equals($self->password, sha1_hex($password));
+    my $authenticator = Crypt::Passphrase->new(
+        encoder    => 'Argon2',
+        validators => [
+            (sub {
+                my ($password, $hash) = @_;
+
+                return String::Compare::ConstantTime::equals($hash, sha1_hex($password));
+            })
+        ],
+    );
+
+    if ($authenticator->verify_password($password, $self->password)) {
+        if ($authenticator->needs_rehash($self->password)) {
+            $self->update({
+                "password" => $authenticator->hash_password($password),
+            });
+        }
+
+        return 1;
+    } else {
+        return 0;
+    }
 }
 
 1;
diff --git a/t/Schema/Users.t b/t/Schema/Users.t
index edfb9002..5f31af76 100644
--- a/t/Schema/Users.t
+++ b/t/Schema/Users.t
@@ -11,11 +11,20 @@ use Test2::V0;
 my $db = Hydra::Model::DB->new;
 hydra_setup($db);
 
-# Catalyst's default password checking is not constant time. To improve
-# the security of the system, we replaced the check password routine.
-# Verify comparing correct and incorrect passwords work.
+# Hydra used to store passwords, by default, as plain unsalted sha1 hashes.
+# We now upgrade these badly stored passwords with much stronger algorithms
+# when the user logs in. Implementing this meant reimplementing our password
+# checking ourselves, so also ensure that basic password checking works.
+#
+# This test:
+#
+# 1. creates a user with the legacy password
+# 2. validates that the wrong password is not considered valid
+# 3. validates that the correct password is valid
+# 4. checks that the checking of the correct password transparently upgraded
+#    the password's storage to a more secure algorithm.
 
-# Starting the user with a sha1 password
+# Starting the user with an unsalted sha1 password
 my $user = $db->resultset('Users')->create({
     "username" => "alice",
     "emailaddress" => 'alice@nixos.org',
@@ -24,6 +33,10 @@ my $user = $db->resultset('Users')->create({
 isnt($user, undef, "My user was created.");
 
 ok(!$user->check_password("barbaz"), "Checking the password, barbaz, is not right");
+
+is($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The unsalted sha1 is in the database.");
 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");
 
 done_testing;

From beb5be43022ac0faa66635df654e40ca9221f9e0 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 10:56:12 -0400
Subject: [PATCH 4/6] Users: password changes via the web UI now use Argon2

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 src/lib/Hydra/Controller/User.pm | 10 ++--------
 src/lib/Hydra/Schema/Users.pm    | 23 +++++++++++++++++------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/lib/Hydra/Controller/User.pm b/src/lib/Hydra/Controller/User.pm
index 9cdece8a..b3512a1b 100644
--- a/src/lib/Hydra/Controller/User.pm
+++ b/src/lib/Hydra/Controller/User.pm
@@ -229,12 +229,6 @@ sub isValidPassword {
 }
 
 
-sub setPassword {
-    my ($user, $password) = @_;
-    $user->update({ password => sha1_hex($password) });
-}
-
-
 sub register :Local Args(0) {
     my ($self, $c) = @_;
 
@@ -294,7 +288,7 @@ sub updatePreferences {
         error($c, "The passwords you specified did not match.")
             if $password ne trim $c->stash->{params}->{password2};
 
-        setPassword($user, $password);
+        $user->setPassword($password);
     }
 
     my $emailAddress = trim($c->stash->{params}->{emailaddress} // "");
@@ -394,7 +388,7 @@ sub reset_password :Chained('user') :PathPart('reset-password') :Args(0) {
         unless $user->emailaddress;
 
     my $password = Crypt::RandPasswd->word(8,10);
-    setPassword($user, $password);
+    $user->setPassword($password);
     sendEmail(
         $c->config,
         $user->emailaddress,
diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm
index e11e0354..55f0f1cb 100644
--- a/src/lib/Hydra/Schema/Users.pm
+++ b/src/lib/Hydra/Schema/Users.pm
@@ -214,9 +214,7 @@ sub json_hint {
     return \%hint;
 }
 
-sub check_password {
-    my ($self, $password) = @_;
-
+sub _authenticator() {
     my $authenticator = Crypt::Passphrase->new(
         encoder    => 'Argon2',
         validators => [
@@ -228,11 +226,16 @@ sub check_password {
         ],
     );
 
+    return $authenticator;
+}
+
+sub check_password {
+    my ($self, $password) = @_;
+
+    my $authenticator = _authenticator();
     if ($authenticator->verify_password($password, $self->password)) {
         if ($authenticator->needs_rehash($self->password)) {
-            $self->update({
-                "password" => $authenticator->hash_password($password),
-            });
+            $self->setPassword($password);
         }
 
         return 1;
@@ -241,4 +244,12 @@ sub check_password {
     }
 }
 
+sub setPassword {
+    my ($self, $password) = @_;;
+
+    $self->update({
+        "password" => _authenticator()->hash_password($password),
+    });
+}
+
 1;

From 1d956be61e5a629beafb97637aa8bc5ce2b29975 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 11:14:47 -0400
Subject: [PATCH 5/6] hydra-create-user: support Argon2

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 src/script/hydra-create-user | 38 ++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/script/hydra-create-user b/src/script/hydra-create-user
index 3fde1aad..6e837270 100755
--- a/src/script/hydra-create-user
+++ b/src/script/hydra-create-user
@@ -5,17 +5,16 @@ use Hydra::Schema;
 use Hydra::Helper::Nix;
 use Hydra::Model::DB;
 use Getopt::Long qw(:config gnu_getopt);
-use Digest::SHA1 qw(sha1_hex);
 
 sub showHelp {
-    print <<EOF;
-Usage: $0 NAME
+    print q%
+Usage: hydra-create-user NAME
   [--rename-from NAME]
   [--type hydra|google|github]
   [--full-name FULLNAME]
   [--email-address EMAIL-ADDRESS]
   [--password PASSWORD]
-  [--password-hash SHA1-HASH]
+  [--password-hash HASH]
   [--wipe-roles]
   [--role ROLE]...
 
@@ -25,9 +24,31 @@ exists, roles are added to the existing roles unless --wipe-roles is
 specified.  If --rename-from is given, the specified account is
 renamed.
 
-Example:
-  \$ hydra-create-user alice --password foobar --role admin
-EOF
+* PASSWORD HASH
+The password hash should be an Argon2id hash, which can be generated
+via:
+
+    $ nix-shell -p libargon2
+    [nix-shell]$ argon2 "$(LC_ALL=C tr -dc '[:alnum:]' < /dev/urandom | head -c16)" -id -t 3 -k 262144 -p 1 -l 16 -e
+    foobar
+    Ctrl^D
+    $argon2id$v=19$m=262144,t=3,p=1$NFU1QXJRNnc4V1BhQ0NJQg$6GHqjqv5cNDDwZqrqUD0zQ
+
+SHA1 is also accepted, but SHA1 support is deprecated and the user's
+password will be upgraded to Argon2id on first login.
+
+
+Examples:
+
+Create a user with an argon2 password:
+
+  $ hydra-create-user alice --password-hash '$argon2id$v=19$m=262144,t=3,p=1$NFU1QXJRNnc4V1BhQ0NJQg$6GHqjqv5cNDDwZqrqUD0zQ' --role admin
+
+Create a user with a password insecurely provided on the commandline:
+
+  $ hydra-create-user alice --password foobar --role admin
+
+%;
     exit 0;
 }
 
@@ -84,8 +105,9 @@ $db->txn_do(sub {
         $user->update({ emailaddress => $userName, password => "!" });
     } else {
         $user->update({ emailaddress => $emailAddress }) if defined $emailAddress;
+
         if (defined $password && !(defined $passwordHash)) {
-            $passwordHash = sha1_hex($password);
+            $user->setPassword($password);
         }
         $user->update({ password => $passwordHash }) if defined $passwordHash;
     }

From 9225be0897ef498169baa79718cfb30a313e3e22 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@floxdev.com>
Date: Thu, 15 Apr 2021 11:15:01 -0400
Subject: [PATCH 6/6] Drop remaining sha1_hex references

Co-authored-by: Graham Christensen <graham@grahamc.com>
---
 src/lib/Hydra/Controller/Admin.pm | 1 -
 src/lib/Hydra/Controller/Root.pm  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/lib/Hydra/Controller/Admin.pm b/src/lib/Hydra/Controller/Admin.pm
index e2a219ff..8dd3c348 100644
--- a/src/lib/Hydra/Controller/Admin.pm
+++ b/src/lib/Hydra/Controller/Admin.pm
@@ -6,7 +6,6 @@ use base 'Catalyst::Controller';
 use Hydra::Helper::Nix;
 use Hydra::Helper::CatalystUtils;
 use Data::Dump qw(dump);
-use Digest::SHA1 qw(sha1_hex);
 use Config::General;
 
 
diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm
index e15c4934..62e793e1 100644
--- a/src/lib/Hydra/Controller/Root.pm
+++ b/src/lib/Hydra/Controller/Root.pm
@@ -7,7 +7,6 @@ use base 'Hydra::Base::Controller::ListBuilds';
 use Hydra::Helper::Nix;
 use Hydra::Helper::CatalystUtils;
 use Hydra::View::TT;
-use Digest::SHA1 qw(sha1_hex);
 use Nix::Store;
 use Nix::Config;
 use Encode;