Bug 2129113

+++ This bug was initially created as a clone of Bug #2121099 +++

Description of problem:
Trying to prevent admin password to expire I set it to 2099-12-13 23:59:59 (with ansible-freeipa/ipauser). This date is now displayed on the Web UI but when doing kinit this warning is printed:

# kinit admin                            
Password for admin.COM:   
Warning: Your password will expire in less than one hour on Mon Dec 14 01:59:59 2099

If the given date is too distant in the future then I would have expected it to be rejected. However, this does not look like a 2038 issue as e.g. 2089-12-31 23:59:59 does not cause such a warning.

Version-Release number of selected component (if applicable):
ipa-common-4.9.8-7.module+el8.6.0+14337+19b76db2.noarch

--- Additional comment from Marko Myllynen on 2022-08-24 13:05:13 UTC ---

It is also unclear what is the timezone in use here, perhaps that should be filed as another bug? Consider a case of 4 IdM servers where two of them are on different timezone than the others. What is the UTC time when the password will actually expire? Thanks.

--- Additional comment from Florence Blanc-Renaud on 2022-08-24 17:08:36 UTC ---

The issue is easily reproduced by setting the expiration date using ipa user-mod --password-expiration=DATE.

(In reply to Marko Myllynen from comment #0)
> If the given date is too distant in the future then I would have expected it
> to be rejected. However, this does not look like a 2038 issue as e.g.
> 2089-12-31 23:59:59 does not cause such a warning.

It is a similar issue because the check is evaluating a delta between 2 timestamps, now and the expiration date, and if the delta overflows the buffer we have unpredictable behavior. The delta is also represented as a signed 32 bit integer, meaning the buffer overflow happens if the 2 dates are distant of more than ~68 years, which fits nicely with the behavior described here (2022 + 68 = 2090 is the pivot point).


Relevant code:
--------------
https://github.com/krb5/krb5/blob/70f61d417261ca17efe3d60d180033bea2da60b0/src/lib/krb5/krb/get_in_tkt.c#L1567-L1571
    delta = ts_delta(pw_exp, now);
    if (delta < 3600) {
        snprintf(banner, sizeof(banner),
                 _("Warning: Your password will expire in less than one hour "
                   "on %s"), ts);

and the definition of ts_delta:
https://github.com/krb5/krb5/blob/70f61d417261ca17efe3d60d180033bea2da60b0/src/include/k5-int.h#L2319-L2325
/* Return the delta between two timestamps (a - b) as a signed 32-bit value,
 * without relying on undefined behavior. */
static inline krb5_deltat
ts_delta(krb5_timestamp a, krb5_timestamp b)
{
    return (krb5_deltat)((uint32_t)a - (uint32_t)b);
}


krb5_deltat is defined in https://github.com/krb5/krb5/blob/70f61d417261ca17efe3d60d180033bea2da60b0/src/include/krb5/krb5.hin:

typedef int32_t krb5_int32;
typedef krb5_int32      krb5_deltat;


Moving the issue to krb5 component.

--- Additional comment from Julien Rische on 2022-09-07 12:16:16 UTC ---

I guess either we switch the type of krb5_deltat from int32_t to int64_t, or we add an extra test to detect integer wrapping before the delta is calculated.

I think the first option is better, but it may have some implications in the rest of the code. We will discuss this issue upstream.

--- Additional comment from Julien Rische on 2022-09-09 16:22:30 UTC ---

The krb5_timestamp and krb5_deltat types are widely used in the MIT krb5 codebase. Their underlying type is int32_t, but krb5_timestamp is actually used as an unsigned 32-bit integer to support later dates in the future. Changing the underlying type would definitely require a lot of changes, including in the ABI, so I simply opened a PR to cast the delta as uint32_t:

https://github.com/krb5/krb5/pull/1266