[
https://issues.apache.org/jira/browse/KNOX-3512?focusedWorklogId=1047379&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1047379
]
ASF GitHub Bot logged work on KNOX-3512:
----------------------------------------
Author: ASF GitHub Bot
Created on: 14/Oct/26 07:29
Start Date: 14/Oct/26 07:29
Worklog Time Spent: 10m
Work Description: hsheinblatt commented on code in PR #1451:
URL: https://github.com/apache/knox/pull/1451#discussion_r4267330722
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
Review Comment:
At first I was confused by this: you're looking up the issuer here below,
checking the trust, but you don't pass in the trust to the resolveJwksUri
method chain so it can retrieve the discovery document using it. But it turns
out the helper just independently retrieves the issuer trust and uses a factory
to generate a one-off client with that context. So this is using the cert if
defined. I'd update the comment here to make that clear.
There are scalability implications of using the one-off clients. This
comment belongs with the helper. I think it's probably fine since we cache the
jwks, so we're not going to reuse the client connection except on retry which
is done once inline anyway. So mostly sharing the client would just refresh
connections each time anyway. But it would be clearer to explain or at least
show it was designed.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -280,26 +293,49 @@ private boolean verifyTokenUsingHMAC(JWT token) throws
TokenServiceException {
@Override
public boolean verifyToken(JWT token, String jwksUrl, String algorithm,
JOSEObjectTypeVerifier<SecurityContext> typeVerifier) throws
TokenServiceException {
+ return verifyToken(token, jwksUrl, algorithm, typeVerifier, null);
+ }
+
+ @Override
+ public boolean verifyToken(JWT token, Set<URI> jwksUrls, String algorithm,
JOSEObjectTypeVerifier<SecurityContext> typeVerifier) throws
TokenServiceException {
+ return verifyToken(token, jwksUrls, algorithm, typeVerifier, null);
+ }
+
+ @Override
+ public boolean verifyToken(JWT token, Set<URI> jwksUrls, String algorithm,
JOSEObjectTypeVerifier<SecurityContext> typeVerifier,
+ IssuerJwksTrust jwksTrust) throws
TokenServiceException {
+ boolean verified = false;
+ for(final URI url : jwksUrls) {
Review Comment:
nit: space before open paren
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java:
##########
@@ -225,9 +231,40 @@ private Map<String, Object> issuerToMap(TrustedOidcIssuer
issuer) {
map.put("registeredAt",
issuer.getRegisteredAt() != null ? issuer.getRegisteredAt().toString()
: null);
map.put("registeredBy", issuer.getRegisteredBy());
+ // CA certificates are public; the value is returned as registered so
callers can detect drift.
+ map.put("jwksCa", issuer.getJwksCa());
return map;
}
+ /**
+ * Validates an optional {@code jwksCa}. A Secret reference is checked for
syntax only, since the
+ * Secret may be created after the issuer is registered.
+ *
+ * @return the error description, or null if the value is valid or absent
+ */
+ private static String validateJwksCa(String jwksCa, boolean dynamicJwks) {
+ if (jwksCa == null) {
+ return null;
+ }
+ if (!dynamicJwks) {
+ return "jwksCa requires dynamicJwks=true; it only applies to OIDC
discovery and JWKS fetches";
+ }
+ if (jwksCa.length() > JwksCa.MAX_LENGTH) {
+ return "jwksCa must not exceed " + JwksCa.MAX_LENGTH + " characters";
+ }
+ try {
+ if (JwksCa.isSecretReference(jwksCa)) {
+ JwksCa.parseSecretReference(jwksCa);
+ } else {
+ JwksCa.parsePemBundle(jwksCa);
+ }
+ } catch (IllegalArgumentException e) {
+ // the secret reference errors already name the field; the PEM errors do
not
+ return JwksCa.isSecretReference(jwksCa) ? e.getMessage() : "Invalid
jwksCa: " + e.getMessage();
Review Comment:
nit: Just add the prefix to the parsePemBundle error message or use separate
try/catch so you don't have to parse it twice.
Issue Time Tracking
-------------------
Worklog Id: (was: 1047379)
Time Spent: 40m (was: 0.5h)
> KnoxIDF: per-issuer CA bundle (jwksCa) for trusted OIDC issuer discovery and
> JWKS fetches
> -----------------------------------------------------------------------------------------
>
> Key: KNOX-3512
> URL: https://issues.apache.org/jira/browse/KNOX-3512
> Project: Apache Knox
> Issue Type: Task
> Components: JWT, Server
> Affects Versions: 3.1.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> *Problem*
> Knox fetches each trusted OIDC issuer's discovery document and JWKS over
> HTTPS using the global JVM/Knox truststore. Kubernetes apiservers acting as
> issuers often use self-signed certificates, each with its own CA. Today the
> only way to trust them is to import every CA into the global truststore at
> startup. That has two costs:
> - Every CA rotation needs a restart.
> - Trust widens for all TLS clients in Knox (LDAP and the rest).
> TrustedOidcIssuer has no way to carry a CA.
> *Proposal*
> Add an optional {{jwksCa}} field to trusted OIDC issuer registrations. It
> takes one of two forms:
> - Inline PEM: one or more {{---{-}{{-}}BEGIN CERTIFICATE{{-}}{-}---}} blocks.
> - Kubernetes Secret reference: {{{}secret:[<namespace>/]<name>#<key>{}}}.
> Namespace defaults to "knox", the other 2 parts are required. Knox reads it
> through the in-cluster Kubernetes API.
> +Behaviour:+
> - When {{jwksCa}} is set, that issuer's discovery and JWKS TLS connections
> trust only the given bundle, and hostname verification stays on. The global
> truststore is never changed, so other TLS clients in Knox do not trust the
> issuer CA.
> - When {{jwksCa}} is not set, nothing changes.
> - {{jwksCa}} is only accepted together with {{{}dynamicJwks: true{}}}.
> - If {{jwksCa}} is configured but cannot be loaded (Secret or key missing,
> unparsable PEM), verification fails closed and does not fall back to the
> global truststore.
> - CA changes take effect without a restart:
> -- Inline: re-registering the issuer replaces its trust material straight
> away.
> -- Secret reference: Knox reads the Secret lazily, with no background
> refresh. It reads it:
> --- on the first discovery or JWKS fetch for the issuer.
> --- on access, when the cached CA is older than
> {{gateway.trusted.oidc.issuer.ca.secret.max.age.secs}} (default
> {{{}1800{}}}). This picks up CAs removed from the bundle.
> --- after a fetch fails with a TLS trust error (handshake or certificate
> validation). Knox re-reads the Secret and retries once, at most once every
> 30s per issuer. Timeouts and non-200 responses don't trigger a re-read.
> --- on {{{}POST /trusted-oidc-issuers/refresh-jwks{}}}, which drops the
> cached CA so the next fetch reads it again.
> -- If the Secret can't be read the first time, verification fails closed. If
> a max-age re-read fails, Knox keeps the last working CA and logs the failure.
> - Rotating the issuer's signing key still needs no config change. When a
> token's kid is not in the cached JWKS, the JWKS is fetched again.
> *Changes*
> - {{{}TrustedOidcIssuer{}}}, {{RegisterIssuerRequest}} and the admin REST
> resource get the new jwksCa field. Registration validates it and returns 400
> invalid_request when:
> -- it is set but dynamicJwks is false
> -- it is too long
> -- a secret: value has the wrong syntax
> -- inline PEM does not parse
> - {{TRUSTED_OIDC_ISSUERS}} table: new {{jwks_ca}} column ({{{}TEXT{}}}, or
> {{CLOB}} on Oracle).
> - {{TrustedOidcIssuerService}} exposes per-issuer TLS trust. It is used by
> OIDC discovery (Apache HttpClient) and by the JWKS fetch (Nimbus
> {{DefaultResourceRetriever}} with a per-issuer {{{}SSLSocketFactory{}}}).
> - JWTokenAuthority gets an overload of verifyToken that takes the JWKS
> SSLSocketFactory.
> - Docs: new field, config properties and RBAC requirements. Also a
> shared-root option: if every cluster chains to one root CA, use the same
> bundle or Secret for every issuer.
> Acceptance criteria
> - Issuers with different self-signed CAs all validate.
> - Replacing an issuer's CA bundle (inline or Secret) takes effect without a
> Knox restart.
> - An unrelated TLS client in Knox does not trust the issuer CA.
> - Rotating an apiserver signing key needs no Knox config change.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
