Skip to content

Fix ClassCastException for otherName SAN entries during inter-cluster handshake#6137

Open
sashetov wants to merge 2 commits intoopensearch-project:mainfrom
sashetov:sashetov/6090-upn-san-handshake
Open

Fix ClassCastException for otherName SAN entries during inter-cluster handshake#6137
sashetov wants to merge 2 commits intoopensearch-project:mainfrom
sashetov:sashetov/6090-upn-san-handshake

Conversation

@sashetov
Copy link
Copy Markdown

@sashetov sashetov commented May 7, 2026

Description

Bug fix. DefaultInterClusterRequestEvaluator#isInterClusterRequest threw ClassCastException during the transport handshake whenever a peer's certificate contained an otherName SAN (e.g. UPN, OID 1.3.6.1.4.1.311.20.2.3, common on Windows-CA-issued certs), preventing affected nodes from joining the cluster. The parser walked each inner SAN list with an Iterator and cast every even-indexed element to int, which works for two-element entries but breaks on otherName entries that return three or more elements with an OID String at index 2. The fix reads each entry as a single [typeId, value] pair via ian.get(0) / ian.get(1) with an instanceof Integer guard, matching the Java API contract and the existing pattern in SPIFFEPrincipalExtractor.

Before

DefaultInterClusterRequestEvaluatorTest#testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow constructs an X509Certificate whose SAN list contains a UPN-shaped otherName entry: [0, "user@example.com", "1.3.6.1.4.1.311.20.2.3"]. With the parser reverted to its pre-fix state, the test reproduces the exact exception from the bug report:

20260507143419:sasheto@seshstop920:security:sashetov/609...$ git checkout HEAD~1 -- src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java
20260507143420:sasheto@seshstop920:security:sashetov/609...$ fg
vim /tmp/pr-6090-description.md

[1]+  Stopped                 vim /tmp/pr-6090-description.md
20260507143425:sasheto@seshstop920:security:sashetov/609...$ ./gradlew :test --tests "org.opensearch.security.transport.DefaultInterClusterRequestEvaluatorTest.testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow"
=======================================
OpenSearch Build Hamster says Hello!
...
DefaultInterClusterRequestEvaluatorTest > testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow FAILED
...
DefaultInterClusterRequestEvaluatorTest > testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow FAILED
...
DefaultInterClusterRequestEvaluatorTest > testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow FAILED
...
DefaultInterClusterRequestEvaluatorTest > testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow FAILED
...
BUILD FAILED in 24s
12 actionable tasks: 3 executed, 9 up-to-date

The exception message and class are identical to the one originally reported in #6090.

After

Restore the fix and re-run the full test class:

20260507143321:sasheto@seshstop920:security:sashetov/609...$ git checkout HEAD -- src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java
20260507143329:sasheto@seshstop920:security:sashetov/609...$ ./gradlew :test --tests "org.opensearch.security.transport.DefaultInterClusterRequestEvaluatorTest"
=======================================
OpenSearch Build Hamster says Hello!
...
BUILD SUCCESSFUL in 7s
12 actionable tasks: 2 executed, 10 up-to-date

All four cases pass

Checkstyle is also clean:

20260507143148:sasheto@seshstop920:security:sashetov/609...$ ./gradlew checkstyleMain checkstyleTest
=======================================
OpenSearch Build Hamster says Hello!
...
BUILD SUCCESSFUL in 15s
23 actionable tasks: 6 executed, 17 up-to-date

Issues Resolved

Closes #6090

This is not a backport.

These changes do not introduce new permissions and require no companion PR in the security dashboards plugin.

Testing

Adds a new unit test class DefaultInterClusterRequestEvaluatorTest (Mockito-based, mirroring the pattern in SPIFFEPrincipalExtractorTest) with four cases:

  • testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow — direct repro of [BUG] OS handshake fails on certificate containing UPN in the Subject Alternative Names #6090; SAN list contains a 3-element otherName entry. Previously threw ClassCastException; now returns false cleanly.
  • testIsInterClusterRequest_oidSanMatchesNodeOid_returnsTrue — sanity: a registeredID (typeId 8) SAN with the configured node OID still returns true.
  • testIsInterClusterRequest_oidSanAlongsideUpn_returnsTrue — mixed list (UPN otherName + DNS + matching OID) still resolves correctly without throwing.
  • testIsInterClusterRequest_nullSan_returnsFalse — null SAN list is handled gracefully.

No integration or manual testing was performed; the bug surface is fully covered by the unit-level repro because getSubjectAlternativeNames() is the only entry point.

Check List

  • New functionality includes testing
  • New functionality has been documented (no public API change; behavior change documented in PR description)
  • New Roles/Permissions have a corresponding security dashboards plugin PR — n/a
  • API changes companion pull request created — n/a, no API change
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

sashetov added 2 commits May 7, 2026 14:14
… handshake (opensearch-project#6090)

Signed-off-by: sashetov <alexander@vassilevski.com>
Signed-off-by: sashetov <alexander@vassilevski.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix invalid SAN entry structure

The SAN entry for otherName (type 0) should contain exactly 2 elements per RFC 5280:
the type ID and the value. Including a third element (the OID string) creates an
invalid structure that doesn't match real certificate data.

src/test/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluatorTest.java [43]

-sanList.add(Arrays.asList(0, "user@example.com", "1.3.6.1.4.1.311.20.2.3"));
+sanList.add(Arrays.asList(0, "user@example.com"));
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that SAN entries should have exactly 2 elements (type ID and value) per RFC 5280. The test data with 3 elements doesn't match real certificate structure and could lead to misleading test results. This is a valid improvement to test data accuracy.

Medium
Possible issue
Handle numeric type casting safely

The cast to Integer may fail if typeId is a primitive int or another numeric type.
Use Number as the intermediate type and call intValue() to safely extract the
integer value, preventing potential ClassCastException.

src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java [130-134]

 final Object typeId = ian.get(0);
-if (!(typeId instanceof Integer)) {
+if (!(typeId instanceof Number)) {
     continue;
 }
-final int id = (Integer) typeId;
+final int id = ((Number) typeId).intValue();
Suggestion importance[1-10]: 3

__

Why: While the suggestion is technically correct that using Number would be more flexible, the original code with Integer is not incorrect. The getSubjectAlternativeNames() API returns Integer objects for the type ID, so the cast is safe. The suggestion provides marginal improvement in defensive programming but doesn't fix an actual bug.

Low

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.

[BUG] OS handshake fails on certificate containing UPN in the Subject Alternative Names

1 participant