[
https://issues.apache.org/jira/browse/KNOX-3546?focusedWorklogId=1054012&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1054012
]
ASF GitHub Bot logged work on KNOX-3546:
----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Nov/26 09:51
Start Date: 11/Nov/26 09:51
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1503:
URL: https://github.com/apache/knox/pull/1503#discussion_r4536362209
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptor.java:
##########
@@ -163,8 +174,10 @@ private boolean isAllGroupsFilterForAncestor(ExprNode
filter) {
children.stream().allMatch(this::isAllGroupsFilter);
}
if (filter instanceof OrNode) {
- // An OR is an all groups filter only if every child is.
- return ((OrNode)
filter).getChildren().stream().allMatch(this::isGroupObjectClassFilter);
+ // Remove any (objectClass=referral) nodes. An OR is an all groups
filter only if every child is.
+ return ((OrNode) filter).getChildren().stream()
+ .filter(node -> !isReferralObjectClassFilter(node))
Review Comment:
ApacheDS's `SearchRequestHandler.modifyFilter` doesn't flatten the filter.
It builds `new OrNode(clientFilter, (objectClass=referral))`. Removing the
referral child here only helps when the client filter is a single objectClass
equality, because the remaining child still has to pass
`isGroupObjectClassFilter`. Once wrapped, these root searches are still not
detected as role listings (I checked with a unit test that wraps the filter the
same way):
- `(&(objectClass=groupOfNames)(cn=*))`
- `(&(objectClass=groupOfNames)(|(cn=viewer)(cn=*)))`
- `(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))`
Could we follow the JIRA instead and unwrap once in `isAllGroupsSearch`
(line 118)? If the filter is a 2-child `OrNode` and one child is
`(objectClass=referral)`, use the other child as `searchFilter`. Then the
existing `isAllGroupsFilter`/`isAllGroupsFilterForAncestor` logic works
unchanged, and this OR branch can go back to how it was.
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptorTest.java:
##########
@@ -667,7 +667,9 @@ public void
testAllGroupsSearchFromRootWithGroupObjectClassFilter() throws Excep
"(objectClass=groupOfUniqueNames)",
"(&(objectClass=groupOfNames)(cn=*))",
"(&(objectClass=groupOfNames)(|(cn=viewer)(cn=*)))",
-
"(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))")) {
+
"(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))",
+ "(|(objectClass=groupOfNames)(objectClass=referral))",
Review Comment:
These two cases hand-write the wrapped form for the simple filters only. The
other entries in this list (the AND and nested-OR ones) still go to the
interceptor unwrapped, which never happens in production unless the client
sends ManageDsaIT. That's the same blind spot the JIRA describes. Could we add
a helper that wraps the way ApacheDS does, e.g. `new
OrNode(FilterParser.parse(schemaManager, filter),
FilterParser.parse(schemaManager, "(objectClass=referral)"))`, and use it for
every filter in this test? The JIRA also asks for the negative side: please
wrap the filters in `testRootSearchWithNonGroupFilterIsNotAllGroupsSearch`
(line 688) as well, so non-group filters are still not treated as listings once
wrapped.
Issue Time Tracking
-------------------
Worklog Id: (was: 1054012)
Time Spent: 0.5h (was: 20m)
> LDAP proxy roles lookup: role listing from an ancestor of ou=groups is never
> detected because ApacheDS adds (objectClass=referral) to the search filter
> --------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: KNOX-3546
> URL: https://issues.apache.org/jira/browse/KNOX-3546
> Project: Apache Knox
> Issue Type: Bug
> Components: LDAP
> Affects Versions: 3.0.0
> Reporter: Sandor Molnar
> Assignee: David Han
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> A group search from an ancestor of {{ou=groups,<proxy base dn>}} (for example
> the proxy root {{{}dc=proxy,dc=com{}}}) with a group objectClass filter
> should be answered with the roles from the role lookup service
> ({{{}listRoles(){}}}). It isn't. The backend groups are translated to roles
> instead, so roles that are assigned only to users are missing from the result.
> *Reproduce* (using the E2E {{{}roles.json{}}}, where {{platform:deployer}} is
> assigned only to the user {{{}recursiveuser{}}}):
> {code:java}
> ldapsearch -x -H ldap://localhost:3890 -b 'dc=proxy,dc=com'
> '(objectClass=groupOfNames)' cn
> {code}
> Expected: every role, including {{{}platform:deployer{}}}. Actual: only the
> roles mapped from backend groups.
> *Root cause:* ApacheDS {{SearchRequestHandler.modifyFilter}} rewrites the
> client's filter to {{{}(|<filter>(objectClass=referral)){}}}, so that
> referral entries are returned too. It skips the rewrite only when the filter
> is {{(objectClass=*)}} or the request carries the ManageDsaIT control
> ({{{}2.16.840.1.113730.3.4.2{}}}). The interceptors therefore get
> {{{}(|(objectClass=groupOfNames)(objectClass=referral)){}}}.
> For a search from an ancestor of {{{}ou=groups{}}},
> {{LDAPRolesLookupInterceptor.isAllGroupsFilterForAncestor}} requires every OR
> branch to be a group objectClass filter. {{objectClass=referral}} isn't one,
> so the search is never detected as a role listing, and {{handleSearch}}
> translates the backend groups instead.
> Searches based at {{ou=groups}} aren't affected. There the OR rule only needs
> one matching branch.
> The bug has been there since KNOX-3487. The unit tests didn't catch it
> because they hand the interceptor the unwrapped filter. The E2E test didn't
> catch it because every role in {{roles.json}} was assigned to a group, so
> translating the groups gave the same set of roles.
> *Workaround:* search from {{{}ou=groups,<proxy base dn>{}}}, or send the
> ManageDsaIT control.
> *Proposed fix:*
> * In {{{}isAllGroupsSearch{}}}, remove the {{(objectClass=referral)}} branch
> that ApacheDS added before checking the filter.
> * Add unit tests that use the rewritten filter: group filters from the root
> are detected as listings, and non-group filters still aren't.
> * Remove the {{@unittest.skip}} from
> {{test_list_all_roles_from_proxy_root_with_group_filter}} in
> {{.github/workflows/tests/test_knox_ldap_proxy_search_with_role_lookup.py}}
> (skipped in KNOX-3539).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
