KNOX-3340: Add Control for LDAPRolesLookupInterceptor#1258
Conversation
This commit adds a RolesLookupBypassControl for use with the LDAPRolesLookupInterceptor. The LDAPRolesLookupInterceptor will skip role mapping if this control is present and true in the request.
| // OID created from a UUID to ensure no collisions: | ||
| // Apache Root OID for core object classes 1.3.6.1.4.1.18060.2 | ||
| // UUID "5236bee0-8a22-4419-9f8e-f1de43312ce1" | ||
| String OID = "1.3.6.1.4.1.18060.2.1379319520.35362.17433.40846.265936912329953"; |
There was a problem hiding this comment.
@smolnar82 do you know how to get an official OID? This OID is generated from a UUID so we shouldn't have any collisions, but it would be nice to get an official OID before this control actually enters use.
There was a problem hiding this comment.
That's an excellent question, @handavid !
You are correct that this UUID-based approach ensures no collisions, but we should aim for a cleaner, official arc under the Apache Private Enterprise Number (PEN).
As a PMC member, I can help facilitate this. Here is the path forward:
- Apache Root: The ASF owns PEN 18060, so our official OID will start with 1.3.6.1.4.1.18060.
- Knox Sub-arc: I've checked the IANA registry and confirmed Knox doesn't have an explicit entry because we fall under the Apache root. The PMC needs to request a dedicated sub-arc (e.g., 1.3.6.1.4.1.18060.X) for Knox by opening a JIRA with ASF Infrastructure (INFRA). - I think @lmccay can help us with that.
- Next Steps: For this PR, it's fine to keep the UUID-based OID as a placeholder so we don't block progress. Once we secure the official Knox sub-arc from INFRA, we can do a quick follow-up task to update this to a shorter, official OID (like 1.3.6.1.4.1.18060.X.1.1).
Test Results22 tests 22 ✅ 2s ⏱️ Results for commit 00b7219. |
smolnar82
left a comment
There was a problem hiding this comment.
A BER-encoded Boolean consists of
- Tag: 1 byte (0x01)
- Length: 1 byte (The size of the value)
- Value: 1 byte (0x00 or 0xFF)
- Total: 3 bytes.
The error in the PR is specifically the content of the middle byte (the "Length" field of the TLV).
In BER, the "Length" byte should only represent the size of the Value portion. For a Boolean, the value is only 1 byte long.
- Current PR encoding: 0x01 (Tag) | 0x03 (Length) | Value (1 byte) -> This says "expect 3 more bytes" but only provides 1.
- Correct BER encoding: 0x01 (Tag) | 0x01 (Length) | Value (1 byte).
Setting the length byte to 0x03 while only providing 1 byte of data might cause standard LDAP clients or strict decoders to fail.
Once this change is done, you may also have to change the documentation.
Great work on the implementation and the tests otherwise!
| RolesLookupBypassControl rolesLookupBypassControl = (RolesLookupBypassControl) control; | ||
|
|
||
| buffer.put(BOOLEAN_TAG_BYTE); | ||
| buffer.put((byte) 3); |
There was a problem hiding this comment.
The correct length is 1.
KNOX-3340 - Add Control to LDAPRolesLookupInterceptor
What changes were proposed in this pull request?
This commit adds a RolesLookupBypassControl for use with the LDAPRolesLookupInterceptor. The LDAPRolesLookupInterceptor will skip role mapping if this control is present and true in the request. This lets the client decide whether they will receive users' groups or roles.
How was this patch tested?
Unit tests were added to cover the new code.
Manual testing was performed. The LDAP Proxy was configured with the RolesLookup interceptor and the following
ldapsearchcommands were run.Integration Tests
no integration tests added
UI changes
no UI changes