Skip to content

NMS-9350: Add support for Kerberos message encryption - #32

Open
dino2gnt wants to merge 5 commits into
release-1.xfrom
dcy/KerberosMessageEncryption
Open

NMS-9350: Add support for Kerberos message encryption#32
dino2gnt wants to merge 5 commits into
release-1.xfrom
dcy/KerberosMessageEncryption

Conversation

@dino2gnt

Copy link
Copy Markdown
Contributor

Adds support for Kerberos message encryption, as detailed in https://msdn.microsoft.com/en-us/library/ee896923.aspx and https://msdn.microsoft.com/en-us/library/cc251574.aspx

This was completed a couple months ago and has been sat on since, so it is not fresh in my head how all this works.

This library is also in a weird half-Jakarta state and could stand some attention once this is merged, before a new release is cut.

Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java Fixed
Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java Fixed
Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/GSSContextManager.java Fixed

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple things that seem like blockers (via claude though I've also looked as much as I can and I think these are all valid).

KerberosDecryptInInterceptor.handleMessage returns early when the body doesn't start with --Encrypted Boundary, so a plaintext SOAP response is accepted.

Also, neither wrap nor unwrap inspects the MessageProp afterwards: getPrivacy() is never checked, so an integrity-only token is accepted as confidential in both directions, and isDuplicateToken/isOldToken/isUnseqToken/isGapToken are discarded, throwing away GSS replay detection. With withKerberosEncryption() on, a non-multipart or non-private response has to be a hard failure.

One GSSContextManager per operation, with correctness resting on a JVM-global socket pool. getIdentifier/getEnumerator/getTransferer each call createProxyFor, which constructs a fresh manager, so every identify/enumerate/pull/get runs its own full AP-REQ/AP-REP pre-flight. The encrypted POST then has to land on the exact TCP connection that that manager's handshake bound, but the HttpURLConnection keep-alive pool is keyed only by (host, port, SSLSocketFactory) and hands out any idle socket. Sequential use mostly works because the latest handshake rebinds the single idle socket; two threads sharing a CXFWSManClient can get each other's session. None of these contexts is ever disposed either.

…d enforce GSS confidentiality and replay checks
Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java Dismissed
@dino2gnt

Copy link
Copy Markdown
Contributor Author

I re-jiggered this because relying on the keepalive pool to maintain the same connection across multiple threads probably wasn't going to work with any real concurrency. Fable helped.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but I think this is worth doing.

Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java Outdated
Comment thread cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java Outdated

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good!

@dino2gnt

Copy link
Copy Markdown
Contributor Author

Gonna wait for Chandra or Christian to critique it before I merge (no offense)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants