ipa: filter DNs for ipa_add_trusted_memberships_send()#8147
ipa: filter DNs for ipa_add_trusted_memberships_send()#8147alexey-tikhonov merged 1 commit intoSSSD:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a filter to ipa_add_trusted_memberships_send to process only 'proper' group DNs, avoiding unnecessary and failing LDAP lookups for other object types. The implementation adds a new function filter_groups_by_attribute_name. My review has identified a couple of critical issues in this new function, including a potential buffer over-read and incorrect memory context usage in talloc. I've also suggested a minor refactoring to remove an unused parameter.
26f4eb5 to
3244d89
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a filtering mechanism for group Distinguished Names (DNs) within ipa_add_trusted_memberships_send(). The purpose is to prevent unnecessary and failing LDAP lookups for objects that are not standard groups, such as iPAAssociation objects used for HBAC and sudo rules. This is achieved by adding a new static function, filter_groups_by_attribute_name, which filters a list of DNs to include only those with a specific RDN attribute name (configured via ldap_group_name). The implementation is clean, well-commented, and appears to correctly solve the issue described. After a thorough review, I have not identified any issues of high or critical severity.
|
Hi @alexey-tikhonov, I tired to address you review comments form #8094 about the filtering and tried to avoid memory allocations as much as possible. I didn't move the filtering closer to the point where the list is actually used because the first usage of the list is in bye, |
3244d89 to
2c83dbb
Compare
|
Note: Covscan is green. |
f5d64b3 to
b854636
Compare
|
@sumit-bose, could you please rebase? |
2c83dbb to
c710633
Compare
done |
|
CI failures not related. |
The ipa_add_trusted_memberships_send() request will use groups_get_send() to lookup missing groups. groups_get_send() can currently only lookup "proper" groups which besides other items means that the group name must be stored under the LDAP attribute given by the 'ldap_group_name' option. So currently it does not make sense to try to lookup other objects where the RDN attribute name is different than this value because those will always be treated as missing in the cache and always trigger an LDAP search which will fail. This will typically happen for iPAAssociation objects which are used to connect users and hosts with HBAC and sudo rules. If in future a more generic search is used this filter can be removed. Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>
c710633 to
7e32114
Compare
The ipa_add_trusted_memberships_send() request will use groups_get_send() to lookup missing groups. groups_get_send() can currently only lookup "proper" groups which besides other items means that the group name must be stored under the LDAP attribute given by the 'ldap_group_name' option. So currently it does not make sense to try to lookup other objects where the RDN attribute name is different than this value because those will always be treated as missing in the cache and always trigger an LDAP search which will fail. This will typically happen for iPAAssociation objects which are used to connect users and hosts with HBAC and sudo rules. If in future a more generic search is used this filter can be removed.