#48833 389 showing inconsistent values for shadowMax and shadowWarning in 1.3.5.1
Closed: wontfix None Opened 8 years ago by hheinze.

We have a userRoot database with users whose credentials are manage outside LDAP and regularly uploaded with ldapmodify. We are using LDAP for password authentication on Linux servers.

We have are ensuring in our loaders the following values for each user:

shadowMax: 99999
shadowWarning: 7

After upgrading from 1.3.4.9 to 1.3.5.1 we see users being prompted to update their passwords immediately.

Inspecting the shadow attributes of a user (myself) shows the following:

shadowWarning: 1
shadowMax: 300
shadowLastChange: 12795
objectClass: shadowAccount
shadowMin: 0
shadowExpire: 17232

Exporting userRoot to an LDIF file, however, shows the following (correct, expected) values:

sudo sed -r '/^dn: uid=helmut/,/^$/!d' /opt/ldap/exports/people_ldap_export.ldif | grep shadow
shadowMax: 99999
objectClass: shadowAccount
shadowLastChange: 12795
shadowWarning: 7


Further debugging suggests that the values are derived from

dn: cn=config, attributes passwordMaxAge resp. passwordWarning.

These values given in days are incorrectly divided by (seconds per day); ie when supplying 604800 (7 days * 24 * 60 *60) I am presented with a value of 7. There's possibly an error of such type in new code in ldap/servers/slapd/pw.c.

Consider the follow patch, tested only superficially and without understanding the bigger picture:

--- current/ldap/servers/slapd/pw.c 2016-03-24 08:13:11.000000000 +1100
+++ pw.c 2016-05-12 16:37:01.785962000 +1000
@@ -2903,7 +2903,7 @@

 /* shadowMax - the maximum number of days for which the user password remains valid. */
 if (pwpolicy->pw_maxage > 0) {
  • shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY;
  • shadowval = pwpolicy->pw_maxage; // hheinze: do not dived by _SEC_PER_DAY ;
    exptime = time_plus_sec(current_time(), pwpolicy->pw_maxage);
    } else {
    shadowval = 99999;

Apologies for supplying this in drips and drops -- but the shadowMax issue took priority.

Find below a patch that also fixes the shadowWarning behaviour (same issue, should not divide by _SEC_PER_DAY).
These might not be the only issues in pw.c but I leave it to someone more familiar than I to review the code.

Cheers,
Helmut

{{{
--- 389-ds-base-1.3.5.1/ldap/servers/slapd/pw.c 2016-03-24 08:13:11.000000000 +1100
+++ ldap/servers/slapd//pw.c 2016-05-13 09:21:54.469930000 +1000
@@ -2903,7 +2903,7 @@

 /* shadowMax - the maximum number of days for which the user password remains valid. */
 if (pwpolicy->pw_maxage > 0) {
  • shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY;
  • shadowval = pwpolicy->pw_maxage;
    exptime = time_plus_sec(current_time(), pwpolicy->pw_maxage);
    } else {
    shadowval = 99999;
    @@ -2923,7 +2923,8 @@

    / shadowWarning - the number of days of advance warning given to the user before the user password expires. /
    if (pwpolicy->pw_warning > 0) {
    - shadowval = pwpolicy->pw_warning / _SEC_PER_DAY;
    + shadowval = pwpolicy->pw_warning;
    + shadowval = 7;
    } else {
    shadowval = 0;
    }

}}}

Thank for the report, hheinze.

I think the problem is rather the shadow values are updated based upon the password policy.

If they are already assigned, the Directory Server should not touch them.

If you have a test environment, could you please apply the preliminary patch 0001-Ticket-48833-389-showing-inconsistent-values-for-sha.patch​ to the master branch and test it? If it works as expected, I'm going to take the next step.

Thanks for your help.

Hi there,

Tested the patch with respect to the handling of shadowMax and shadowWarning. Works as specified.

We are now being presented consistently with the values configured directly for entries in userRoot. Changes to passwordMax and passwordWarning remain without any impact in these cases.

Looking good -- at least to us.

Thanks
Helmut

Replying to [comment:6 hheinze]:

Hi there,

Tested the patch with respect to the handling of shadowMax and shadowWarning. Works as specified.

We are now being presented consistently with the values configured directly for entries in userRoot. Changes to passwordMax and passwordWarning remain without any impact in these cases.

Looking good -- at least to us.

Thanks
Helmut
Hi Helmut,

Thanks so much for testing the patch. I'm going to push it and rebuild the new version of 389-ds-base as soon as possible.
Best regards,
--noriko

The problem here is we now have a conflict between shadow and password policy. If a user is setting the value of shadow to conflict with pwpol, they may see odd results.

I was under the impression the whole point of automatically setting the shadow values was to prevent these inconsistencies. So that a posixAccount's password warnings and such were in sync with the directory servers.

I feel this change is really only appropriate where there is no password policy / expiry in place at all. Otherwise, this kind of actually breaks expectations IMO.

I would rather leave the code as it is, and fix the time issues that Helmut noticed so that we aren't erroneously dividing by seconds_per_day.

I would prefer to discuss this before I ack / nack the change, sorry.

Talking of surprises ...

As a large organisation with a user base of ~ 90,000 accounts we manage account expiry outside the 389 directory server. We use LDAP as one distribution channel among others and prefer to be able to control all attributes directly.

When upgrading to 1.3.5.1 we were faced with the confusing issue of a discrepancy between the uploaded values of shadowMax=99,999 being equal with values extracted out of an LDIF dump also being 99,999 as opposed to the projected value as derived, without our knowledge, from cn=config passwordMax.

I see that both arguments have their valid points -- but if there are no standards otherwise we would prefer to be able to see values reflected as we feed them into LDAP through our loaders.

Cheers,
Helmut

Helmut,

I thought the automatic update on the shadow attribute values is more problematic for you, but it may not be?

Could you tell us your use case of the password policy? Are you using it in the global manner of a fine-grained? Each has an ability to set password max age, warning, etc. They store the values in seconds. Your shadowMax == 300 is from pw_maxage / (24 * 60 * 60). So, if you don't mind, could you please set your password max age to 99999 * 24 * 60 * 60 == 8639913600, and see how it behaves?

Thanks.

Hi Noriko,

To clarify the confusing value of 300: The shadowMax of 300 resulted from increasing the default value of passwordMax from 99999 to 25920000 during an eventually aborted attempt of upgrading to 1.3.5.1.
We were wildly stabbing around in an desperate attempt to get the logon to UNIX servers going:

25920000 == 300 * 24 * 60 * 60.

When I try much bigger numbers (as I have during our failed upgrade attempt) I get an error, maybe due to an LDAP range limitation. Doing the test with 8639913600:

    ldap_modify: Operations error (1)
            additional info: passwordMaxAge: password maximum age "8639913600" is invalid. 
    modifying entry "cn=config"

As to our password policy: we are not managing the password freshness through LDAP at all (users cannot interactively update their passwords through LDAP). For us it's just relevant that we are able to make sure that passwords stored in LDAP are guaranteed not to be rejected as out-of-date. Our loaders take care of adding/changing/removing password-related information from sources outside LDAP.

I have tested my own quick-fix patch and your more elaborate one. Both would work for us, however I prefer your patch as it allows us to control individual accounts independently should if we modify our way of using LDAP in future. So my vote goes to your solution which we have been testing now for a week in our UAT environment.

Cheers,
Helmut

Thanks for your clear explanation. Sorry, I forgot the 32 bit integer value limitation... :(

passwordMaxAge (Password Maximum Age)
Valid Range 1 to the maximum 32 bit integer value (2147483647) in seconds

The largest value 2147483647 is only 24855 days.

Probably, we could change the type of pw_maxage (and pw_minage and pw_pw_warning, as well?) to long long?
long pw_maxage;

I your suggestion to switch from days to seconds?

I would prefer to leave current meaning of passwordMax etc as is: as the number of days, not seconds. That is how it's documented in various places. Not only would change cause a big upset but handling very large numbers easily leads to errors such as accidentally dropping one digit.

I you consider allowing values to be specified down to a seconds I would consider introducing another attribute that determines the unit of password.... and shadow.... . For backward compatibility it should be set to 'days'.

Hold on... They ARE in seconds. We have never stored a value in days...

I would prefer to leave current meaning of passwordMax etc as is: as the number of days, not seconds

https://access.redhat.com/documentation/en-US/Red_Hat_Directory_Server/10/html/Configuration_Command_and_File_Reference/Core_Server_Configuration_Reference.html#cnconfig-passwordMaxAge_Password_Maximum_Age
3.1.1.146. passwordMaxAge (Password Maximum Age)
Indicates the number of '''seconds''' after which user passwords expire. To use this attribute, password expiration has to be enabled using the passwordExp attribute.

passwordMax in the password policy is in seconds.
shadowMax in the shadow account is in days.

That's my understanding... Is this incorrect?

You may be right -- so be it seconds for password (we only started looking at the password attributes when our upgrade went haywire, without any knowledge of standards).

Defining the password attributes in seconds is fine, we can even live with the limitation to a 32 bit integer, as long as it only is applied where shadow... attributes that have not been set individually (just along the lines of your patch)

Helmut

Thank you sooooo much for reviewing the patch, Mark!

This patch [1] is pushed to master:

1b8baa8..78f730a master -> master
commit 78f730a

[1] 0001-Ticket-48833-389-showing-inconsistent-values-for-sha.2.patch​ added
git patch file (master) -- second proposal

Metadata Update from @nhosoi:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.3.5.5

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1893

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Log in to comment on this ticket.

Metadata