Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ldap provider doesn't properly handle shadow access control #1714

Closed
sssd-bot opened this issue May 2, 2020 · 0 comments
Closed

ldap provider doesn't properly handle shadow access control #1714

sssd-bot opened this issue May 2, 2020 · 0 comments
Assignees
Labels
Closed: Fixed Issue was closed as fixed.

Comments

@sssd-bot
Copy link

sssd-bot commented May 2, 2020

Cloned from Pagure issue: https://pagure.io/SSSD/sssd/issue/672

  • Created at 2010-11-09 13:45:53 by ossman
  • Closed as Fixed
  • Assigned to sbose

I found a couple of discrepancies between pam_ldap and sssd in how shadow information is handled. Since rfc2307 doesn't specify semantics, I guess "do what pam_ldap does" should as a rule be followed.

Issues found:

  1. The fields aren't checked by default. You need to specify ldap_pwd_policy.

  2. Account expire is only checked if there is a registered password change.

  3. Access control is checked in the "auth" stage, rather than the proper "account" stage.

What this means is that accounts that were previously locked out from systems when using pam_ldap, will be allowed in using sssd. Even if you disagree about 1. being a problem, 2. still means that lots of previously locked accounts will be opened up.

An expired account is not the same thing as an expired password, and should not be dealt with in the same routine. PAM does after all separate the concept of authentication and access control, and for good reason.

That brings us to the more serious matter of when password authentication isn't used. In that case sssd has no mechanism to disable accounts. The code for access control in ldap_auth.c is this:

    case SSS_PAM_ACCT_MGMT:
    case SSS_PAM_SETCRED:
    case SSS_PAM_OPEN_SESSION:
    case SSS_PAM_CLOSE_SESSION:
        pd->pam_status = PAM_SUCCESS;
        dp_err = DP_ERR_OK;
        break;

IOW, let everything through, assuming that the password stage handled it. This is of course not true when using things like Kerberos, public key authentication or even other password based systems like a RADIUS OTP server.

I hope someone can have a look at this soon, and hopefully also backport patches to the versions of sssd already deployed in Fedora and the upcoming RHEL6.

Comments


Comment from ossman at 2010-11-09 14:04:08

Hmm... there also seems to be an extra safety net on "old" installations. As nss_ldap is exporting shadow information properly, pam_unix also checks that the account is valid and refuses access when it is expired. As sssd doesn't give out shadow data (at least not here), this extra check doesn't work.

Another note is that neither pam_ldap nor sssd is very administrator or user friendly in telling why access was denied. This should probably be logged so that we know why the users are complaining about failures to log in. :)


Comment from dpal at 2010-11-09 15:02:23

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.6.0


Comment from sbose at 2010-11-09 15:46:03

Your observation is correct. Currently sssd only use the shadow attribute to determine if a password is expired or not. At the moment LDAP based access control can only be done with the ldap_access_filter option.

I think access control based on the shadow attributes is a new feature for sssd.

Displaying useful information to the user about why the access was denied is not always that easy, because want want to avoid disclosing useful information for an attacker. We are currently adding user massages on a use-case basis. So if you have an issue where you think a more detailed message should be displayed it would be nice if you can open a ticket and explain the specific issue.


Comment from ossman at 2010-11-09 16:25:46

Replying to [comment:3 sbose]:

Displaying useful information to the user about why the access was denied is not always that easy, because want want to avoid disclosing useful information for an attacker. We are currently adding user massages on a use-case basis. So if you have an issue where you think a more detailed message should be displayed it would be nice if you can open a ticket and explain the specific issue.

I can understand that concern, and I won't object to it. But the administrator should be given that information at least, so that you can determine what the problem is without having to enable debugging.


Comment from sgallagh at 2010-11-19 13:14:42

Fields changed

milestone: SSSD 1.6.0 => SSSD 1.5.0


Comment from sgallagh at 2010-11-19 13:34:11

Fields changed

owner: somebody => sbose


Comment from sbose at 2010-12-01 09:27:56

Fields changed

status: new => assigned


Comment from sbose at 2010-12-07 09:15:56

Fixed by 32266b2

resolution: => fixed
status: assigned => closed


Comment from ossman at 2010-12-22 18:43:42

It seems sssd doesn't notice attributes being deleted, so an expired account that gets the expiration removed doesn't result in sssd letting the user in.

(Workaround is to set the expiration to something <= 0, but it seems that there is no good consensus on a value meaning "no expiration", meaning such an ldap setting might cause a fuss with other LDAP clients.)

coverity: =>


Comment from ossman at 2010-12-22 19:02:59

Problem in last comment moved to ticket #750.


Comment from ossman at 2010-12-22 19:22:51

With the exception of the previous note, the implementation seems to work fine. I've tested dates in the past, in the future, the same day and the day just before. Also tested public key auth without any issues.


Comment from dpal at 2012-01-19 02:58:51

Fields changed

rhbz: => 0


Comment from ossman at 2017-02-24 14:39:52

Metadata Update from @ossman:

  • Issue assigned to sbose
  • Issue set to the milestone: SSSD 1.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed: Fixed Issue was closed as fixed.
Projects
None yet
Development

No branches or pull requests

2 participants