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.
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.
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.
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.
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.
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.
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.
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.
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
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
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
The code looks good and it seems to work correctly. Ack from me.
Works as expected, issue seems fixed, Ack from me.
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
Created sssd tracking bugs for this issue Affects: fedora-18 [bug 923838]