Bug 910938 (CVE-2013-0287)

Summary: CVE-2013-0287 sssd: simple access provider flaw prevents intended ACL use when client to an AD provider
Product: [Other] Security Response Reporter: Vincent Danen <vdanen>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: CLOSED ERRATA QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: unspecifiedCC: dpal, grajaiya, jgalipea, jhrozek, mkosek, pbrezina, security-response-team, ssorce
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: impact=moderate,public=20130319,reported=20130208,source=redhat,cvss2=4.9/AV:N/AC:M/Au:S/C:P/I:P/A:N,fedora-18/sssd=affected,rhel-6/sssd=affected,rhel-5/sssd=notaffected
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-01-10 10:01:45 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 911298, 911299, 923838    
Bug Blocks: 911296    
Attachments:
Description Flags
[PATCH 1/5] Provide a be_file_account_request function
none
[PATCH 2/5] Split the data provider interface into its own module
none
[PATCH 3/5] Getter for private callback data
none
[PATCH 4/5] Add unit tests for simple access test by groups
none
[PATCH 5/5] Resolve GIDs in the simple access provider
none
[PATCH 1/4] Provide a be_get_account_info_send function
jhrozek: review?
[PATCH 2/4] Add unit tests for simple access test by groups
jhrozek: review?
[PATCH 3/4] Do not compile main() in DP if UNIT_TESTING is defined
jhrozek: review?
[PATCH 4/4] Resolve GIDs in the simple access provider jhrozek: review?

Description Vincent Danen 2013-02-13 17:54:47 EST
Kaushik Banerjee discovered that SSSD's "simple" access provider did not work as expected when SSSD is configured as an Active Directory client when using the new (as of version 1.9.0) Active Directory provider.  During the PAM account phase, SSSD may not not know the group name of a group that the user is a member of, but only the Windows Security Identifier.  Because the group name is not known, the simple_deny_groups option does not work at all, and will always permit access; if any groups are noted in simple_deny_groups, all groups are permitted access.  In addition, if any groups are noted in simple_allow_groups, access is always denied to everyone.

By default, the configuration will allow all users to login (both simple_deny_groups and simple_allow_groups are empty).

The Active Directory provider was introduced in version 1.9.0; earlier versions of SSSD are not vulnerable to this flaw.


Acknowledgements:

This issue was discovered by Kaushik Banerjee of Red Hat.
Comment 2 Jakub Hrozek 2013-03-04 08:58:40 EST
Created attachment 704993 [details]
[PATCH 1/5] Provide a be_file_account_request function

In order to resolve group names in the simple access provider we need to contact the Data Provider in a generic fashion from the access provider. We can't call any particular implementation (like sdap_generic_send()) because we have no idea what kind of provider is configured as the id_provider.

This patch splits out the be_file_account_request() function from the data_provider_be module and makes it public.
Comment 3 Jakub Hrozek 2013-03-04 09:01:51 EST
Created attachment 704995 [details]
[PATCH 2/5] Split the data provider interface into its own module

The simple access provider unit tests now need to link against the Data Provider when they start using the be_file_account_request() function. But then we would start having conflicts as at least the main() functions would clash.

I was considering either the approach I took in this patch or simply wrapping the main() function in something like #ifndef UNDER_TEST. I can still take that option, I don't really mind, but it seemed to me that the data_provider_be module was doing too much anyway.
Comment 4 Jakub Hrozek 2013-03-04 09:04:39 EST
Created attachment 704996 [details]
[PATCH 3/5] Getter for private callback data

When external interfaces start using be_req, they might need to access the callback data, too. This patch exposes a getter. In the long run, I would prefer that the callback data would be passed through another parameter of a callback, but that seemed too invasive for this patch. I would rather file an upstream ticket.
Comment 5 Jakub Hrozek 2013-03-04 09:05:58 EST
Created attachment 704997 [details]
[PATCH 4/5] Add unit tests for simple access test by groups

I realized that the current unit tests for the simple access provider only tested the user directives. To have a baseline and be able to detect new bugs in the upcoming patch, I implemented unit tests for the group lists, too.
Comment 6 Jakub Hrozek 2013-03-04 09:07:58 EST
Created attachment 704998 [details]
[PATCH 5/5] Resolve GIDs in the simple access provider

Changes the simple access provider's interface to be asynchronous. When the simple access provider encounters a group that has gid, but no meaningful name, it attempts to resolve the name using the be_file_account_request function. This patch resolves the CVE.
Comment 7 Jakub Hrozek 2013-03-04 17:18:38 EST
Created attachment 705198 [details]
[PATCH 1/4] Provide a be_get_account_info_send function

In order to resolve group names in the simple access provider we need to contact the Data Provider in a generic fashion from the access provider.  We can't call any particular implementation (like sdap_generic_send()) because we have no idea what kind of provider is configured as the id_provider.
 
This patch splits introduces the be_file_account_request() function into the data_provider_be module and makes it public.

A future patch should make the be_get_account_info function use the be_get_account_info_send function.
Comment 8 Jakub Hrozek 2013-03-04 17:21:41 EST
Created attachment 705200 [details]
[PATCH 2/4] Add unit tests for simple access test by groups

I realized that the current unit tests for the simple access provider only tested the user directives. To have a baseline and be able to detect new bugs in the upcoming patch, I implemented unit tests for the group lists, too.
Comment 9 Jakub Hrozek 2013-03-04 17:24:02 EST
Created attachment 705201 [details]
[PATCH 3/4] Do not compile main() in DP if UNIT_TESTING is defined

The simple access provider unit tests now needs to link against the Data Provider when they start using the be_file_account_request() function. But then we would start having conflicts as at least the main() functions would clash.

If UNIT_TESTING is defined, then the data_provider_be.c module does not contain the main() function and can be linked against directly from another module that contains its own main() function
Comment 10 Jakub Hrozek 2013-03-04 17:25:19 EST
Created attachment 705202 [details]
[PATCH 4/4] Resolve GIDs in the simple access provider

Changes the simple access provider's interface to be asynchronous. When the simple access provider encounters a group that has gid, but no meaningful name, it attempts to resolve the name using the be_file_account_request function.

Some providers (like the AD provider) might perform initgroups without resolving the group names. In order for the simple access provider to work correctly, we need to resolve the groups before performing the access check. In AD provider, the situation is even more tricky b/c the groups HAVE name, but their name attribute is set to SID and they are set as non-POSIX
Comment 11 Jakub Hrozek 2013-03-04 17:30:28 EST
The main difference compared to the first iteration is based on discussion I had with Simo on IRC. The new patchset doesn't use be_req directly at all but uses a new request exported from data_provider_be.c
Comment 12 Pavel Březina 2013-03-05 11:05:40 EST
The code looks good and it seems to work correctly. Ack from me.
Comment 13 Ondrej Kos 2013-03-05 11:22:04 EST
Works as expected, issue seems fixed, Ack from me.
Comment 14 errata-xmlrpc 2013-03-19 14:34:48 EDT
This issue has been addressed in following products:

  Red Hat Enterprise Linux 6

Via RHSA-2013:0663 https://rhn.redhat.com/errata/RHSA-2013-0663.html
Comment 15 Vincent Danen 2013-03-20 10:58:44 EDT
Created sssd tracking bugs for this issue

Affects: fedora-18 [bug 923838]