Rex icon indicating copy to clipboard operation
Rex copied to clipboard

Account bumps password set time, even when it doesnt

Open djzort opened this issue 4 years ago • 2 comments

Describe the bug

The 'account' function always bumps the password set time, even when it doesnt change anything.

Consider this simple config

    account 'mp',
        ensure => 'present',
        uid    => 1001,
        shell  => '/bin/false',
        home   => '/home/mp',
        create_home => 1,
        crypt_password => '!'; # disabled login
    account 'dc',
        ensure => 'present',
        uid    => 1002,
        shell  => '/bin/false',
        home   => '/home/dc',
        create_home => 1,
        crypt_password => '!'; # disabled login

Then i run rex ( i have etckeeper installed )

git diff /etc/shadow
diff --git a/shadow b/shadow
index 02e510e..2b472fe 100644
--- a/shadow
+++ b/shadow
@@ -22,7 +22,7 @@ systemd-network:*:18496:0:99999:7:::
 mp:!:18876:0:99999:7:::
-dc:!:18876:0:99999:7:::
+mp:!:18911:0:99999:7:::
+dc:!:18911:0:99999:7:::

Run rex again, then...

root@batou:/etc# git diff /etc/shadow
diff --git a/shadow b/shadow
index 02e510e..1053301 100644
--- a/shadow
+++ b/shadow
-mp:!:18876:0:99999:7:::
-dc:!:18876:0:99999:7:::
+mp:!:18912:0:99999:7:::
+dc:!:18912:0:99999:7:::

The "date of last password change" field changes (per man shadow)

I dont think that field should change if the password doesnt change.

YMMV on that. I can see how someone could make an argument that it should.

How to reproduce it

Create a user and run rex over and over. Diff the shadow files

Expected behavior

I dont think that field should change if the password doesnt change. Or perhaps be controllable.

Circumstances

  • Rex version: (R)?ex 1.13.4
  • Perl version: 5.32.0
  • OS running rex: Debian bookworm/sid
  • OS managed by rex: Debian bookworm/sid
  • How rex was installed: cpanm Rex

Debug log

Is this really needed?

djzort avatar Oct 12 '21 02:10 djzort

Thank for the report, @djzort!

In the case of account ..., crypt_password => $encrypted_password, Rex translates that to a usermod -p '$encrypted_password' call (on Linux endpoints), which in turn will set the password. I believe usermod does not check if it's the same encrypted password that is already set, just sets it directly, and bumps the last updated date (once a day only, though). It also doesn't seem to have a command line option to "check password first, and only set if it's different".

I'd say the intent of account is to be declarative and idempotent, while create_user is imperative. In that sense, we could to the following:

  • Differentiate between account and create_user calls a bit more in Rex::User::* implementations, based on the caller. Always update the password when called from create_user, and only update via account when the password is different.
  • For crypt_password, read the contents of /etc/shadow first, and only run the usermod call if the passed value doesn't match what is currently set.
  • For the account ..., password => $password case (=pass the plain password string), call perl's crypt function on the password string, and compare it to the one stored in /etc/shadow, and only set it if it's different.

ferki avatar Oct 24 '21 18:10 ferki

In Rex/User/Linux.pm perhaps something like

  my $expire = q();
  if (exists $data->{expire}) {
     $expire = $data->{expire};
     $run_cmd = 1;
  }
  $cmd .= " --expiredate '" . $expire . "'";

The problem with the above is that if the expiry is removed from the config, then you would expect it to be removed from the system - but it wouldn't be.

Reviewing that Linux.pm module, that same sort of counter-intuitive expectation seems to exist with other values too, such as 'crypt_password' and 'comment'. In both cases i would assume that removing them would remove their respective value.

On the other hand I would expect the absence of 'uid' or 'home' to mean 'don't care'.

Not sure what the right answer is.

djzort avatar Nov 13 '21 21:11 djzort