smolnar82 commented on code in PR #1437:
URL: https://github.com/apache/knox/pull/1437#discussion_r4152839029
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -312,11 +320,60 @@ public boolean verifyToken(JWT token, String jwksUrl,
String algorithm, JOSEObje
verified = true;
}
} catch (BadJOSEException | JOSEException | ParseException |
MalformedURLException e) {
+ if (isIpLiteralTlsFailure(jwksUrl, e)) {
+ /* BC-FIPS refuses HTTPS endpoint identification against a bare IP */
+ if (FipsUtils.isFipsEnabledWithBCProvider()) {
+ LOG.jwksIpLiteralHostUnderFips(jwksUrl);
+ } else {
+ LOG.jwksIpLiteralHost(jwksUrl);
+ }
+ }
throw new TokenServiceException("Cannot verify token.", e);
}
return verified;
}
+ /**
+ * Whether a JWKS failure is a TLS/trust failure against an IP literal host,
the one case where
+ * {@code certificate_unknown(46)} says nothing at all about the contents of
the truststore.
+ *
+ * @param jwksUrl the JWKS endpoint that was being fetched, possibly {@code
null}
+ * @param failure the failure to inspect
+ * @return {@code true} when the host is an IP literal and the chain carries
a TLS/trust failure
+ */
+ static boolean isIpLiteralTlsFailure(final String jwksUrl, final Throwable
failure) {
+ if (jwksUrl == null) {
+ return false;
+ }
+ final String host;
+ try {
+ host = URI.create(jwksUrl).getHost();
+ } catch (IllegalArgumentException e) {
+ /* not a URI we can reason about; we have nothing useful to add */
+ return false;
+ }
+ if (host == null) {
+ return false;
+ }
+ /* getHost() hands an IPv6 literal with brackets. Sanitize it */
+ final String bare = host.length() > 1 && host.charAt(0) == '['
+ ? host.substring(1, host.length() - 1) : host;
+
+ return InetAddresses.isInetAddress(bare) && isTlsTrustFailure(failure);
Review Comment:
We are on Guava 32.1.3-jre, where we can use
`com.google.common.net.InetAddresses.isUriInetAddress(java.lang.String)`
Using that, we could:
```
return host != null
&& InetAddresses.isUriInetAddress(host)
&& isTlsTrustFailure(failure);
```
It's not only simpler and easier to read, but drops the `bare` variable, the
`charAt(0) == '['` logic, and the `substring` call as well as removes the
subtle correctness risk in hand-rolled bracket handling (the current
`substring(1, length-1)` assumes a well-formed closing bracket).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
