usermod -E off-by-one

[zeha]

Looks like usermod -E has a similar problem like #1057.

NB: man passwd documents: administrators should use usermod --expiredate 1 (this sets the account's expire date to Jan 2, 1970).

So we can assume, and indeed in old versions this was the case, '0' in /etc/shadow means Jan 1 1970, '1', should mean Jan 2 1970.

However, one can now observe:

Correct, passing 1 directly (as days since 1970-01-01):

% sudo usermod --expiredate 1 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

Passing 1970-01-02:

% sudo usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:

When explicitly using UTC:

% sudo TZ=UTC usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

My timezone in the other examples was CET.

This was reported as Debian bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1095430

Version: 4.17.1

[zeha]

After rechecking this, it looks like chage still has this problem, I thought #1057 was supposed to fix this?

% sudo chage -E 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=UTC chage -E 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

Or is this now "working as intended"?

[alejandro-colomar]

Hmmmm, I seem to remember this was a nightmare. I think you're experiencing something like #1059 (comment).

Cc: @hallyn , @kenion, @ikerexxe , @timparenti, @eggert

I oppose adding configurability to this. Whatever we do, I want to hard-code it. Let's propose alternatives (I very much prefer the first one):

• Revert lib/, src/: Use local time for human-readable dates #1058 (so print UTC dates again, which is what we store anyway). Append a Z to the date string yyyy-mm-ddZ to indicate UTC.

• Revert lib/, src/: Use local time for human-readable dates #1058 (so print UTC dates again). Do not append a Z. This is old behavior. But it's a lie, since non-suffixed dates should be local dates.

• Keep being in a mix of UTC and local dates when we only store day-precision UTC dates. This is hell.

[jubalh]

And we just found that useradd --expiredate has the problem as well.

[timparenti]

My context here is simply from a general UX/design perspective, informed by a great deal of "thinking about time".

Conceptually, the act of expiration occurs at one specific instant; in this case, 24:00:00 UTC on the specified date. So it is always better to talk about expiration times rather than expiration dates wherever possible.

Since, however, due to the precision of how expiration times are stored here, it is not possible for expiration to occur at any other time-of-day — or, say, in accordance with any other non-UTC time zone — then it does not make much sense to confuse matters by using "a mix of UTC and local" times. So, to me, that rules out the viability of that option.

But that still leaves some potentially nasty surprises for administrators naïvely entering expiration dates with local time in mind:

• In west-of-Greenwich time zones, expiration occurs some hours earlier than intended, raising availability concerns.
• In east-of-Greenwich time zones, expiration occurs some hours later than intended, raising security concerns.

The architectural decision to store expiration dates as opposed to expiration times having already been made, though, the main strategy to alleviate these concerns is through providing useful feedback.

So my recommendation, wherever possible, would be to fully qualify any printed expiration times, e.g., yyyy-mm-dd 23:59:59 UTC, if that is indeed the actual effect. While Z is concise, it is not as clear in communicating the necessary meaning. Z is defined in ISO 8601 as the confusingly-named "UTC of day" and can only be applied to time-of-day representations under the standard, not dates alone. In other words, times are localizable in a way that dates are not; communicate accordingly.

As an aside, I've dealt with account management for a small Kerberos realm. Expiration times are stored as full times there, of course, but if specified as only a date, are parsed to 23:59:59 UTC, e.g.:

% kadmin modify --expiration-time=2027-02-10 foo
% kadmin get foo | grep "Principal expires"
    Principal expires: 2027-02-10 23:59:59 UTC

Ideally, one would further help avoid potential surprises by providing the full expanded timestamp in output to relevant commands, possibly even alongside a conversion to local time, but admittedly that's more of a stylistic point.

[alejandro-colomar]

Hi @timparenti !

Here's a link to a discussion we had in the tzdb mailing list last year:
https://lists.iana.org/hyperkitty/list/tz@iana.org/message/ENE5IFV3GAH6WK22UJ6YU57D6TQINSP5/

I'll continue there, because I asked @eggert something, and he didn't respond. It was about specifying 2023-09-20[+0000]. AFAIR, the suffix can be applied to a date, right? Or at least, the standard was fuzzy enough that we could consider it a good-taste extension to the standard.

(please follow-up in the tzdb thread.)

The only concern I have with printing date&time when we only store date is that it is common practice to not print more significant digits than the error. That is, the error should usually have 1 or 2 significant digits. I don't know if any standards actually codify this.

Cheers,
Alex

[alejandro-colomar]

Conceptually, the act of expiration occurs at one specific instant; in this case, 24:00:00 UTC on the specified date. So it is always better to talk about expiration times rather than expiration dates wherever possible.

I'm not happy expressing more precision than we really store. Telling the user 2023-09-20Z gives accurate information, and gives the hint that we don't even look at a specific time; we round to the day.

The architectural decision to store expiration dates as opposed to expiration times having already been made, though, the main strategy to alleviate these concerns is through providing useful feedback.

Indeed. I would have stored seconds since UTC, which is the usual thing to do. But the decision was made long before us.

So my recommendation, wherever possible, would be to fully qualify any printed expiration times, e.g., yyyy-mm-dd 23:59:59 UTC, if that is indeed the actual effect. While Z is concise, it is not as clear in communicating the necessary meaning. Z is defined in ISO 8601 as the confusingly-named "UTC of day" and can only be applied to time-of-day representations under the standard, not dates alone. In other words, times are localizable in a way that dates are not; communicate accordingly.

While the standard doesn't allow writing yyyy-mm-ddZ, our dialect, which is GNU date(1), does allow that. Let's consider it a tasteful extension to the language. Similarly, we write GNU C, not ISO C, because standards aren't necessarily the most useful dialects.

alx@devuan:~$ date -d 2023-09-20Z
Wed Sep 20 02:00:00 CEST 2023

Everybody using a GNU system should understand such a date, or at least be prepared to ask date(1) to clarify it (or read some documentation, or ask someone, ...).

As an aside, I've dealt with account management for a small Kerberos realm. Expiration times are stored as full times there, of course, but if specified as only a date, are parsed to 23:59:59 UTC, e.g.:

% kadmin modify --expiration-time=2027-02-10 foo
% kadmin get foo | grep "Principal expires"
    Principal expires: 2027-02-10 23:59:59 UTC

One problem we had is spaces being meaningful in the format we were using, so we can't use a space in the date.

Ideally, one would further help avoid potential surprises by providing the full expanded timestamp in output to relevant commands, possibly even alongside a conversion to local time, but admittedly that's more of a stylistic point.

We tried the conversion to local time, and it proved to be more problematic than keeping it in UTC. Not having a time is really problematic with date translations.

As @eggert was happy with 2023-09-20Z, I'll take that option.

@hallyn , I know there's the danger that some old script might have a hardcoded parser for the dates, but I think that:

• Parsing the date from a program like this isn't critical information. I suspect it would be for printing it or something similarly cosmetic, but nothing very important. We can break those.
• If a server relies on that for something more important, I expect them to see the all the dates being wrong, so they can start asking questions, and we can help them.
• If they have such old scripts that might bit-rot so easily, they will probably have bigger problems.
• While that is theoretical, the current bugs are affecting real people.
• Maybe we're lucky and the scripts actually use date(1) to parse the date. And date(1) understands the trailing Z.

I would very much like a breaking change in the dates, by appending a hardcoded Z. I'd like to avoid more configuration.

I expect that the way to configure something would be to add a toggle to print the Z or not. But then, distros would eventually toggle that, and break the hypothetical scripts anyway.

What do you think?

[timparenti]

it is common practice to not print more significant digits than the error.

Be careful not to conflate error (lack of precision) with quantization (lack of accuracy). Error implies that the exact expiration time is somehow unknown or perhaps subject to some statistical distribution, but I don't get the sense that that's what's actually happening. We know — and can choose to report with high precision — exactly when the expiration will be, even if only certain times can be selected due to the accuracy with which the data is stored. That said:

Telling the user 2023-09-20Z gives accurate information, and gives the hint that we don't even look at a specific time; we round to the day.

I agree that this could indeed be a useful hint. But, if there is any intent to consider changing the quantization of expiration times in the future, using this construction as a stepping stone may further complicate future backwards-compatibility. (More on that in a bit.)

One problem we had is spaces being meaningful in the format we were using, so we can't use a space in the date.

If spaces are a concern, constructions such as yyyy-mm-ddT23:59:59Z are, of course, equivalent suggestions.

Not having a time is really problematic with date translations.

Correct. A time-of-day with a some sort of zonal component is the way to refer to any given instant within a day which, again, is what we're actually dealing with here. The date component serves to disambiguate which instance of that time-of-day you're referring to. As such, the notion of a "local date", while occasionally useful, is rather flawed as a concept; i.e., one can say "My Wednesday comprises part of your Wednesday and part of your Thursday", but there's just not enough information at this level to be more specific than that.

If you have an established and documented dialect (already in use elsewhere) where yyyy-mm-ddZ is an acceptable shorthand for a well-defined timestamp that matches the actual behavior of the system, then that's certainly an acceptable solution. Though I would caution against adopting it as an idiolect if it's just for this problem.

But it seems we may not quite be out of the woods just yet with respect to ambiguity:

alx@devuan:~$ date -d 2023-09-20Z
Wed Sep 20 02:00:00 CEST 2023

This conversion returns a timestamp based on 00:00:00 UTC on the specified date, not 24:00:00 UTC as we've previously discussed. Which is the actual observed behavior of the system?

Again, applying successive levels of naïve thinking to the overall user experience, one's first thought in setting an expiry date might be the end of that calendar date according to local time. One's next thought might then be the end of that date according to some other zone, which one might even learn is UTC. But if it's actually the beginning of that date? That would just seem like its own bug, as it raises the potential for both availability and security concerns in east-of-Greenwich zones, and potential availability issues increase to more than 24 hours ahead of the naïve case in west-of-Greenwich zones.

I appreciate that existing behavior is baked in, but if expirations do indeed take place at 00:00:00 UTC as your GNU date(1) example would imply, I would advise that appending something like T00Z would be required to at the very least avoid ambiguity as to which midnight is meant. (Though it seems you'd need T00:00Z to get GNU date(1) to parse it properly, at which point you might as well include the seconds.) My original recommendation to specify T23:59:59Z, T24Z, or similar is analogous for the other case, but for expiration times in general it is reasonable for 24:00:00 to be more tacitly assumed, whereas the same cannot be said for 00:00:00.

In any case, however this particular issue is ultimately resolved, it is likely time to start thinking with an eye toward future extensions and their eventual backwards-compatibility here. To my mind, starting to be explicit about the time-of-day is a better next step on that path than just appending a Z which doesn't actually resolve that much of the underlying ambiguity on its own. Especially if the fix here is already going to be a breaking change in some way, I would encourage longer-term thinking to avoid accidentally digging a more awkwardly-shaped hole for later.

As this isn't my project, I don't intend to press any further on the matter unless or until prompted. I'm happy to clarify any points above at any time. But, since it seemed you were reaching out for both immediate-term and longer-term advice, that's my "outsider" take on the most prudent paths forward from a timekeeping perspective. Best of luck.

[alejandro-colomar]

Hi @timparenti ,

Thanks for the thorough review of the ideas!

Most of it makes sense, but I have a few caveats.

Telling the user 2023-09-20Z gives accurate information, and gives the hint that we don't even look at a specific time; we round to the day.

I agree that this could indeed be a useful hint. But, if there is any intent to consider changing the quantization of expiration times in the future, using this construction as a stepping stone may further complicate future backwards-compatibility. (More on that in a bit.)

One problem we had is spaces being meaningful in the format we were using, so we can't use a space in the date.

If spaces are a concern, constructions such as yyyy-mm-ddT23:59:59Z are, of course, equivalent suggestions.

Yep, that could be it. However, as @eggert reminded last year, not all days were made equal, and we can't know for sure what will be the last second of each UTC day. Most days will end in 23:59:59, but some will be different. We might be able to add +1 to the days, and then subtract 1 second... That might work...

I think I prefer keeping it simple for now, and later reconsider if we can add the time-of-day information.

But it seems we may not quite be out of the woods just yet with respect to ambiguity:

alx@devuan:~$ date -d 2023-09-20Z
Wed Sep 20 02:00:00 CEST 2023

This conversion returns a timestamp based on 00:00:00 UTC on the specified date, not 24:00:00 UTC as we've previously discussed. Which is the actual observed behavior of the system?

Hmmm, yeah, there date(1) is guessing a time of day without more information. Let's say that's just a guess, and not aconsideration that a date without a time implies time 00:00:00. The same could be said of a local date (without Z) without a time, which is what we were using.

In any case, however this particular issue is ultimately resolved, it is likely time to start thinking with an eye toward future extensions and their eventual backwards-compatibility here. To my mind, starting to be explicit about the time-of-day is a better next step on that path than just appending a Z which doesn't actually resolve that much of the underlying ambiguity on its own. Especially if the fix here is already going to be a breaking change in some way, I would encourage longer-term thinking to avoid accidentally digging a more awkwardly-shaped hole for later.

While I'm suggesting a breaking change in how we print a date, I think a breaking change on how the date is stored would be significantly more problematic.

If anyone has ideas on how to improve that, it would be great to consider them. I just don't have ideas for that. I'm worried that we probably won't be able to fix that at all.

Anyway, if we break the printing once, we could break it twice if we eventually find a way to change the system information. Especially if we find a solution in the near future, so that the two breaks can be considered just an unstable period.

As this isn't my project, I don't intend to press any further on the matter unless or until prompted. I'm happy to clarify any points above at any time. But, since it seemed you were reaching out for both immediate-term and longer-term advice, that's my "outsider" take on the most prudent paths forward from a timekeeping perspective. Best of luck.

Thanks a lot for the feedback!

[timparenti]

While I'm suggesting a breaking change in how we print a date, I think a breaking change on how the date is stored would be significantly more problematic.

Certainly. My main point was just to consider, when making the nearer-term change, where that might eventually end up in the longer-term.

If anyone has ideas on how to improve that, it would be great to consider them. I just don't have ideas for that.

If current implementations would truncate stored figures at a radix point, ignoring a fractional portion, then that might lend itself to a mostly backwards-compatible way of sub-delimiting the existing field; e.g., five digits after a radix point could be used to refer to a decimal number of seconds after 00:00:00 UTC. Thus you could have ddddd.00000 to ddddd.86399, for ease of lexical sorting, assuming second-level precision is deemed sufficient.

Of course, if such a construction causes an error in existing implementations, then I don't see an advantaged way out: Most solutions I can think of in any case would involve introducing some as-yet invalid character as a signal to calculate expiration times in a new way rather than the old way. (But this is a conversation better suited for its own issue, should that be desired.)

[ikerexxe]

Thanks for raising this issue and for the insightful discussion so far. After reviewing the tickets and emails, I realize the complexity of this situation is greater than I initially anticipated. I agree with @timparenti that a long-term solution is ideal, but we should definitely consider that long-term vision even in our short-term fix.

It seems we're dealing with two distinct, though related, issues:

• User interaction: How the user sets and views the expiration date/time.
• System storage: How the expiration date/time is stored within the system.

Given the potential compatibility issues, I believe changing the system storage format (point 2) is not a feasible option at this time. Therefore, our focus should be on adjusting the user interaction (point 1) to work effectively within the existing storage constraints.

Currently, the user both sets and views the expiration date. I propose the following approach:

• Setting the date: The user should only be able to set the date. This aligns with how the date is ultimately stored in the system and avoids any ambiguity. And yeah, I think this should be in UTC format explicitly.

• Displaying the date: We need to clearly document the displayed expiration date's meaning. Specifically, we must specify whether the displayed date represents:

  • The beginning of the expiration day (UTC), meaning the count expires at the start of that UTC day.
  • The end of the expiration day (UTC), meaning the count expires at the end of that UTC day.

This clarification is crucial for user understanding and will prevent potential confusion about when the expiration actually occurs. We should clearly communicate this behavior in the user documentation and potentially within the UI itself.

[zeha]

but we should definitely consider that long-term vision even in our short-term fix.

I agree. I'll also note the downstream bug was files as a compatibility issue with ansible. So this affects existing other software.

[alejandro-colomar]

but we should definitely consider that long-term vision even in our short-term fix.

I agree. I'll also note the downstream bug was files as a compatibility issue with ansible. So this affects existing other software.

Would you mind sending a link to that problem with ansible? It would be useful to keep their problem in mind while writing a fix.

[alejandro-colomar]

Next week I'm on a trip to a C Committee meeting in Austria. I'll use the train time to start writing a solution to this.

If it's easy enough, I'll try to do something like this:

gmtime_r(&date, &tm);  // Get the UTC expiration date

// Find the last second of that date
tm.tm_mday++;
tm.tm_sec--;

mktime(&tm);  // Normalize

STRFTIME(s, "%FT%TZ", &tm);  // "yyyy-mm-ddThh:mm:ssZ"

If that proves to be easy, I'll do that.

If that proves to be more tricky than it seems, I'll do the following instead:

gmtime_r(&date, &tm);  // Get the UTC expiration date

STRFTIME(s, "%FZ", &tm);  // "yyyy-mm-ddZ"

(plus of course appropriate error handling.)

Cc: @timparenti , @eggert

[zeha]

but we should definitely consider that long-term vision even in our short-term fix.

I agree. I'll also note the downstream bug was files as a compatibility issue with ansible. So this affects existing other software.

Would you mind sending a link to that problem with ansible? It would be useful to keep their problem in mind while writing a fix.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1095430

@leegarrett can you provide details?

[leegarrett]

Would you mind sending a link to that problem with ansible? It would be useful to keep their problem in mind while writing a fix.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1095430

@leegarrett can you provide details?

Sure! The issue is in an ansible module called 'user', which is used for all kinds of user management. Every ansible module executed returns a status, and every module is supposed to be idempotent. Due to this bug, running a task that sets the expire date on a user consecutively will report it always as changed, see for example the upstream tests: https://github.com/ansible/ansible/blob/devel/test/integration/targets/user/tasks/test_expires.yml#L2-L20

A full reproducer is to install ansible, and run ansible-playbook user_expire.yml -K with user_expire.yml having following contents:

### test that fails on Debian sid 2025-02-07
- hosts: localhost
  become: yes
  become_method: sudo
  connection: local
  tasks:
    - name: Set user expiration
      user:
        name: ansibulluser
        state: present
        expires: 2529881062
      become: yes
      register: user_test_expires1

    - name: Set user expiration again to ensure no change is made
      user:
        name: ansibulluser
        state: present
        expires: 2529881062
      become: yes
      register: user_test_expires2

    - name: Ensure that account with expiration was created and did not change on subsequent run
      assert:
        that:
          - user_test_expires1 is changed
          - user_test_expires2 is not changed

It will prompt for the sudo password. Change to become_method: su if you prefer su instead. Note that this will create the user ansibulluser on the system.

This is the upstream test. The real world, user-visible result will be that a lot of admins will be scratching their heads why their ansible-maintained user management is apparently not taking effect.

Adding my 2ct: Looks like it's not as trivial bug as I thought, and I think the issue regarding date precision is a valid concern. Nonetheless I suggest keeping the semantics of usermod -e YYYY-MM-DD compatible with previous versions of usermod, as it's otherwise going to be a PITA for ansible to fix. We'd have to somehow detect the actual version of usermod (it doesn't have a --version parameter), and then have two different codepaths depending on which version is installed. I'd rather like to avoid that.

[alejandro-colomar]

@leegarrett

Sure! The issue is in an ansible module called 'user', which is used for all kinds of user management. Every ansible module executed returns a status, and every module is supposed to be idempotent. Due to this bug, running a task that sets the expire date on a user consecutively will report it always as changed, see for example the upstream tests: https://github.com/ansible/ansible/blob/devel/test/integration/targets/user/tasks/test_expires.yml#L2-L20

A full reproducer is to install ansible, and run ansible-playbook user_expire.yml -K with user_expire.yml having following contents:

### test that fails on Debian sid 2025-02-07
- hosts: localhost
  become: yes
  become_method: sudo
  connection: local
  tasks:
    - name: Set user expiration
      user:
        name: ansibulluser
        state: present
        expires: 2529881062
      become: yes
      register: user_test_expires1

    - name: Set user expiration again to ensure no change is made
      user:
        name: ansibulluser
        state: present
        expires: 2529881062
      become: yes
      register: user_test_expires2

    - name: Ensure that account with expiration was created and did not change on subsequent run
      assert:
        that:
          - user_test_expires1 is changed
          - user_test_expires2 is not changed

It will prompt for the sudo password. Change to become_method: su if you prefer su instead. Note that this will create the user ansibulluser on the system.

This is the upstream test. The real world, user-visible result will be that a lot of admins will be scratching their heads why their ansible-maintained user management is apparently not taking effect.

Adding my 2ct: Looks like it's not as trivial bug as I thought, and I think the issue regarding date precision is a valid concern. Nonetheless I suggest keeping the semantics of usermod -e YYYY-MM-DD compatible with previous versions of usermod, as it's otherwise going to be a PITA for ansible to fix. We'd have to somehow detect the actual version of usermod (it doesn't have a --version parameter), and then have two different codepaths depending on which version is installed. I'd rather like to avoid that.

Thanks!

My intention includes reverting the recent changes, which changed the shown date from UTC to localtime, thus printing a UTC date again. The format will be slightly different, but you should be able to handle both just fine.

Maybe I should split the fix into several steps, so that it's easier to check that each change is good for you.

I have these steps in mind:

• Revert the use of local time, back to UTC.
• Append a Z to the printed date, so that it's clear that it's UTC.
• Print the last second of the day together with the date.

I'll keep you CCd in every change, so that you can test them all.

[ikerexxe]

I have these steps in mind:

* Revert the use of local time, back to UTC.

* Append a Z to the printed date, so that it's clear that it's UTC.

* Print the last second of the day together with the date.

Let's add an additional step:

• Check the documentation and indicate that the expiration date has a minimum precision of a day, it's UTC timezone and the count expires at the start/end of that UTC day (not sure which of the two actually apply).

[zeha]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

[alejandro-colomar]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.

Or do you observe the new behavior to be incorrect?

[alejandro-colomar]

Maybe there are a combination of bugs, and fixing that one left the code in an unstable state where the other bug shows up?

[zeha]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.

Or do you observe the new behavior to be incorrect?

The new behaviour is absolutely incorrect, as it produces the wrong data and treats 1970-01-02 (supplied) and 1970-01-01 (stored) as the same date, and the manpage also doesn't reflect the new behavior.

[zeha]

It's possible the old implementation was wrong too, but maybe it only affected locales with high TZ offsets or something. It's less wrong than the new implementation, at least as it manifests itself in usermod.
I noticed the original motivation for the change was about chage. Maybe chage does something different from usermod.

[alejandro-colomar]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.
Or do you observe the new behavior to be incorrect?

The new behaviour is absolutely incorrect, as it produces the wrong data and treats 1970-01-02 (supplied) and 1970-01-01 (stored) as the same date, and the manpage also doesn't reflect the new behavior.

Please remember that the date that you pass is in local time, and the one printed is in UTC time (even if it doesn't say Z or UTC), like the one stored in file. Do you think the behavior is still incorrect if you use the UTC timezone?

[zeha]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.
Or do you observe the new behavior to be incorrect?

The new behaviour is absolutely incorrect, as it produces the wrong data and treats 1970-01-02 (supplied) and 1970-01-01 (stored) as the same date, and the manpage also doesn't reflect the new behavior.

Please remember that the date that you pass is in local time, and the one printed is in UTC time (even if it doesn't say Z or UTC), like the one stored in file. Do you think the behavior is still incorrect if you use the UTC timezone?

How is that useful behaviour. You cannot tell me with a straight face that this is intended behaviour:

% sudo ./src/usermod --expiredate 1970-01-02 irc
% sudo ./src/usermod --expiredate 1970-01-01 irc
usermod: no changes

man usermod(8) states nothing about localtime. If localtime cannot be preserved in the files, then the YYYY-MM-DD date string should probably not be treated as localtime in the first place.

With the commit reverted, it works correctly:

% sudo TZ=America/Los_Angeles ./src/usermod --expiredate 1970-01-01 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=America/Los_Angeles ./src/usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-01 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

... at least if you consider the input date to be in days since the epoch, which is likely what was intended.

[timparenti]

% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-01 irc

FWIW, Japan/Tokyo is not a valid TZ ID, so this would be equivalent to using Etc/UTC. @zeha For the sake of completeness, do you still see the same behavior with TZ=Asia/Tokyo?

[zeha]

% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-01 irc

FWIW, Japan/Tokyo is not a valid TZ ID, so this would be equivalent to using Etc/UTC. @zeha For the sake of completeness, do you still see the same behavior with TZ=Asia/Tokyo?

Ah I was scared this might happen. But anyway, yes it (= commit reverted) works fine:

% sudo TZ=Asia/Tokyo ./src/usermod --expiredate 1970-01-01 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=Asia/Tokyo ./src/usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

[alejandro-colomar]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.
Or do you observe the new behavior to be incorrect?

The new behaviour is absolutely incorrect, as it produces the wrong data and treats 1970-01-02 (supplied) and 1970-01-01 (stored) as the same date, and the manpage also doesn't reflect the new behavior.

Please remember that the date that you pass is in local time, and the one printed is in UTC time (even if it doesn't say Z or UTC), like the one stored in file. Do you think the behavior is still incorrect if you use the UTC timezone?

How is that useful behaviour. You cannot tell me with a straight face that this is intended behaviour:

% sudo ./src/usermod --expiredate 1970-01-02 irc
% sudo ./src/usermod --expiredate 1970-01-01 irc
usermod: no changes

It is not intended behavior. Ideally, I'd like to use the same library that GNU date(1) uses, so that you can pass whatever you want, for example "1970-01-02Z" to express UTC.

It is indeed a bug that you can't express UTC dates but we're storing UTC dates. But it's a different bug.

The main problem for fixing that bug is that our date parser is written in getdate.y, and none of us is comfortable editing that file. I did it once for fixing a bug, but ughhh.

And linking to (or rather compiling against) gnulib is a mess of its own (oh, and due to the license, we couldn't use it even if we wanted). I wish there was a normal library with an .so file that implemented a sane date parser which we could rely upon.

That bug is going to take time to fix. But I acknowledge the bug.

man usermod(8) states nothing about localtime.

In general, if a date doesn't express the timezone, local timezone is implied. For expressing UTC, you must say so. For example:

alx@devuan:~$ date -d 1970-01-02
Fri Jan  2 00:00:00 CET 1970
alx@devuan:~$ date -d 1970-01-02Z
Fri Jan  2 01:00:00 CET 1970

This is specified in some ISO standard about dates and times, IIRC.

If localtime cannot be preserved in the files, then the YYYY-MM-DD date string should probably not be treated as localtime in the first place.

I'll try to look into that code, and see what can be done. I'm not happy with that bug, but I suspect that I won't be able to fix it easily.

With the commit reverted, it works correctly:

% sudo TZ=America/Los_Angeles ./src/usermod --expiredate 1970-01-01 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=America/Los_Angeles ./src/usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-01 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::0:
% sudo TZ=Japan/Tokyo ./src/usermod --expiredate 1970-01-02 irc
% sudo getent shadow irc
irc:*:19812:0:99999:7::1:

... at least if you consider the input date to be in days since the epoch, which is likely what was intended.

[zeha]

In general, if a date doesn't express the timezone, local timezone is implied. For expressing UTC, you must say so.

Well, if shadow treated it (by means of the effective overall implementation) as UTC for the last 30 years or so, then UTC is probably what people want by now.

[alejandro-colomar]

In general, if a date doesn't express the timezone, local timezone is implied. For expressing UTC, you must say so.

Well, if shadow treated it (by means of the effective overall implementation) as UTC for the last 30 years or so, then UTC is probably what people want by now.

I'll check what I can do. Once I look into the code, I'll tell you if it's easy to reform it to mean UTC or not. I can't promise anything. :)

[alejandro-colomar]

@alejandro-colomar I've bisected this particular bug down to commit 1175932. You might want to consider reverting that.

Hmmmm, then I fear that you were relying on a bug, and the current behavior is the right one, I think.
Or do you observe the new behavior to be incorrect?

The new behaviour is absolutely incorrect, as it produces the wrong data and treats 1970-01-02 (supplied) and 1970-01-01 (stored) as the same date, and the manpage also doesn't reflect the new behavior.

Please remember that the date that you pass is in local time, and the one printed is in UTC time (even if it doesn't say Z or UTC), like the one stored in file. Do you think the behavior is still incorrect if you use the UTC timezone?

How is that useful behaviour. You cannot tell me with a straight face that this is intended behaviour:

% sudo ./src/usermod --expiredate 1970-01-02 irc
% sudo ./src/usermod --expiredate 1970-01-01 irc
usermod: no changes

It is not intended behavior. Ideally, I'd like to use the same library that GNU date(1) uses, so that you can pass whatever you want, for example "1970-01-02Z" to express UTC.

It is indeed a bug that you can't express UTC dates but we're storing UTC dates. But it's a different bug.

Actually, the code in getdate.y does look at timezone information. It should be able to interpret correctly if you specify it in the command line:

shadow/lib/getdate.y

Line 542 in 77eb67d

/* Military timezone table. */

The main problem for fixing that bug is that our date parser is written in getdate.y, and none of us is comfortable editing that file. I did it once for fixing a bug, but ughhh.

And linking to (or rather compiling against) gnulib is a mess of its own (oh, and due to the license, we couldn't use it even if we wanted). I wish there was a normal library with an .so file that implemented a sane date parser which we could rely upon.

That bug is going to take time to fix. But I acknowledge the bug.

man usermod(8) states nothing about localtime.

In general, if a date doesn't express the timezone, local timezone is implied. For expressing UTC, you must say so. For example:

alx@devuan:$ date -d 1970-01-02
Fri Jan 2 00:00:00 CET 1970
alx@devuan:$ date -d 1970-01-02Z
Fri Jan 2 01:00:00 CET 1970

This is specified in some ISO standard about dates and times, IIRC.

If localtime cannot be preserved in the files, then the YYYY-MM-DD date string should probably not be treated as localtime in the first place.

[leegarrett]

Please consider reverting the commit. I totally understand the rationale behind it, and I think it's a move in the right direction. However, it's been known "buggy" behaviour for the last 30 years, and a lot of tools depend on this behaviour. Such a user-visible breaking change needs more preparation and better communication in advance.

For my specific use case, ansible, it's impossible to support both the pre-1175932 and post-1175932 behaviour robustly. ansible runs on a wide range of OSes and releases, and AFAICS it's impossible to detect which version is running (there's no usermod --version for example which would ease detection). I also don't see a way to set the expire date which ends in the same result for both usermod versions.

[alejandro-colomar]

Please consider reverting the commit.

I'm researching options.

I totally understand the rationale behind it, and I think it's a move in the right direction. However, it's been known "buggy" behaviour for the last 30 years, and a lot of tools depend on this behaviour. Such a user-visible breaking change needs more preparation and better communication in advance.

We had users that had issues with the old buggy behavior, which is why we realized about it and fixed it. Here, some software relies on a bug, and other users are affected by it.

I'll try to make it work for both of you. I still don't know how. :-)

For my specific use case, ansible, it's impossible to support both the pre-1175932 and post-1175932 behaviour robustly. ansible runs on a wide range of OSes and releases, and AFAICS it's impossible to detect which version is running (there's no usermod --version for example which would ease detection). I also don't see a way to set the expire date which ends in the same result for both usermod versions.

You should be able to specify the timezone information together with the date, as in yyyy-mm-ddZ, but I'm not sure that works correctly.

If that worked correctly, I think you should be able to use that to work in both the old and the new behavior.

Would you mind helping me test with timezone information? I'm not sure why I'm not getting it to work properly.