Merge: Correct password option docs and add related tests (#310484)

nixos/modules/config/users-groups.nix

@@ -53,37 +53,71 @@ let
         "*"    # password unset
       ]);
 
+  overrideOrderMutable = ''{option}`initialHashedPassword` -> {option}`initialPassword` -> {option}`hashedPassword` -> {option}`password` -> {option}`hashedPasswordFile`'';
+
+  overrideOrderImmutable = ''{option}`initialHashedPassword` -> {option}`hashedPassword` -> {option}`initialPassword` -> {option}`password` -> {option}`hashedPasswordFile`'';
+
+  overrideOrderText = isMutable: ''
+    If the option {option}`users.mutableUsers` is
+    `${if isMutable then "true" else "false"}`, then the order of precedence is as shown
+    below, where values on the left are overridden by values on the right:
+    ${if isMutable then overrideOrderMutable else overrideOrderImmutable}
+  '';
+
+  multiplePasswordsWarning = ''
+    If multiple of these password options are set at the same time then a
+    specific order of precedence is followed, which can lead to surprising
+    results. The order of precedence differs depending on whether the
+    {option}`users.mutableUsers` option is set.
+  '';
+
+  overrideDescription = ''
+    ${multiplePasswordsWarning}
+
+    ${overrideOrderText false}
+
+    ${overrideOrderText true}
+  '';
+
   passwordDescription = ''
-    The options {option}`hashedPassword`,
-    {option}`password` and {option}`hashedPasswordFile`
-    controls what password is set for the user.
-    {option}`hashedPassword` overrides both
-    {option}`password` and {option}`hashedPasswordFile`.
-    {option}`password` overrides {option}`hashedPasswordFile`.
-    If none of these three options are set, no password is assigned to
-    the user, and the user will not be able to do password logins.
-    If the option {option}`users.mutableUsers` is true, the
-    password defined in one of the three options will only be set when
-    the user is created for the first time. After that, you are free to
-    change the password with the ordinary user management commands. If
-    {option}`users.mutableUsers` is false, you cannot change
-    user passwords, they will always be set according to the password
-    options.
+    The {option}`initialHashedPassword`, {option}`hashedPassword`,
+    {option}`initialPassword`, {option}`password` and
+    {option}`hashedPasswordFile` options all control what password is set for
+    the user.
+
+    In a system where [](#opt-systemd.sysusers.enable) is `false`, typically
+    only one of {option}`hashedPassword`, {option}`password`, or
+    {option}`hashedPasswordFile` will be set.
+
+    In a system where [](#opt-systemd.sysusers.enable) is `true`, typically
+    only one of {option}`initialPassword`, {option}`initialHashedPassword`,
+    or {option}`hashedPasswordFile` will be set.
+
+    If the option {option}`users.mutableUsers` is true, the password defined
+    in one of the above password options will only be set when the user is
+    created for the first time. After that, you are free to change the
+    password with the ordinary user management commands. If
+    {option}`users.mutableUsers` is false, you cannot change user passwords,
+    they will always be set according to the password options.
+
+    If none of the password options are set, then no password is assigned to
+    the user, and the user will not be able to do password-based logins.
+
+    ${overrideDescription}
   '';
 
   hashedPasswordDescription = ''
     To generate a hashed password run `mkpasswd`.
 
-    If set to an empty string (`""`), this user will
-    be able to log in without being asked for a password (but not via remote
-    services such as SSH, or indirectly via {command}`su` or
-    {command}`sudo`). This should only be used for e.g. bootable
-    live systems. Note: this is different from setting an empty password,
-    which can be achieved using {option}`users.users.<name?>.password`.
+    If set to an empty string (`""`), this user will be able to log in without
+    being asked for a password (but not via remote services such as SSH, or
+    indirectly via {command}`su` or {command}`sudo`). This should only be used
+    for e.g. bootable live systems. Note: this is different from setting an
+    empty password, which can be achieved using
+    {option}`users.users.<name?>.password`.
 
-    If set to `null` (default) this user will not
-    be able to log in using a password (i.e. via {command}`login`
-    command).
+    If set to `null` (default) this user will not be able to log in using a
+    password (i.e. via {command}`login` command).
   '';
 
   userOpts = { name, config, ... }: {
@@ -281,6 +315,7 @@ let
         default = null;
         description = ''
           Specifies the hashed password for the user.
+
           ${passwordDescription}
           ${hashedPasswordDescription}
         '';
@@ -294,6 +329,7 @@ let
           Warning: do not set confidential information here
           because it is world-readable in the Nix store. This option
           should only be used for public accounts.
+
           ${passwordDescription}
         '';
       };
@@ -307,6 +343,7 @@ let
           password. The password file is read on each system activation. The
           file should contain exactly one line, which should be the password in
           an encrypted form that is suitable for the `chpasswd -e` command.
+
           ${passwordDescription}
         '';
       };
@@ -329,9 +366,7 @@ let
           {command}`passwd` command. Otherwise, it's
           equivalent to setting the {option}`hashedPassword` option.
 
-          Note that the {option}`hashedPassword` option will override
-          this option if both are set.
-
+          ${passwordDescription}
           ${hashedPasswordDescription}
         '';
       };
@@ -351,8 +386,7 @@ let
           used for guest accounts or passwords that will be changed
           promptly.
 
-          Note that the {option}`password` option will override this
-          option if both are set.
+          ${passwordDescription}
         '';
       };
 
@@ -960,12 +994,11 @@ in {
           (filter (x: x != null) (map (flip getAttr user) passwordOptions));
       in optional (!unambiguousPasswordConfiguration) ''
         The user '${user.name}' has multiple of the options
-        `hashedPassword`, `password`, `hashedPasswordFile`, `initialPassword`
-        & `initialHashedPassword` set to a non-null value.
-        The options silently discard others by the order of precedence
-        given above which can lead to surprising results. To resolve this warning,
-        set at most one of the options above to a non-`null` value.
+        `initialHashedPassword`, `hashedPassword`, `initialPassword`, `password`
+        & `hashedPasswordFile` set to a non-null value.
 
+        ${multiplePasswordsWarning}
+        ${overrideOrderText cfg.mutableUsers}
         The values of these options are:
         ${concatMapStringsSep
           "\n"

nixos/tests/all-tests.nix

@@ -797,6 +797,7 @@ in {
   pantheon = handleTest ./pantheon.nix {};
   paperless = handleTest ./paperless.nix {};
   parsedmarc = handleTest ./parsedmarc {};
+  password-option-override-ordering = handleTest ./password-option-override-ordering.nix {};
   pdns-recursor = handleTest ./pdns-recursor.nix {};
   peerflix = handleTest ./peerflix.nix {};
   peering-manager = handleTest ./web-apps/peering-manager.nix {};
@@ -1050,6 +1051,7 @@ in {
   systemd-sysupdate = runTest ./systemd-sysupdate.nix;
   systemd-sysusers-mutable = runTest ./systemd-sysusers-mutable.nix;
   systemd-sysusers-immutable = runTest ./systemd-sysusers-immutable.nix;
+  systemd-sysusers-password-option-override-ordering = runTest ./systemd-sysusers-password-option-override-ordering.nix;
   systemd-timesyncd = handleTest ./systemd-timesyncd.nix {};
   systemd-timesyncd-nscd-dnssec = handleTest ./systemd-timesyncd-nscd-dnssec.nix {};
   systemd-user-linger = handleTest ./systemd-user-linger.nix {};

nixos/tests/password-option-override-ordering.nix

@@ -0,0 +1,171 @@
+let
+  password1 = "foobar";
+  password2 = "helloworld";
+  hashed_bcrypt = "$2b$05$8xIEflrk2RxQtcVXbGIxs.Vl0x7dF1/JSv3cyX6JJt0npzkTCWvxK"; # fnord
+  hashed_yeshash = "$y$j9T$d8Z4EAf8P1SvM/aDFbxMS0$VnTXMp/Hnc7QdCBEaLTq5ZFOAFo2/PM0/xEAFuOE88."; # fnord
+  hashed_sha512crypt = "$6$ymzs8WINZ5wGwQcV$VC2S0cQiX8NVukOLymysTPn4v1zJoJp3NGyhnqyv/dAf4NWZsBWYveQcj6gEJr4ZUjRBRjM0Pj1L8TCQ8hUUp0"; # meow
+in
+
+import ./make-test-python.nix (
+  { pkgs, ... }:
+  {
+    name = "password-option-override-ordering";
+    meta = with pkgs.lib.maintainers; {
+      maintainers = [ fidgetingbits ];
+    };
+
+    nodes =
+      let
+        # The following users are expected to have the same behavior between immutable and mutable systems
+        # NOTE: Below given A -> B it implies B overrides A . Each entry below builds off the next
+        users = {
+          # mutable true/false: initialHashedPassword -> hashedPassword
+          fran = {
+            isNormalUser = true;
+            initialHashedPassword = hashed_yeshash;
+            hashedPassword = hashed_sha512crypt;
+          };
+
+          # mutable false: initialHashedPassword -> hashedPassword -> initialPassword
+          # mutable true: initialHashedPassword -> initialPassword -> hashedPassword
+          greg = {
+            isNormalUser = true;
+            hashedPassword = hashed_sha512crypt;
+            initialPassword = password1;
+          };
+
+          # mutable false: initialHashedPassword -> hashedPassword -> initialPassword -> password
+          # mutable true: initialHashedPassword -> initialPassword -> hashedPassword -> password
+          egon = {
+            isNormalUser = true;
+            initialPassword = password2;
+            password = password1;
+          };
+
+          # mutable true/false: hashedPassword -> password
+          # NOTE: minor duplication of test above, but to verify no initialXXX use is consistent
+          alice = {
+            isNormalUser = true;
+            hashedPassword = hashed_sha512crypt;
+            password = password1;
+          };
+
+          # mutable false: initialHashedPassword -> hashedPassword -> initialPassword -> password -> hashedPasswordFile
+          # mutable true: initialHashedPassword -> initialPassword -> hashedPassword -> password -> hashedPasswordFile
+          bob = {
+            isNormalUser = true;
+            hashedPassword = hashed_sha512crypt;
+            password = password1;
+            hashedPasswordFile = (pkgs.writeText "hashed_bcrypt" hashed_bcrypt).outPath; # Expect override of everything above
+          };
+
+          # Show hashedPassword -> password -> hashedPasswordFile -> initialPassword is false
+          # to explicitly show the following lib.trace warning in users-groups.nix (which was
+          # the wording prior to PR 310484) is in fact wrong:
+          # ```
+          # The user 'root' has multiple of the options
+          # `hashedPassword`, `password`, `hashedPasswordFile`, `initialPassword`
+          # & `initialHashedPassword` set to a non-null value.
+          # The options silently discard others by the order of precedence
+          # given above which can lead to surprising results. To resolve this warning,
+          # set at most one of the options above to a non-`null` value.
+          # ```
+          cat = {
+            isNormalUser = true;
+            hashedPassword = hashed_sha512crypt;
+            password = password1;
+            hashedPasswordFile = (pkgs.writeText "hashed_bcrypt" hashed_bcrypt).outPath;
+            initialPassword = password2; # lib.trace message implies this overrides everything above
+          };
+
+          # Show hashedPassword -> password -> hashedPasswordFile -> initialHashedPassword is false
+          # to also explicitly show the lib.trace explained above (see cat user) is wrong
+          dan = {
+            isNormalUser = true;
+            hashedPassword = hashed_sha512crypt;
+            initialPassword = password2;
+            password = password1;
+            hashedPasswordFile = (pkgs.writeText "hashed_bcrypt" hashed_bcrypt).outPath;
+            initialHashedPassword = hashed_yeshash; # lib.trace message implies this overrides everything above
+          };
+        };
+
+        mkTestMachine = mutable: {
+          environment.systemPackages = [ pkgs.shadow ];
+          users = {
+            mutableUsers = mutable;
+            inherit users;
+          };
+        };
+      in
+      {
+        immutable = mkTestMachine false;
+        mutable = mkTestMachine true;
+      };
+
+    testScript = ''
+      import crypt
+
+      def assert_password_match(machine, username, password):
+        shadow_entry = machine.succeed(f"getent shadow {username}")
+        print(shadow_entry)
+        hash = shadow_entry.split(":")[1]
+        seed = "$".join(hash.split("$")[:-1])
+        assert crypt.crypt(password, seed) == hash, f"{username} user password does not match"
+
+      with subtest("alice user has correct password"):
+        for machine in machines:
+          assert_password_match(machine, "alice", "${password1}")
+          assert "${hashed_sha512crypt}" not in machine.succeed("getent shadow alice"), f"{machine}: alice user password is not correct"
+
+      with subtest("bob user has correct password"):
+        for machine in machines:
+          print(machine.succeed("getent shadow bob"))
+          assert "${hashed_bcrypt}" in machine.succeed("getent shadow bob"), f"{machine}: bob user password is not correct"
+
+      with subtest("cat user has correct password"):
+        for machine in machines:
+          print(machine.succeed("getent shadow cat"))
+          assert "${hashed_bcrypt}" in machine.succeed("getent shadow cat"), f"{machine}: cat user password is not correct"
+
+      with subtest("dan user has correct password"):
+        for machine in machines:
+          print(machine.succeed("getent shadow dan"))
+          assert "${hashed_bcrypt}" in machine.succeed("getent shadow dan"), f"{machine}: dan user password is not correct"
+
+      with subtest("greg user has correct password"):
+        print(mutable.succeed("getent shadow greg"))
+        assert "${hashed_sha512crypt}" in mutable.succeed("getent shadow greg"), "greg user password is not correct"
+
+        assert_password_match(immutable, "greg", "${password1}")
+        assert "${hashed_sha512crypt}" not in immutable.succeed("getent shadow greg"), "greg user password is not correct"
+
+      for machine in machines:
+        machine.wait_for_unit("multi-user.target")
+        machine.wait_until_succeeds("pgrep -f 'agetty.*tty1'")
+
+      def check_login(machine: Machine, tty_number: str, username: str, password: str):
+        machine.send_key(f"alt-f{tty_number}")
+        machine.wait_until_succeeds(f"[ $(fgconsole) = {tty_number} ]")
+        machine.wait_for_unit(f"getty@tty{tty_number}.service")
+        machine.wait_until_succeeds(f"pgrep -f 'agetty.*tty{tty_number}'")
+        machine.wait_until_tty_matches(tty_number, "login: ")
+        machine.send_chars(f"{username}\n")
+        machine.wait_until_tty_matches(tty_number, f"login: {username}")
+        machine.wait_until_succeeds("pgrep login")
+        machine.wait_until_tty_matches(tty_number, "Password: ")
+        machine.send_chars(f"{password}\n")
+        machine.send_chars(f"whoami > /tmp/{tty_number}\n")
+        machine.wait_for_file(f"/tmp/{tty_number}")
+        assert username in machine.succeed(f"cat /tmp/{tty_number}"), f"{machine}: {username} password is not correct"
+
+      with subtest("Test initialPassword override"):
+        for machine in machines:
+          check_login(machine, "2", "egon", "${password1}")
+
+      with subtest("Test initialHashedPassword override"):
+        for machine in machines:
+          check_login(machine, "3", "fran", "meow")
+    '';
+  }
+)

nixos/tests/systemd-sysusers-password-option-override-ordering.nix

@@ -0,0 +1,77 @@
+{
+  lib,
+  pkgs ? import ../..,
+  ...
+}:
+let
+  password = "test";
+  hashedPassword = "$y$j9T$wLgKY231.8j.ciV2MfEXe1$P0k5j3bCwHgnwW0Ive3w4knrgpiA4TzhCYLAnHvDZ51"; # test
+  hashedPassword1 = "$y$j9T$s8TyQJtNImvobhGM5Nlez0$3E8/O8EVGuA4sr1OQmrzi8GrRcy/AEhj454JjAn72A2"; # test
+
+  hashedPasswordFile = pkgs.writeText "hashed-password" hashedPassword1;
+in
+{
+  name = "systemd-sysusers-password-option-override-ordering";
+
+  meta.maintainers = with lib.maintainers; [ fidgetingbits ];
+
+  nodes.machine = {
+    systemd.sysusers.enable = true;
+    system.etc.overlay.enable = true;
+    boot.initrd.systemd.enable = true;
+
+    users.mutableUsers = true;
+
+    # NOTE: Below given A -> B it implies B overrides A . Each entry below builds off the next
+
+    users.users.root = {
+      hashedPasswordFile = lib.mkForce null;
+      initialHashedPassword = password;
+    };
+
+    users.groups.test = { };
+
+    # initialPassword -> initialHashedPassword
+    users.users.alice = {
+      isSystemUser = true;
+      group = "test";
+      initialPassword = password;
+      initialHashedPassword = hashedPassword;
+    };
+
+    # initialPassword -> initialHashedPassword -> hashedPasswordFile
+    users.users.bob = {
+      isSystemUser = true;
+      group = "test";
+      initialPassword = password;
+      initialHashedPassword = hashedPassword;
+      hashedPasswordFile = hashedPasswordFile.outPath;
+    };
+  };
+
+  testScript = ''
+    machine.wait_for_unit("systemd-sysusers.service")
+
+    with subtest("systemd-sysusers.service contains the credentials"):
+      sysusers_service = machine.succeed("systemctl cat systemd-sysusers.service")
+      print(sysusers_service)
+      # Both are in the unit, but the hashed password takes precedence as shown below.
+      assert "SetCredential=passwd.plaintext-password.alice:${password}" in sysusers_service
+      assert "SetCredential=passwd.hashed-password.alice:${hashedPassword}" in sysusers_service
+
+    with subtest("Correct mode on the password files"):
+      assert machine.succeed("stat -c '%a' /etc/passwd") == "644\n"
+      assert machine.succeed("stat -c '%a' /etc/group") == "644\n"
+      assert machine.succeed("stat -c '%a' /etc/shadow") == "0\n"
+      assert machine.succeed("stat -c '%a' /etc/gshadow") == "0\n"
+
+    with subtest("alice user has correct password"):
+      print(machine.succeed("getent shadow alice"))
+      assert "${hashedPassword}" in machine.succeed("getent shadow alice"), "alice user password is not correct"
+
+    with subtest("bob user has new password after switching to new generation"):
+        print(machine.succeed("getent passwd bob"))
+        print(machine.succeed("getent shadow bob"))
+        assert "${hashedPassword1}" in machine.succeed("getent shadow bob"), "bob user password is not correct"
+  '';
+}