Bug 910938 (CVE-2013-0287)

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]