[
https://issues.apache.org/jira/browse/KNOX-2527?focusedWorklogId=536067&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-536067
]
ASF GitHub Bot logged work on KNOX-2527:
----------------------------------------
Author: ASF GitHub Bot
Created on: 14/Jan/21 16:04
Start Date: 14/Jan/21 16:04
Worklog Time Spent: 10m
Work Description: lmccay commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r557500864
##########
File path:
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java
##########
@@ -205,31 +238,37 @@ private String getSigningKeyAlias(String
signingKeystoreAlias) {
}
@Override
- public boolean verifyToken(JWT token)
- throws TokenServiceException {
+ public boolean verifyToken(JWT token) throws TokenServiceException {
return verifyToken(token, null);
}
@Override
- public boolean verifyToken(JWT token, RSAPublicKey publicKey)
- throws TokenServiceException {
- boolean rc;
- PublicKey key;
+ public boolean verifyToken(JWT token, RSAPublicKey publicKey) throws
TokenServiceException {
+ return TokenUtils.useHMAC(getHmacSecret(),
config.getSigningKeystoreName()) ? verifyTokenUsingHMAC(token) :
verifyTokenUsingRSA(token, publicKey);
Review comment:
For verification, I would expect to make this determination based on the
alg set on the token header rather than any serverside config. This would
better allow for a mix of signing approaches in a single Knox instance. Even in
a deployment where KnoxSSO and other token based authentication is being done
by the gateway itself - there may be 3rd party integrations that would be
better to use PKI rather than having to securely share a password/secret across
that boundary.
##########
File path:
gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/TokenUtils.java
##########
@@ -67,4 +75,26 @@ public static boolean
isServerManagedTokenStateEnabled(FilterConfig filterConfig
return isServerManaged;
}
+ public static String getSignatureAlgorithm(String
configuredSignatureAlgorithm, AliasService aliasService, String
signingKeystoreName) throws AliasServiceException {
+ final char[] hmacSecret =
aliasService.getPasswordFromAliasForGateway(SIGNING_HMAC_SECRET_ALIAS);
+ final String hmacSecretAsString = hmacSecret == null ? null : new
String(hmacSecret);
+ return getSignatureAlgorithm(configuredSignatureAlgorithm,
hmacSecretAsString, signingKeystoreName);
+ }
+
+ public static String getSignatureAlgorithm(String
configuredSignatureAlgorithm, String hmacSecret, String signingKeystoreName) {
+ if (StringUtils.isNotBlank(configuredSignatureAlgorithm)) {
+ return configuredSignatureAlgorithm;
+ } else {
+ return useHMAC(hmacSecret == null ? null :
hmacSecret.getBytes(StandardCharsets.UTF_8), signingKeystoreName) ?
DEFAULT_HMAC_SIG_ALG : DEFAULT_RSA_SIG_ALG;
+ }
+ }
+
+ /**
+ * @return true, if the HMAC secret is configured via the alias service for
the gateway AND there is no previously pre-configured
+ * gateway.signing.keystore.name ; false otherwise
+ */
+ public static boolean useHMAC(byte[] hmacSecret, String signingKeystoreName)
{
+ return hmacSecret != null && StringUtils.isBlank(signingKeystoreName);
Review comment:
This may not be sufficient a check. It may be that we have a deployment
with an HMAC topology and a default RSA topology with only one instance. In
this case, the signing material defaults to the gateway-identity keypair used
for TLS.
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTAuthCodeAssertionFilter.java
##########
@@ -54,6 +60,17 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
GatewayServices services = (GatewayServices)
filterConfig.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
authority = services.getService(ServiceType.TOKEN_SERVICE);
sr = services.getService(ServiceType.SERVICE_REGISTRY_SERVICE);
+
+ setSignatureAlgorithm(services, (GatewayConfig)
filterConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE));
+ }
+
+ private void setSignatureAlgorithm(GatewayServices services, GatewayConfig
gatewayConfig) throws ServletException {
Review comment:
If this class is indeed still needed then should we make this method
common code?
##########
File path:
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java
##########
@@ -167,23 +152,71 @@ public JWT issueToken(Principal p, List<String>
audiences, String algorithm, lon
claimArray[3] = String.valueOf(expires);
}
- JWT token;
- if (SUPPORTED_SIG_ALGS.contains(algorithm)) {
- token = new JWTToken(algorithm, claimArray, audiences);
- try {
- RSAPrivateKey key = getSigningKey(signingKeystoreName,
signingKeystoreAlias, signingKeystorePassphrase);
- // allowWeakKey to not break existing 1024 bit certificates
- JWSSigner signer = new RSASSASigner(key, true);
- token.sign(signer);
- } catch (KeystoreServiceException e) {
- throw new TokenServiceException(e);
+ final JWT token = SUPPORTED_PKI_SIG_ALGS.contains(algorithm) ||
SUPPORTED_HMAC_SIG_ALGS.contains(algorithm) ? new JWTToken(algorithm,
claimArray, audiences) : null;
+ if (token != null) {
+ signToken(algorithm, signingKeystoreName, signingKeystoreAlias,
signingKeystorePassphrase, token);
+ return token;
+ } else {
+ throw new TokenServiceException("Cannot issue token - Unsupported
algorithm: " + algorithm);
+ }
+ }
+
+ private void signToken(String algorithm, String signingKeystoreName, String
signingKeystoreAlias, char[] signingKeystorePassphrase, JWT token) throws
TokenServiceException {
+ if (TokenUtils.useHMAC(getHmacSecret(), signingKeystoreName)) {
Review comment:
Again, the alg to use here should drive the determination of which
approach to use not serverside config. This allows the topology to fully
determine the method to use and for other topologies to use a different method.
Otherwise, I think we will be stuck with one method for the knox instance.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 536067)
Time Spent: 50m (was: 40m)
> Support HMAC signature/verification in JWT token authority
> ----------------------------------------------------------
>
> Key: KNOX-2527
> URL: https://issues.apache.org/jira/browse/KNOX-2527
> Project: Apache Knox
> Issue Type: New Feature
> Components: KnoxSSO, Server
> Affects Versions: 1.5.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 1.6.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> As of now, in {{DefaultTokenAuthorityService}}, the generated JWT token is
> signed by RSA (PKI). It would be beneficial to add support for HMAC as well
> so that token signature/verification would not require a keystore being set
> but using a secret stored via Knox's alias service. The recommended alias
> name is {{gateway.signing.hmac.secret}}
> To support backward compatibility, the implementation should use HMAC
> signature/verification only if:
> - the HMAC secret is configured via the alias service for the gateway, and
> - there is no previously pre-configured {{gateway.signing.keystore.name}}
> which is a clear indication of end-user preference of using PKI signatures.
> The default HMAC signing algorithm should be {{HS256}} (HMAC using SHA-256
> hash algorithm) and clients should be able to change it by already existing
> request parameters called {{knoxsso.token.sigalg}} or {{knox.token.sigalg}}.
> Other valid values are:
> - {{HS384}} (HMAC using SHA-384 hash algorithm)
> - {{HS512}} (HMAC using SHA-512 hash algorithm)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
