Skip to content

shadow-core: read expiry dates in the forms date -d takes - #398

Open
pierre-warnier wants to merge 6 commits into
mainfrom
fix-365-expire-dates
Open

pierre-warnier wants to merge 6 commits into
mainfrom
fix-365-expire-dates

Conversation

@pierre-warnier

Copy link
Copy Markdown
Collaborator

Fixes #365

chage -E/-d, usermod -e and useradd -e accepted only YYYY-MM-DD or a day count. They now also take the forms upstream reads with its date parser, through parse_datetime (MIT; its jiff and winnow dependencies pass cargo deny check).

A value made only of digits, signs and dashes keeps its strict reading, so YYYY-MM-DD, a day count and -1 behave exactly as before, and an impossible date such as 2023-02-29 is still refused. Anything else goes to parse_datetime in the local time zone, and the time of day is dropped.

Behaviour checked against Ubuntu 26.04's packaged passwd (1:4.17.4-2ubuntu3) in ubuntu:26.04, the same for chage -E, chage -d, usermod -e and useradd -e:

  • 01/02/2030, 2030/01/02, 1/2/30, 2 Jan 2030, January 2, 2030, 2030-1-2 and 2030-01-02 23:30 all store 21916 (2030-01-02).
  • today, yesterday, tomorrow, next week and +1 day are relative to the current day.
  • The stored day is the date as written, whatever TZ is (checked at UTC+14 and UTC-11).
  • garbage, -5, @86400 and an out-of-range day count are rejected (exit 2 for chage, 3 for usermod).

Deliberate differences, kept from today's behaviour: upstream rolls 2023-02-29 over to 2023-03-01 and 2024-13-01 to 2025-01-01, and stores 1969-12-31 as "never"; all three are still rejected here.

Tests: every form in the issue is a unit test in shadow-core (including tomorrow, relative to today) and an integration test for chage -E/-d and usermod -e. The new tests fail on main. The useradd unit test that expected 2025/01/01 to be refused now expects it accepted. The chage, usermod and useradd man pages list the forms, and there is a CHANGELOG entry.

Reviews welcome. Without objection, this will be merged after 7 days.

assert!(
(today - utc_today).abs() <= 1,
"today {today}, UTC {utc_today}"
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can flake if today and tomorrow are read on either side of midnight, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it could: the two calls each read the clock, so a run across midnight got tomorrow = today + 2. Fixed in 8a4c77d: parse_expire_date now reads the clock once and passes that instant to the parser (parse_datetime_at_date), and the test resolves today/tomorrow/yesterday/next week/+3 days against a fixed 2030-01-02 23:59:59 UTC. A separate check still ties the public entry point to the system clock, with a one-day bound that holds at any instant. No other test in the PR reads the clock.

#[test]
fn test_parse_expire_date_invalid_format() {
assert!(parse_expire_date("2025/01/01").is_err());
fn test_parse_expire_date_other_forms() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a test in tests/by-util/test_useradd.rs too

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 6533c3c: test_expiredate_accepts_other_date_forms in tests/by-util/test_useradd.rs runs useradd -e with the same forms as the chage and usermod tests, plus 2030-01-02, the day count 21916, and -1, which leaves the expire field blank. It fails on main at the first new form.

@sylvestre

Copy link
Copy Markdown
Collaborator

sorry but it needs to be rebased

@pierre-warnier

Copy link
Copy Markdown
Collaborator Author

Rebased onto main (b0fa47e). The only conflict was in CHANGELOG.md, where the branch still carried the #391 and #392 entries from #393; they are gone with the revert, so only the #365 entry stays. Head is now 2ac0477.

Comment thread Cargo.toml

# Dates
parse_datetime = "0.17"
jiff = { version = "0.2", default-features = false, features = ["std"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zoned::now() needs jiff's tz-system to read the local zone, no?
here it only works because parse_datetime turns it on, could you please enable it explicitly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was only on through parse_datetime. 70d9375 enables tz-system and tzdb-zoneinfo (for a named zone such as TZ=Europe/Paris) on shadow-core's jiff. cargo tree -e features -i jiff now lists uu_shadow_core under both; cargo deny check is clean. chage -E today gives 20765 under TZ=Pacific/Kiritimati and 20764 under TZ=Etc/GMT+12.

@sylvestre

Copy link
Copy Markdown
Collaborator

sorry but it needs to be rebased

Upstream reads the chage -E/-d, usermod -e and useradd -e argument with
its general date parser, so 01/02/2030, 2030/01/02, 1/2/30, 2 Jan 2030,
January 2, 2030 and tomorrow all work there. Here only YYYY-MM-DD and a
day count did.

A value made only of digits, signs and dashes keeps its strict reading,
so YYYY-MM-DD, a day count and -1 behave as before and an impossible
date is still refused. Anything else goes to parse_datetime in the local
time zone, and the time of day is dropped, as upstream does.
parse_free_form_date now takes the reference time, and parse_expire_date
reads the clock once and passes it in. The relative-date test resolved
"today" and "tomorrow" with two separate clock readings, so a run across
midnight saw tomorrow two days after today. It now resolves every
relative form against a fixed 2030-01-02 23:59:59 UTC; a separate check
keeps the public entry point tied to the system clock with a bound that
holds at any instant.
Cover the same forms the chage and usermod tests take, plus YYYY-MM-DD,
a bare day count, and -1 leaving the expire field blank.
Zoned::now() reads the local zone only with tz-system, and a zone given
by name in TZ is looked up in /usr/share/zoneinfo only with
tzdb-zoneinfo. Both were on only because parse_datetime enables them;
without them a relative date such as 'today' would be counted in UTC.
@pierre-warnier

Copy link
Copy Markdown
Collaborator Author

Rebased on main. New head: 62a57f4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chage -E, usermod -e: dates other than YYYY-MM-DD are rejected

2 participants