chage -d YYYY-MM-DD off by one

[kenion]

We've run into another chage issue - since issue #939 was fixed in pr #952, a similar issue has emerged, in which dates input in the YYYY-MM-DD format are parsed incorrectly.

> sudo chage -d 2024-07-29 gus > passwd -S gus > gus P 2024-07-28 0 99999 7 -1

I have been able to reproduce the issue reliably on my Tumbleweed system running shadow 4.16.0-2.1, and cannot reproduce it running versions of shadow without the change in #952.

[kenion]

Tagging @alejandro-colomar for visibility

[alejandro-colomar]

Hi @kenion !

Thanks; the problem seems to have something to do with timezones.

I have applied this patch:

diff --git i/lib/strtoday.c w/lib/strtoday.c
index 55103f88..e667b394 100644
--- i/lib/strtoday.c
+++ w/lib/strtoday.c
@@ -67,5 +67,6 @@ strtoday(const char *str)
        if ((time_t) - 1 == t) {
                return -2;
        }
+fprintf(stderr, "ALX: t: %jd\n", (intmax_t) t);
        return t / DAY;
 }

to see the exact timestamp that it's using. And here's what I see:

$ sudo TZ='-0700' ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
$ passwd -S alx
alx P 2023-09-21 0 99999 7 -1
$ sudo TZ='+0700' ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
$ passwd -S alx
alx P 2023-09-21 0 99999 7 -1
$ sudo TZ='+0200' ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
$ passwd -S alx
alx P 2023-09-21 0 99999 7 -1
$ sudo ./src/chage -d 2023-09-21 alx
ALX: t: 1695247200
alx@debian:~/src/shadow/shadow/stpsep$ passwd -S alx
alx P 2023-09-20 0 99999 7 -1

Somehow, if I specify a timezone, it works as if using UTC (Edit: I should use strings such as Africa/Brazzaville for the timezone; my bad). I suspect chage(1) probably clears the environment, and thus ignores the value I provide, and removes my system setting (+0200). But if I don't specify TZ, it seems to use my system timezone, and then it behaves as you say.

https://www.epochconverter.com/

According to date(1), not specifying a timezone, the interpretation of a date means local time, not UTC:

$ date -d 2023-09-21
Thu Sep 21 00:00:00 CEST 2023

Which means that this is not a bug in chage(1).

But then, passwd(1) seems to be using UTC, which doesn't match what chage(1) does.

I like that passwd(1) uses UTC, so I wouldn't claim there to be a bug either. But we have some issue here.

[alejandro-colomar]

I still see something I don't like:

alx@debian:~/src/shadow/shadow/master$ sudo TZ=CET ./src/chage -d 2023-09-21 alx
ALX: t: 1695247200
alx@debian:~/src/shadow/shadow/master$ sudo TZ=CEST ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=UTC ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=EAT ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=WAT ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=AKST ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400

Why does it behave different for TZ=CET, but uses the same timestamp for many (all?) other timezones?

Cc: @eggert

[alejandro-colomar]

I still see something I don't like:

alx@debian:~/src/shadow/shadow/master$ sudo TZ=CET ./src/chage -d 2023-09-21 alx
ALX: t: 1695247200
alx@debian:~/src/shadow/shadow/master$ sudo TZ=CEST ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=UTC ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=EAT ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=WAT ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400
alx@debian:~/src/shadow/shadow/master$ sudo TZ=AKST ./src/chage -d 2023-09-21 alx
ALX: t: 1695254400

Why does it behave different for TZ=CET, but uses the same timestamp for many (all?) other timezones?

Cc: @eggert

Ahhh, maybe I can't use those codes there. I guess I need things like Africa/Brazzaville, but I don't know why CET was different.

Edit: It seems CET is a valid TZ identifier, while the others I tried are not. This I learned today. :)

2nd edit: CEST is a time zone abbreviation, which shadow doesn't seem to support. CET, while being a time zone abbreviation, it also is a TZ identifier itself, which is probably why shadow supports it. date(1) can handle both in TZ=, but shadow uses a very old implementation in get_date(), which is probably the reason why it's so bad.

[alejandro-colomar]

@hallyn , @ikerexxe , @kenion

What do you think about the fact that some shadow programs respect the locale but others don't. This results in confusing issues like this one.

alx@debian:~$ ls -l $(which passwd)
-rwsr-xr-x 1 root root 118168 Jul 21 21:05 /usr/bin/passwd
alx@debian:~$ ls -l $(which chage)
-rwxr-sr-x 1 root shadow 117944 Jul 21 21:05 /usr/bin/chage

passwd(1) is setuid-root chage(1) is setgid-shadow

[alejandro-colomar]

@kenion

The following should solve the discrepancies you're seeing:

diff --git i/lib/time/day_to_str.h w/lib/time/day_to_str.h
index 374240f5..f908a752 100644
--- i/lib/time/day_to_str.h
+++ w/lib/time/day_to_str.h
@@ -38,7 +38,7 @@ day_to_str(size_t size, char buf[size], long day)
                return;
        }

-       if (gmtime_r(&date, &tm) == NULL) {
+       if (localtime_r(&date, &tm) == NULL) {
                strtcpy(buf, "future", size);
                return;
        }

However, I'm not sure we want that.

[hallyn]

@hallyn , @ikerexxe , @kenion

What do you think about the fact that some shadow programs respect the locale but others don't. This results in confusing issues like this one.

alx@debian:~$ ls -l $(which passwd)
-rwsr-xr-x 1 root root 118168 Jul 21 21:05 /usr/bin/passwd
alx@debian:~$ ls -l $(which chage)
-rwxr-sr-x 1 root shadow 117944 Jul 21 21:05 /usr/bin/chage

passwd(1) is setuid-root chage(1) is setgid-shadow

Sorry, what is the issue you're showing in this example?

[hallyn]

Hm, localtime probably should be used whenever interacting with the user. Right?

[alejandro-colomar]

@hallyn , @ikerexxe , @kenion What do you think about the fact that some shadow programs respect the locale but others don't. This results in confusing issues like this one.

alx@debian:~$ ls -l $(which passwd)
-rwsr-xr-x 1 root root 118168 Jul 21 21:05 /usr/bin/passwd
alx@debian:~$ ls -l $(which chage)
-rwxr-sr-x 1 root shadow 117944 Jul 21 21:05 /usr/bin/chage

passwd(1) is setuid-root chage(1) is setgid-shadow

Sorry, what is the issue you're showing in this example?

Nothing, just for reminding that we should be careful with them. :)

(Just like we usually do (void) setlocale (LC_ALL, ""); at the begining of most programs. It seems to me a bit strange to respect the timezone and translate strings, but not respect the locale.)

(Edit: my bad; setlocale with an empty string is to read the environment. You can guess that I don't do much locale programming.)

[alejandro-colomar]

Hm, localtime probably should be used whenever interacting with the user. Right?

Hmm, possibly; or maybe probably. A user can always set the C locale, and a UTC timezone if they want that. I'll send a patch.

[kenion]

Thanks for looking into this!