KNOX-3519 - Set the JWKS host on the TLS socket so HTTPS endpoint identification works under BC-FIPS by moresandeep · Pull Request #1459 · apache/knox

moresandeep · 2026-10-20T02:52:44Z

KNOX-3519 - Set the JWKS host on the TLS socket so HTTPS endpoint identification works under BC-FIPS

What changes were proposed in this pull request?

Under the BouncyCastle FIPS JSSE provider, fetching a JWKS over HTTPS fails before any
certificate is examined:

java.security.cert.CertificateException: No hostname specified for HTTPS endpoint ID check
    at org.bouncycastle.jsse.provider.HostnameUtil.checkHostname(HostnameUtil.java:31)
    at org.bouncycastle.jsse.provider.ProvX509TrustManager.checkEndpointID(ProvX509TrustManager.java:333)

JWT verification then fails and the request is rejected with 401, even when the JWKS
endpoint's certificate is fully trusted and carries a matching SAN.

The change. A new SetHostSSLSocketFactory decorator stamps the JWKS URL's host onto
every socket it creates, via setHost(String) as declared by
org.bouncycastle.jsse.BCSSLSocket. It is wired in at both JWKS retrieval paths:
DefaultTokenAuthorityService.buildJwkSource (default truststore) and
IssuerTrustResourceRetriever (per-issuer CA bundle).

Notes on the implementation:

The BC type is reached reflectively, so Knox takes no compile dependency on the BouncyCastle
TLS provider, it is present only in FIPS deployments. Lookup walks interfaces first,
because BC's socket implementations are package-private and the method is only invocable
through the public BCSSLSocket interface.
On sockets that do not expose setHost the decorator is a no-op. That is correct for the
stock JSSE, which identifies HTTPS endpoints from the URL already, so non-FIPS deployments
are unaffected.
Endpoint identification is preserved, not bypassed. The server certificate must still
carry a SAN matching the JWKS host.
The retriever built for the default path reproduces Nimbus's own defaults exactly
(connect/read 500 ms, 51200 byte limit, disconnectAfterUse = true), so timeout and size
behaviour are unchanged.

How was this patch tested?

Patch was tested locally.

…uses IP literals

github-actions · 2026-10-20T03:45:46Z

Test Results

4 files 4 suites 1m 26s ⏱️
145 tests 145 ✅ 0 💤 0 ❌
169 runs 169 ✅ 0 💤 0 ❌

Results for commit a7131cf.

♻️ This comment has been updated with latest results.

lmccay

Made some comments and asked questions.

lmccay · 2026-10-20T04:05:09Z

+        /* Not reachable through this type; a public interface may still expose it. */
+        continue;
+      } catch (InvocationTargetException | RuntimeException e) {
+        return false;


Should a single failure to set it even though it appears to have such a method fail the entire iteration over the types? Should we not continue to walk the hierarchy to see if one will work? Are we assuming that someone overrode another implementation in order to purposely not support it and throw a RTE rather than just no-op it?

The IllegalAccessException which we tolerate and keep moving is caused due to reflection access, with this exception we know we hit, say a private method in impl e.g. org.bouncycastle.jsse.provider.ProvSSLSocketDirect

InvocationTargetException and RuntimeException errors will be thown in (or within) the method body, e.g. BCSSLSocket.setHost (interface) and ProvSSLSocketDirect.setHost (concrete impl) land on the same method body so If the call itself throws, continuing the walk will invokes the identical code and gets an identical exception. So, we'll just be throwing errors and not gaining much.

I will make sure to add docs to the method, let me know if this explanation helps!

lmccay · 2026-10-20T04:23:39Z

+   * @return {@code true} when the host was applied, {@code false} when the socket does not
+   *         support it or the call failed
+   */
+  static boolean setHost(Socket socket, String host) {


It seems that we are iterating over the class hierarchy of the socket looking for the first one that accepts or rejects a setHost method call. IIUC, then this looks for the most specific implementation of setHost that is public. If it fails due to method visibility it continues to walk the hierarchy to try and find a public one that will work. If that's accurate then I think we should document this a bit here to describe the intent at least.

Yes this needs some more docs. Specifically, we walk the hierarchy and add interfaces first then the classes, this is due to the fact that BC's socket implementations are package private e.g. org.bouncycastle.jsse.provider.ProvSSLSocketDirect, no public method, so setHost is only invocable through the public org.bouncycastle.jsse.BCSSLSocket interface.

I will document it better.

hanicz · 2026-10-20T10:13:10Z

+   * HTTPS endpoint identification can run under the BouncyCastle FIPS provider. See
+   * SetHostSSLSocketFactory. Timeouts and size limit match the Nimbus defaults.
+   */
+  private static ResourceRetriever defaultResourceRetriever(URL jwksUrl) {


We should gate this on FIPS. We already have the FipsUtils class that identifies FIPS environments.

We actuially gate this on FIPS so the actual socket setHost(String) only exists on BCSSLSocket, for stock JSSE forUrl's decorator hands the socket back untouched.

THis is also covered in testLeavesSocketWithoutSetHostUnchanged.

Plus we don't want this fix just for FIPS, we want this independent of provider.

hanicz · 2026-10-20T10:14:03Z

+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;


There is already a Fips related package in gateway-spi org.apache.knox.gateway.fips

Similar to previous comment, this fix is not really FIPS-specific in nature (even though the manifestation is in fips) it fixes a HttpsURLConnection/Nimbus interaction and is a no-op elsewhere, BC is just where the symptom shows up. So fips would mislabel it.

I understand that your saying it isn't FIPS specific but isn't there a risk of some unrelated socket type hierarchy having a setHost method available? Shouldn't we be explicitly enabling this for deployments that we know we want this behavior? So there is really only a no-op when there is no unknown type with a setHost method. Right?

True, the noop is conditional on setHost being absent, any type in the hierarchy (for JSSE provider) with a public setHost(String) gets called regardless.

I'll make the lookup itself specific to match on the declaring type by name org.bouncycastle.jsse.BCSSLSocket, and call setHost only through that interface.

I'll also move this to gateway-spi's fips package

KNOX-3519 - Fix an issue where BC-FIPS HostnameUtil.checkHostname ref…

bef5a3e

…uses IP literals

moresandeep requested review from hanicz, pzampino and smolnar82 October 20, 2026 02:52

lmccay reviewed Oct 20, 2026

View reviewed changes

hanicz requested changes Oct 20, 2026

View reviewed changes

KNOX-3519 - Added comments

a7131cf

moresandeep requested a review from hanicz October 20, 2026 17:23

Navigation Menu

Repository navigation

KNOX-3519 - Set the JWKS host on the TLS socket so HTTPS endpoint identification works under BC-FIPS - #1459

KNOX-3519 - Set the JWKS host on the TLS socket so HTTPS endpoint identification works under BC-FIPS#1459
moresandeep wants to merge 2 commits into
apache:masterfrom
moresandeep:KNOX-3519

moresandeep commented Oct 20, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

lmccay left a comment

Uh oh!

Uh oh!

lmccay Oct 20, 2026

Uh oh!

moresandeep Oct 20, 2026

Uh oh!

lmccay Oct 20, 2026

Uh oh!

moresandeep Oct 20, 2026

Uh oh!

hanicz Oct 20, 2026

Uh oh!

moresandeep Oct 20, 2026

Uh oh!

hanicz Oct 20, 2026

Uh oh!

moresandeep Oct 20, 2026

Uh oh!

lmccay Oct 20, 2026

Uh oh!

moresandeep Oct 20, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

moresandeep commented Oct 20, 2026

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

github-actions Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

lmccay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 20, 2026 •

edited

Loading