[Freeipa-devel] [PATCH 0026] Prevent integer overflow when setting krbPasswordExpiration

Simo Sorce simo at redhat.com
Wed Jan 16 13:47:01 UTC 2013


On Wed, 2013-01-16 at 12:52 +0100, Tomas Babej wrote:
> On 01/15/2013 11:55 PM, Simo Sorce wrote:
> > On Tue, 2013-01-15 at 17:36 -0500, Dmitri Pal wrote:
> >> On 01/15/2013 03:59 PM, Simo Sorce wrote:
> >>> On Tue, 2013-01-15 at 15:53 -0500, Rob Crittenden wrote:
> >>>> Dmitri Pal wrote:
> >>>>> On 01/15/2013 08:48 AM, Simo Sorce wrote:
> >>>>>> On Mon, 2013-01-14 at 16:46 +0100, Tomas Babej wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> Since in Kerberos V5 are used 32-bit unix timestamps, setting
> >>>>>>> maxlife in pwpolicy to values such as 9999 days would cause
> >>>>>>> integer overflow in krbPasswordExpiration attribute.
> >>>>>>>
> >>>>>>> This would result into unpredictable behaviour such as users
> >>>>>>> not being able to log in after password expiration if password
> >>>>>>> policy was changed (#3114) or new users not being able to log
> >>>>>>> in at all (#3312).
> >>>>>>>
> >>>>>>> https://fedorahosted.org/freeipa/ticket/3312
> >>>>>>> https://fedorahosted.org/freeipa/ticket/3114
> >>>>>> Given that we control the KDC LDAP driver I think we should not limit
> >>>>>> the time in LDAP but rather 'fix-it-up' for the KDC in the DAL driver.
> >>>>> Fix how? Truncate to max in the driver itself if it was entered beyond max?
> >>>>> Shouldn't we also prevent entering the invalid value into the attribute?
> >>>>>
> >>>> I've been mulling the same question for a while. Why would we want to
> >>>> let bad data get into the directory?
> >>> It is not bad data and the attribute holds a Generalize time date.
> >>>
> >>> The data is valid it's the MIT code that has a limitation in parsing it.
> >>>
> >>> Greg tells me he plans supporting additional time by using the
> >>> 'negative' part of the integer to represent the years beyond 2038.
> >>>
> >>> So we should represent data in the directory correctly, which means
> >>> whtever date in the future and only chop it when feeding MIT libraries
> >>> until they support the additional range at which time we will change and
> >>> chop further in the future (around 2067 or so).
> >>>
> >>> If we chopped early in the directory we'd not be able to properly
> >>> represent/change rapresentation later when MIT libs gain additional
> >>> range capabilities.
> >>>
> >>> Simo.
> >>>
> >> We would have to change our code either way and the amount of change
> >> will be similar so does it really matter?
> > Yes it really matters IMO.
> >
> > Simo.
> >
> Updated patch attached.

This part looks ok but I think you also need to properly set
krb5_db_entry-> {expiration, pw_expiration, last_success, last_failed}
in ipadb_parse_ldap_entry()

Perhaps the best way is to introduce a new function
ipadb_ldap_attr_to_krb5_timestamp() in ipa_kdb_common.c so that you do
all the overflow checkings once.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the Freeipa-devel mailing list