Bug 973428

I did some investigation and this is what I have found out:

 1. If local user password is expired using "passwd -e", sshd forces the user to change their password on login.

 2. If IPA user password is expired using kadmin.local, sshd allows login. I think that Kerberos principal and password expiration must be checked in pam_sss during PAM account phase in order to fix this.

(In reply to Jan Cholasta from comment #1)
>  2. If IPA user password is expired using kadmin.local, sshd allows login. I
> think that Kerberos principal and password expiration must be checked in
> pam_sss during PAM account phase in order to fix this.

We already do check the account lockout during IPA's account phase. I wonder if the functionality is working correctly even along with SSH integration. I'll ping you on IRC to investigate.

As this issue is closely related to #972451, kindly see comment #3 there as a follow up to the discussion.

I suspect you meant bug #973451 comment #3.

Bug #973451 is a non-issue.  Doing some testing here, on a local user, if I expire their account I cannot login via password _or_ via ssh pubkey.

Aug 19 15:48:39 odxcent6 sshd[30775]: fatal: Access denied for user foo by PAM account configuration

This message is reported for both authentication types.

So IPA must do the same -- it is not enough for it to prevent authentication via passwords and allow ssh pubkey authentication.

Do we have any plans/progress regarding a patch for this?

Hmmm... I just had a thought.  I was using chage to expire the _account_, not the password (they are not necessarily the same thing).

What happens in either case with IPA?  If you expire an _account_, does it permit ssh pubkey logins?  And then what happens if a password is expired (passed the maxage threshold and requires a user to set it)?  Does it deny password-based login but allow ssh key-based login?

If the _password_ is expired, it might not be bad to allow pubkey login (it's not actually using the expired password), however if the _account_ is expired and pubkey login is permitted, then this is definitely bad.

(In reply to Vincent Danen from comment #6)
> Hmmm... I just had a thought.  I was using chage to expire the _account_,
> not the password (they are not necessarily the same thing).
> 
> What happens in either case with IPA?  If you expire an _account_, does it
> permit ssh pubkey logins?  
> And then what happens if a password is expired
> (passed the maxage threshold and requires a user to set it)?  Does it deny
> password-based login but allow ssh key-based login?
> 
> If the _password_ is expired, it might not be bad to allow pubkey login
> (it's not actually using the expired password), however if the _account_ is
> expired and pubkey login is permitted, then this is definitely bad.


I think we need to define what exactly is account expiration and how it happens.

The _password_ can be expired as a result of password policy. This is reflected by the krbPasswordExpiration in the user entry. Even if the password is expired, the SSSD would still allow access using SSH keys.

I haven't found any way for the admin to easily set the _account_ expiration from the IPA CLI. Maybe the IPA developers CCed to this bug would show me some. 

But it's possible to expire the principal using the kadmin tool. That would set the krbPrincipalExpiration attribute of the user entry. The SSSD would still allow access to a user with an expired principal with the current code. Extending the access control to also check krbPrincipalExpiration is trivial actually (see attached patch) but the question is if we want to do it? If there is no way to set the attribute short of kadmin..

Created attachment 788405 [details]
Check the principal expiration attribute during account phase, too.

IPA doesn't provide a mechanism to manage krbPasswordExpiration yet except through generic means (functionally equivalent to ldapmodify). So if you know what this attribute can/should do you can set it, but it isn't a feature we advertise.

You could set it with:

$ ipa user-mod --setattr krbPasswordExpiration=<date> testuser

We would probably want equivalent support for LDAP binds in this case as well which would mean a change on the IPA side too. We do check this value when changing a password, but not when doing an LDAP bind AFAICT. It doesn't look like the KDB driver enforces this either (except in the case of password changes).

Right and the krbPrincipalExpiration falls into the same bucket (at least when it comes to how the admin can set it), so I wonder if it's worth checking on the client.

Oh, crap, I meant krbPrincipalExpiration in #9. We DO handle krbPasswordExpiration.

I agree that this should be handled on the server side. I guess the question is: does our lack of enforcing this attribute constitute a CVE?

(In reply to Rob Crittenden from comment #11)
> I agree that this should be handled on the server side. 

Are you sure it should be handled on the server side? I mean, the server wouldn't get contacted at all (except for refreshing the user entry) at all in case the user logs in with SSH keys. I would argue that the client makes more sense. To me, it is analogous to the nsAccountLock attribute (except it's a timestamp).

(In reply to Jakub Hrozek from comment #12)
> Are you sure it should be handled on the server side?

I think we'll need a split decision and enforce in both. You're right about the ssh case, but right now we don't, for example, prevent LDAP binds. It does seem to be correct enforced by the KDC (so kinit fails, for example).

Also, I was wrong about using user-mod to set krbPrincipalExpiration. We don't allow writing that attribute. So given this, it seems like we are sneaking into new feature territory.

(In reply to Rob Crittenden from comment #13)
> (In reply to Jakub Hrozek from comment #12)
> > Are you sure it should be handled on the server side?
> 
> I think we'll need a split decision and enforce in both. You're right about
> the ssh case, but right now we don't, for example, prevent LDAP binds. It
> does seem to be correct enforced by the KDC (so kinit fails, for example).
> 

Ah, you're right.

> Also, I was wrong about using user-mod to set krbPrincipalExpiration. We
> don't allow writing that attribute. So given this, it seems like we are
> sneaking into new feature territory.

I agree. Given how obscure it is to set the attribute, I don't think this necessarily needs to be treated as a CVE, although I'll let SRT decide..

Update from Citrix:

http://docs.fedoraproject.org/en-US/Fedora/17/html/FreeIPA_Guide/user-keys.html

In own FreeIPA Guide, we declare the following 'problems' with the standard SSH keys implementation:

There are a couple of problems with this system:

* SSH keys have to be distributed manually and separately to all machines in an environment.
* Administrators have to approve user keys to add them to the configuration, but it is difficult to verify either the user or key issuer properly, which can create security problems.

But I contest the argument you presented earlier today:

"IPA isn't doing anything different than the standard OpenSSH + SSH Keys, with respect to allowing the keys to authenticate the user even after password expiration"

The risk scope is significantly greater in the context of IPA.

Vanilla OpenSSH + SSH Keys requires Administrator approval and the keys have to be manually placed on relevant systems

IPA  requires only the users credentials to upload an ssh key that is potentially applicable to -all- systems that the user is permitted to login to via HBAC.

Given the difference in wide scope, I do think that there is sufficient enough of concern to warrant a control on the IPA side to treat the key as expired if the account password is expired.

While this isn't a 1-1 analog to the account vs password expiration, I do think its worth discussing further.

(In reply to Rob Crittenden from comment #15)
> Vanilla OpenSSH + SSH Keys requires Administrator approval and the keys have
> to be manually placed on relevant systems

This is not default behavior. Administrator approval is not required on systems which use default OpenSSH setup (authorized keys are read from ~/.ssh/authorized_keys) and which you already have password access to.

> IPA  requires only the users credentials to upload an ssh key that is
> potentially applicable to -all- systems that the user is permitted to login
> to via HBAC.

I think it is worth noting that user uploads can be disabled by deleting the "Users can manage their own SSH public keys" self-service permission. You can do exactly that, should you need greater control over the keys and want administrators to approve them first.

(In reply to Jan Cholasta from comment #16)
> (In reply to Rob Crittenden from comment #15)
> > Vanilla OpenSSH + SSH Keys requires Administrator approval and the keys have
> > to be manually placed on relevant systems
> 
> This is not default behavior. Administrator approval is not required on
> systems which use default OpenSSH setup (authorized keys are read from
> ~/.ssh/authorized_keys) and which you already have password access to.

Do you know what do the "Administrator approval" actually refer to?  Is that used for some custom configuration that places authorized_keys out of user's home and make it only writable by root?

I would think it is something like that, yes.

Sorry I have not looked at this in a long time.

Is the patch attached to this bug suitable to fix this issue?  It still looks like there is some waffling back and forth about whether or not this is a security issue (that should be treated as a flaw (with CVE) vs hardening (without a CVE)).  Any consensus either way?

It sounds to me like this should be considered a flaw.  It's likely a low/moderate-impact flaw, but a flaw nonetheless.

Thoughts?  And again, my apologies for seriously dropping the ball here.

(In reply to Vincent Danen from comment #19)
> Sorry I have not looked at this in a long time.
> 
> Is the patch attached to this bug suitable to fix this issue?  It still
> looks like there is some waffling back and forth about whether or not this
> is a security issue (that should be treated as a flaw (with CVE) vs
> hardening (without a CVE)).  Any consensus either way?
> 

Partially. The patch only fixes the client side, Rob thought it was more appropriate to have a complete patch on the server side.

So the patch does solve the SSH case, but not e.g. LDAP binds (see comments #11-#14)

I think that whether this is a CVE or not comes down to the current implementation of IPA password policy. Is the purpose to require rotating passwords or is it to limit account availability?

Since from an IPA password policy view we are talking about krbPasswordExpiration rather than krbPrincipalExpiration I believe we do the former only (at present). We have RFEs to add the ability to limit account times for things like contractors.

So an expired password does not necessarily deny access, it just requires user intervention. Therefore denying access via SSH isn't necessary.

Rob, are you saying that you feel this is _NOT_ a flaw then?  But that this kind of blocking is currently not expected and may be implemented along with the implementation of an RFE for expiring _accounts_ rather than expiring _passwords_?

Correct. I discussed this with Simo, Sumit, Alexander, Martin and Jakub and we don't believe that one password mechanism should affect another. Passwords are changed frequently because they are relatively weak. ssh keys generally don't have the same problems so are rotated less frequently (if at all in practice).

So from our perspective, an expired password just says you can't log in using this password, not that the account is completely locked out.

Really, the same holds true for Kerberos. Imagine you get a ticket moments before the password expires. You'd be able to use that ticket to log into systems for the remaining validity time, even though a password authentication would require a reset.

We will have to clearly document this behavior so people will know what to expect.

Thanks, Rob.  This works for me.  Clearer documentation would probably be welcome, but I'll leave that to you guys.

So I will close this as NOTABUG then.

Thanks!