*/: shadow(5): sp_min: Ignore field, and clear it

Whenever we were reading it, let's assume it contains a -1 (the integer
representation of an empty field).  Whenever we were writing it, let's
write a -1.

I think this would be worth a CVE.  I've contacted Debian, in case they
can issue a CVE.  Here's the draft I've sent them for the CVE:

	shadow-utils: inability to change a compromised password

Description
	A flaw was found in /etc/shadow, and the programs that handle
	the file.

	The /etc/shadow database (documented in shadow(5)) entries
	contain a "minimum password age" field.  This field imposes
	a minimum password age before the password can be changed.

	If a user's password is compromised before that period expires,
	the user must contact the administrator to be able to change the
	password.  This can usually take hours or days, and during these
	hours or days, an attacker would have unlimited access to the
	credentials.

	One option of the user would be to lock its own password with
	'passwd -l', thus being locked out of its own account.

	The history of this 4th field of /etc/shadow entries is
	considered dubious in retrospective, and the recommended action
	is to never set this field.  If the field is set, the
	recommended action is to ignore the field, and if possible,
	remove it.

Link: <https://github.com/shadow-maint/shadow/pull/1482>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/lib/age.c b/lib/age.c
index bdb789e30db5a963505feeba05955f8f2df46e40..fe8b0e6198d448b055c8781523c20e71e1803753 100644 (file)
--- a/lib/age.c
+++ b/lib/age.c
@@ -71,7 +71,7 @@ int expire (const struct passwd *pw, /*@null@*/const struct spwd *sp)
         * change that password.
         */
 
-       if ((status > 1) || (sp->sp_max < sp->sp_min)) {
+       if (status > 1) {
                (void) puts (_("  Contact the system administrator."));
                exit (EXIT_FAILURE);
        }

diff --git a/lib/pwd2spwd.c b/lib/pwd2spwd.c
index ea1bf064b28500e94e5a8ff2ef8cae6f199fdc0e..0d71089cfaa533f69973c1fde00579d6b5915ad4 100644 (file)
--- a/lib/pwd2spwd.c
+++ b/lib/pwd2spwd.c
@@ -38,7 +38,7 @@ struct spwd *pwd_to_spwd (const struct passwd *pw)
         * file.  They are set to uninitialized values.
         */
        sp.sp_lstchg = -1;
-       sp.sp_min = 0;
+       sp.sp_min = -1;
        sp.sp_max = -1;
        sp.sp_warn = -1;
        sp.sp_expire = -1;

diff --git a/lib/shadow/shadow/sgetspent.c b/lib/shadow/shadow/sgetspent.c
index 1c2fbe22b5ea6de530aa9a221dd59f04f80d3ebc..33e3ecc43f169d9edf01756e7cf5219b7d8ed9e9 100644 (file)
--- a/lib/shadow/shadow/sgetspent.c
+++ b/lib/shadow/shadow/sgetspent.c
@@ -75,14 +75,7 @@ sgetspent(const char *s)
        else if (a2sl(&spwd.sp_lstchg, fields[2], NULL, 0, 0, LONG_MAX) == -1)
                return NULL;
 
-       /*
-        * Get the minimum period between password changes.
-        */
-
-       if (streq(fields[3], ""))
-               spwd.sp_min = -1;
-       else if (a2sl(&spwd.sp_min, fields[3], NULL, 0, 0, LONG_MAX) == -1)
-               return NULL;
+       spwd.sp_min = -1;
 
        /*
         * Get the maximum number of days a password is valid.

diff --git a/lib/shadowmem.c b/lib/shadowmem.c
index 1a7b9a2a755d500f7b8a0bad5c839e1ff86f7aba..751a511b7034476e5538974c6881728721f77d58 100644 (file)
--- a/lib/shadowmem.c
+++ b/lib/shadowmem.c
@@ -32,7 +32,7 @@
        }
        /* The libc might define other fields. They won't be copied. */
        sp->sp_lstchg = spent->sp_lstchg;
-       sp->sp_min    = spent->sp_min;
+       sp->sp_min    = -1;
        sp->sp_max    = spent->sp_max;
        sp->sp_warn   = spent->sp_warn;
        sp->sp_inact  = spent->sp_inact;

diff --git a/man/shadow.5.xml b/man/shadow.5.xml
index d3cc03982c47d8a7766a31c3492d6fd7b2afddc0..ea6af2817cc2baa4528a27f644a794f752d1aa45 100644 (file)
--- a/man/shadow.5.xml
+++ b/man/shadow.5.xml
@@ -125,14 +125,7 @@
       <varlistentry>
        <term><emphasis role="bold">minimum password age</emphasis></term>
        <listitem>
-         <para>
-           The minimum password age is the number of days the user must wait
-           before they can change their password again.
-         </para>
-         <para>
-           An empty field and value 0 mean that there is no minimum
-           password age.
-         </para>
+         <para>This deprecated field should be empty, and is ignored.</para>
        </listitem>
       </varlistentry>
       <varlistentry>

diff --git a/src/chage.c b/src/chage.c
index f13a935617dbd54aef817b165639e999684e95c6..ac215ec146a810e97d43fd34a8533fadf9c0d2a1 100644 (file)
--- a/src/chage.c
+++ b/src/chage.c
@@ -70,7 +70,6 @@ static bool spw_locked = false;       /* Indicate if the shadow file is locked */
 static char user_name[BUFSIZ] = "";
 static uid_t user_uid = -1;
 
-static long mindays;
 static long maxdays;
 static long lstchgdate;
 static long warndays;
@@ -167,8 +166,6 @@ static int new_fields (void)
        (void) puts (_("Enter the new value, or press ENTER for the default"));
        (void) puts ("");
 
-       mindays = -1;
-
        stprintf_a(buf, "%ld", maxdays);
        change_field(buf, sizeof(buf), _("Maximum Password Age"));
        if (a2sl(&maxdays, buf, NULL, 0, -1, LONG_MAX) == -1)
@@ -314,15 +311,6 @@ static void list_fields (void)
        (void) fputs (_("Account expires\t\t\t\t\t\t: "), stdout);
        print_day_as_date(expdate);
 
-       /*
-        * Start with the easy numbers - the number of days before the
-        * password can be changed, the number of days after which the
-        * password must be changed, the number of days before the password
-        * expires that the user is told, and the number of days after the
-        * password expires that the account becomes unusable.
-        */
-       printf (_("Minimum number of days between password change\t\t: %ld\n"),
-               mindays);
        printf (_("Maximum number of days between password change\t\t: %ld\n"),
                maxdays);
        printf (_("Number of days of warning before password expires\t: %ld\n"),
@@ -618,7 +606,7 @@ static void update_age (/*@null@*/const struct spwd *sp,
         * password files will commit any changes that have been made.
         */
        spwent.sp_max = maxdays;
-       spwent.sp_min = mindays;
+       spwent.sp_min = -1;
        spwent.sp_lstchg = lstchgdate;
        spwent.sp_warn = warndays;
        spwent.sp_inact = inactdays;
@@ -645,7 +633,6 @@ static void get_defaults (/*@null@*/const struct spwd *sp)
                if (!Mflg) {
                        maxdays = sp->sp_max;
                }
-               mindays = sp->sp_min;
                if (!dflg) {
                        lstchgdate = sp->sp_lstchg;
                }
@@ -666,7 +653,6 @@ static void get_defaults (/*@null@*/const struct spwd *sp)
                if (!Mflg) {
                        maxdays = -1;
                }
-               mindays = -1;
                if (!dflg) {
                        lstchgdate = -1;
                }

diff --git a/src/passwd.c b/src/passwd.c
index 7e9f63e6b4802e3f870b300d34824c305a9ecfbe..1873bf89fcee8a936967c43c62cd093afe22ff4b 100644 (file)
--- a/src/passwd.c
+++ b/src/passwd.c
@@ -378,14 +378,11 @@ static void check_password (const struct passwd *pw, const struct spwd *sp, bool
 
        /*
         * Expired accounts cannot be changed ever. Passwords which are
-        * locked may not be changed. Passwords where min > max may not be
-        * changed. Passwords which have been inactive too long cannot be
-        * changed.
+        * locked may not be changed.
+        * Passwords which have been inactive too long cannot be changed.
         */
        if (   strprefix(sp->sp_pwdp, "!")
-           || (exp_status > 1)
-           || (   (sp->sp_max >= 0)
-               && (sp->sp_min > sp->sp_max))) {
+           || (exp_status > 1)) {
                (void) fprintf (stderr,
                                _("The password for %s cannot be changed.\n"),
                                sp->sp_namp);
@@ -393,28 +390,6 @@ static void check_password (const struct passwd *pw, const struct spwd *sp, bool
                closelog ();
                fail_exit(E_NOPERM, process_selinux);
        }
-
-       /*
-        * Passwords may only be changed after sp_min time is up.
-        */
-       if (sp->sp_lstchg > 0) {
-               long now, ok;
-               now = time(NULL) / DAY;
-               ok = sp->sp_lstchg;
-               if (   (sp->sp_min > 0)
-                   && __builtin_add_overflow(ok, sp->sp_min, &ok)) {
-                       ok = LONG_MAX;
-               }
-
-               if (now < ok) {
-                       (void) fprintf (stderr,
-                                       _("The password for %s cannot be changed yet.\n"),
-                                       sp->sp_namp);
-                       SYSLOG(LOG_WARN, "now < minimum age for '%s'", sp->sp_namp);
-                       closelog ();
-                       fail_exit(E_NOPERM, process_selinux);
-               }
-       }
 }
 
 static /*@observer@*/const char *pw_status (const char *pass)
@@ -439,11 +414,10 @@ static void print_status (const struct passwd *pw)
        sp = prefix_getspnam (pw->pw_name); /* local, no need for xprefix_getspnam */
        if (NULL != sp) {
                day_to_str_a(date, sp->sp_lstchg);
-               (void) printf ("%s %s %s %ld %ld %ld %ld\n",
+               (void) printf ("%s %s %s -1 %ld %ld %ld\n",
                               pw->pw_name,
                               pw_status (sp->sp_pwdp),
                               date,
-                              sp->sp_min,
                               sp->sp_max,
                               sp->sp_warn,
                               sp->sp_inact);

diff --git a/tests/system/tests/test_newusers.py b/tests/system/tests/test_newusers.py
index 0cc584c6b83b998a272ef8e5442e72399809f2d2..a8edf47d202c748b3f7b72f013f04bbf05fcee25 100644 (file)
--- a/tests/system/tests/test_newusers.py
+++ b/tests/system/tests/test_newusers.py
@@ -47,7 +47,7 @@ def test_newusers__create_users_from_stdin(shadow: Shadow):
     assert shadow_entry.name == "tuser1", "Incorrect username"
     assert shadow_entry.password is not None, "Incorrect password"
     assert shadow_entry.last_changed == days_since_epoch(), "Incorrect last changed"
-    assert shadow_entry.min_days == 0, "Incorrect min days"
+    assert shadow_entry.min_days is None, "Incorrect min days"
     assert shadow_entry.max_days == 99999, "Incorrect max days"
     assert shadow_entry.warn_days == 7, "Incorrect warn days"
 
@@ -79,7 +79,7 @@ def test_newusers__create_users_from_stdin(shadow: Shadow):
     assert shadow_entry.name == "tuser2", "Incorrect username"
     assert shadow_entry.password is not None, "Incorrect password"
     assert shadow_entry.last_changed == days_since_epoch(), "Incorrect last changed"
-    assert shadow_entry.min_days == 0, "Incorrect min days"
+    assert shadow_entry.min_days is None, "Incorrect min days"
     assert shadow_entry.max_days == 99999, "Incorrect max days"
     assert shadow_entry.warn_days == 7, "Incorrect warn days"
 
@@ -135,7 +135,7 @@ def test_newusers__create_users_from_file(shadow: Shadow):
     assert shadow_entry.name == "tuser1", "Incorrect username"
     assert shadow_entry.password is not None, "Incorrect password"
     assert shadow_entry.last_changed == days_since_epoch(), "Incorrect last changed"
-    assert shadow_entry.min_days == 0, "Incorrect min days"
+    assert shadow_entry.min_days is None, "Incorrect min days"
     assert shadow_entry.max_days == 99999, "Incorrect max days"
     assert shadow_entry.warn_days == 7, "Incorrect warn days"
 
@@ -167,7 +167,7 @@ def test_newusers__create_users_from_file(shadow: Shadow):
     assert shadow_entry.name == "tuser2", "Incorrect username"
     assert shadow_entry.password is not None, "Incorrect password"
     assert shadow_entry.last_changed == days_since_epoch(), "Incorrect last changed"
-    assert shadow_entry.min_days == 0, "Incorrect min days"
+    assert shadow_entry.min_days is None, "Incorrect min days"
     assert shadow_entry.max_days == 99999, "Incorrect max days"
     assert shadow_entry.warn_days == 7, "Incorrect warn days"
 

diff --git a/tests/system/tests/test_useradd.py b/tests/system/tests/test_useradd.py
index ac40ea6bcdaaf0ce7547d1035c430e74aaa9b712..b66cf7b026f50502cdfc68153e2c8102ba33c1f2 100644 (file)
--- a/tests/system/tests/test_useradd.py
+++ b/tests/system/tests/test_useradd.py
@@ -51,7 +51,7 @@ def test_useradd__add_user(shadow: Shadow):
     assert shadow_entry.name == "tuser", "Incorrect username"
     assert shadow_entry.password == "!", "Incorrect password"
     assert shadow_entry.last_changed == days_since_epoch(), "Incorrect last changed"
-    assert shadow_entry.min_days == 0, "Incorrect min days"
+    assert shadow_entry.min_days is None, "Incorrect min days"
     assert shadow_entry.max_days == 99999, "Incorrect max days"
     assert shadow_entry.warn_days == 7, "Incorrect warn days"