chage -d YYYY-MM-DD off by one
kenion opened this issue · 11 comments
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.
Tagging @alejandro-colomar for visibility
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 -1Somehow, 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 2023Which 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.
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: 1695254400Why does it behave different for TZ=CET, but uses the same timestamp for many (all?) other timezones?
Cc: @eggert
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: 1695254400Why 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.
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/chagepasswd(1) is setuid-root
chage(1) is setgid-shadow
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.
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/chagepasswd(1) is setuid-root chage(1) is setgid-shadow
Sorry, what is the issue you're showing in this example?
Hm, localtime probably should be used whenever interacting with the user. Right?
@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.)
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.
Thanks for looking into this!