Bug 1487697 (CVE-2017-12169)

Acknowledgments:

Name: Rob Crittenden (Red Hat)

I reproduced the problem and indeed it is a pb of badly (not strict enough) ACIs in stage user container.

I identified a fix but have not yet prepare a fix for it. The fix is simple and should be ready early next week.

Created attachment 1331008 [details]
Final fix

The Tentative fix was missing a 'add' directive in 30-provisioning.update

Comment on attachment 1331008 [details]
Final fix

NACK, doesn't handle upgrades properly:

1. This doesn't replace any existing ACIs. You can fix part of that with something like:

+# Update existing ACI that granted too broad permissions for reading
+# staged-users attributes
+replace:aci: (targetattr="userPassword || krbPrincipalKey")(version 3
.0; acl "Search existence of password and kerberos keys"; allow(read, search) us
erdn = "ldap:///uid=admin,cn=users,cn=accounts,$SUFFIX";)::(targetattr="userPass
word || krbPrincipalKey")(version 3.0; acl "Search existence of password and ker
beros keys"; allow(search) groupdn = "ldap:///cn=System: Read Stage Users,cn=per
missions,cn=pbac,$SUFFIX";)

2. The system permission isn't being updated properly for some reason:

$ ipa permission-show "System: Read Stage Users"
  Permission name: System: Read Stage Users
  Granted rights: search, compare, read
  Effective attributes: *, cn, createtimestamp, description, displayname,
                        entryusn, gecos, gidnumber, givenname, homedirectory,
                        initials, ipantsecurityidentifier, loginshell, manager,
                        modifytimestamp, objectclass, sn, title, uid, uidnumber
  Default attributes: *, gidnumber, displayname, entryusn, objectclass,
                      uidnumber, modifytimestamp, givenname, initials, title,
                      gecos, description, ipantsecurityidentifier, loginshell,
                      createtimestamp, sn, uid, homedirectory, manager, cn
  Bind rule type: permission
  Subtree: cn=staged users,cn=accounts,cn=provisioning,dc=greyoak,dc=com
  Extra target filter: (objectclass=*)
  Target DN: uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=greyoak,dc=com
  Permission flags: SYSTEM, V2, MANAGED
  Granted to Privilege: Stage User Administrators
  Indirect Member of roles: User Administrator

* is still in the attributes list.

The other changed ACI does seem to properly handle not disclosing keys.

(In reply to Adam Mariš from comment #0)
> An information disclosure vulnerability has been found in ipa allowing
> attacker to retrieve user password hash when utilizing 'System: Read Stage
> Users' permission.

Thierry CCed me on this CVE. I am not convinced that this is a valid CVE.

As you can see in the FreeIPA User Life-Cycle feature design, the Stage User password is treated in a special way:
http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Staging_entry
When the Staged User is being Activated, the entry is actually deleted and added again to an active IdM container, which means that the Stage User Administrator performing the Activation must have access also to userPassword attribute. Otherwise, the activated user would not have the provisioned password set properly.

See following section for details:
http://www.freeipa.org/page/V4/User_Life-Cycle_Management#MODRDN_vs._ADD-DEL

A change in the way how user is Activated would require a (too) costly architectural change, given this is an escalated privilege that needs to be explicitly (and wisely) assigned first, before it can show this password.

I strongly disagree. Password hashes should never be visible. That is one of the key features of IPA.

Just because this may be hard to fix doesn't mean it shouldn't be done.

Password hashes are indeed not visible for active standard users - we are in agreement there.

The reported behavior, however, only affects provisioned inactive users (a.k.a. Stage User) and only exposes the password to an established Stage User Administrator, not to random authenticated user. The actual exploitability of the exposed password to Stage User Administrator is questionable, because Stage User Administrator would typically already be quite powerful and have permissions to create a Staged User and Activating it (thus creating normal user account). Also note that the exposed password that is provisioned for the Staged User should be expired during Activation (IIRC), which means that the active user will be forced to change on the first login.

So considering above, the feature is now behaving according to an approved upstream design, released in FreIPA 4.2.0:
http://www.freeipa.org/page/Releases/4.2.0
If you want to change it, it would require re-design and new architecture to fix this exposure.

So given unknown exploitability of this behavior and high cost for fixing it, I would consider it rather as "low" priority, "low" severity CVE (if not closing it as NOTABUG, which would be my preference).

Conversely an IPA administrator can see none of the keys of a typical IPA user.

I agree the password being reset upon moving to Staging is a mitigating factor. Thierry can you confirm this?

Martin the problem is password hash disclosure in itself. There is no other "exploitation factor" except that an admin can take those hashes and do offline bruteforce attacks on them.
If the user used a "valuable" password used elsewhere then those creds are at risk.
It is a low security issue, but it is an issue that we should address. Our policy is that hashes are not visible to users, so we either remove the hashes when a user is moved to staging or we make it impossible for admin users to see them.

My understanding is that access (read) to stage user password is a security issue (even from an admin: stage user admin). user password hash could be hidden from IPA CLI, but it will remain readable from a ldapsearch so changing the CLI to hide the password looks not enough.

If we prevent the read access to the user password hash, the password is reset.

I can think to those options, but likely there are others:

- Disallow password setting for Stage user

- Use MODRDN to activate a stage user than ADD_DEL
  The drawback for MODRDN was the complexity to make sure that the activated stage user is fully matching freeipa rules for regular user.
  https://www.freeipa.org/page/V4/User_Life-Cycle_Management#MODRDN_vs._ADD-DEL

Was there any decision if the CVE is valid?

Given different opinions, was there any progress to address the issues raised?

(In reply to Salvatore Bonaccorso from comment #14)
> Was there any decision if the CVE is valid?
>
Yes, we decided that the CVE is valid (though its a low impact security flaw)
 
> Given different opinions, was there any progress to address the issues
> raised?

Seems like a patch is available at:
https://bugzilla.redhat.com/attachment.cgi?id=1331008

Huzaifa, thanks for the clarification.

(In reply to Huzaifa S. Sidhpurwala from comment #15)
...
> Seems like a patch is available at:
> https://bugzilla.redhat.com/attachment.cgi?id=1331008

I doubt this patch is valid, it was not reviewed. Based on discussion in the team, fixing this CVE would need a significant rewrite of staging feature in FreeIPA.

I agree, this patch is not valid. It was attached before than some discussion about side effects of hiding the userpasswd. A summary of the discussion was https://bugzilla.redhat.com/show_bug.cgi?id=1487697#c11.