Bug 2129113 - Incorrect password expiration handling [fedora-rawhide]
Summary: Incorrect password expiration handling [fedora-rawhide]
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: krb5
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Julien Rische
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 2121099
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-09-22 14:32 UTC by Julien Rische
Modified: 2023-01-13 14:43 UTC (History)
13 users (show)

Fixed In Version: krb5-1.20.1-3.fc38
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 2121099
Environment:
Last Closed: 2023-01-13 14:43:51 UTC
Type: Bug


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Fedora Package Sources krb5 pull-request 27 0 None None None 2022-11-23 18:51:11 UTC
Red Hat Issue Tracker FREEIPA-8800 0 None None None 2022-09-22 14:45:07 UTC

Description Julien Rische 2022-09-22 14:32:12 UTC
+++ 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

Comment 1 Fedora Update System 2022-12-01 15:02:28 UTC
FEDORA-2022-8050ab2c35 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-8050ab2c35

Comment 2 Fedora Update System 2022-12-01 17:38:00 UTC
FEDORA-2022-311128dd7e has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-311128dd7e

Comment 3 Fedora Update System 2022-12-07 13:29:07 UTC
FEDORA-2022-311128dd7e has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.