You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/01/11 15:49:54 UTC
[GitHub] [knox] smolnar82 opened a new pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
smolnar82 opened a new pull request #397:
URL: https://github.com/apache/knox/pull/397
## What changes were proposed in this pull request?
In this change, I'm extending the current RSA based token signing with the use of HMAC. As described in the corresponding JIRA, from now on end-users are able to configure an HMAC secret in the gateway's alias service which then will be used as a signature secret (pay attention to the key size requirements!).
## How was this patch tested?
Updated and ran a full maven build successfully and executed E2E tests:
1. configured the `gateway.signing.hmac.secret` alias:
```
$ bin/knoxcli.sh list-alias
Listing aliases for: __gateway
gateway.signing.hmac.secret
```
2. Added the `KNOXTOKEN` service in the `sandbox` topology:
```
<service>
<role>KNOXTOKEN</role>
<param>
<name>knox.token.ttl</name>
<value>36000000</value>
</param>
<param>
<name>knox.token.audiences</name>
<value>tokenbased</value>
</param>
<param>
<name>knox.token.target.url</name>
<value>https://localhost:8443/gateway/tokenbased</value>
</param>
<!--
<param>
<name>knox.token.sigalg</name>
<value>HS256</value>
</param>
-->
</service>
```
3. Added a new topology - called tokenbased - to be able to test the generated tokens
4. Acquired a Knox token:
```
curl -iku admin:admin-password https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token
...
HTTP/1.1 200 OK
Date: Mon, 11 Jan 2021 15:08:34 GMT
Set-Cookie: KNOXSESSIONID=node01eow5gtl4jq0z68xpcuoo9cut0.node0; Path=/gateway/sandbox; Secure; HttpOnly
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Set-Cookie: rememberMe=deleteMe; Path=/gateway/sandbox; Max-Age=0; Expires=Sun, 10-Jan-2021 15:08:34 GMT; SameSite=lax
Content-Type: application/json
Content-Length: 1862
{"access_token":"eyJhbGciOiJSUzI1NiJ9...FN7RE8Xsw","target_url":"https://localhost:8443/gateway/tokenbased","endpoint_public_cert":"MIIDe...W6Z2nJarXg==","token_type":"Bearer","expires_in":1610413714993}
```
5. Tested the acquired token by invoking the HDFS UI service in `tokenbased`:
```
curl -ik -H "Authorization: Bearer eyJhbGciOiJSUzI1NiJ9...FN7RE8Xsw" https://localhost:8443/gateway/tokenbased/hdfs
```
6. Changed the value of `knox.token.sigalg` parameter to `HS384`, reset the `gateway.signing.hmac.secret` with an appropriate secret (key length matters) and repeated steps 4-5. The call finished successfully.
7. Changed the value of `knox.token.sigalg` parameter to `HS512`, reset the `gateway.signing.hmac.secret` with an appropriate secret (key length matters) and repeated steps 4-5. The call finished successfully.
8. Changed the value of `knox.token.sigalg` parameter to `HS123` and repeated steps 4-5. Call failed as expected.
9. Removed the `gateway.signing.hmac.secret` and re-tried the get/verify steps; all were OK.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r561935644
##########
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:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r561936179
##########
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:
This makes sense; I fixed the code.
##########
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:
This makes sense; I fixed the code.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] risdenk commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r555234268
##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java
##########
@@ -78,17 +83,22 @@
private GatewayConfig config;
private char[] cachedSigningKeyPassphrase;
+ private byte[] cachedSigningHmacSecret;
private RSAPrivateKey signingKey;
static {
- // Only standard RSA signature algorithms are accepted
+ // Only standard RSA and HMAC signature algorithms are accepted
// https://tools.ietf.org/html/rfc7518
SUPPORTED_SIG_ALGS.add("RS256");
SUPPORTED_SIG_ALGS.add("RS384");
SUPPORTED_SIG_ALGS.add("RS512");
SUPPORTED_SIG_ALGS.add("PS256");
SUPPORTED_SIG_ALGS.add("PS384");
SUPPORTED_SIG_ALGS.add("PS512");
+ //HMAC
+ SUPPORTED_SIG_ALGS.add("HS256");
+ SUPPORTED_SIG_ALGS.add("HS384");
+ SUPPORTED_SIG_ALGS.add("HS512");
Review comment:
Would it make sense to have two `SUPPORTED_SIG_ALGS` lists? One for PKI and the other for HMAC? Then can check and hopefully provide a better error message if the wrong signature algorithm is chosen for the wrong method?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r555630478
##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java
##########
@@ -78,17 +83,22 @@
private GatewayConfig config;
private char[] cachedSigningKeyPassphrase;
+ private byte[] cachedSigningHmacSecret;
private RSAPrivateKey signingKey;
static {
- // Only standard RSA signature algorithms are accepted
+ // Only standard RSA and HMAC signature algorithms are accepted
// https://tools.ietf.org/html/rfc7518
SUPPORTED_SIG_ALGS.add("RS256");
SUPPORTED_SIG_ALGS.add("RS384");
SUPPORTED_SIG_ALGS.add("RS512");
SUPPORTED_SIG_ALGS.add("PS256");
SUPPORTED_SIG_ALGS.add("PS384");
SUPPORTED_SIG_ALGS.add("PS512");
+ //HMAC
+ SUPPORTED_SIG_ALGS.add("HS256");
+ SUPPORTED_SIG_ALGS.add("HS384");
+ SUPPORTED_SIG_ALGS.add("HS512");
Review comment:
Done and tested:
```
2021-01-12 10:32:35,717 ERROR service.knoxtoken (TokenResource.java:getAuthenticationToken(439)) - Unable to issue token.
org.apache.knox.gateway.services.security.token.TokenServiceException: The Knox Gateway is configured to use PKI as signing method, however the supplied algorithm (HS256) is incompatible with that method
at org.apache.knox.gateway.services.token.impl.DefaultTokenAuthorityService.signToken(DefaultTokenAuthorityService.java:175)
at org.apache.knox.gateway.services.token.impl.DefaultTokenAuthorityService.issueToken(DefaultTokenAuthorityService.java:157)
at org.apache.knox.gateway.services.token.impl.DefaultTokenAuthorityService.issueToken(DefaultTokenAuthorityService.java:137)
at org.apache.knox.gateway.service.knoxtoken.TokenResource.getAuthenticationToken(TokenResource.java:401)
at org.apache.knox.gateway.service.knoxtoken.TokenResource.doGet(TokenResource.java:233)
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] lmccay commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r555288724
##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java
##########
@@ -78,17 +83,22 @@
private GatewayConfig config;
private char[] cachedSigningKeyPassphrase;
+ private byte[] cachedSigningHmacSecret;
private RSAPrivateKey signingKey;
static {
- // Only standard RSA signature algorithms are accepted
+ // Only standard RSA and HMAC signature algorithms are accepted
// https://tools.ietf.org/html/rfc7518
SUPPORTED_SIG_ALGS.add("RS256");
SUPPORTED_SIG_ALGS.add("RS384");
SUPPORTED_SIG_ALGS.add("RS512");
SUPPORTED_SIG_ALGS.add("PS256");
SUPPORTED_SIG_ALGS.add("PS384");
SUPPORTED_SIG_ALGS.add("PS512");
+ //HMAC
+ SUPPORTED_SIG_ALGS.add("HS256");
+ SUPPORTED_SIG_ALGS.add("HS384");
+ SUPPORTED_SIG_ALGS.add("HS512");
Review comment:
👍
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 merged pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #397:
URL: https://github.com/apache/knox/pull/397
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r561935644
##########
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:
Done.
##########
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 makes sense; I fixed the 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:
This makes sense; I fixed the code.
##########
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:
This makes sense; I fixed the code.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on pull request #397:
URL: https://github.com/apache/knox/pull/397#issuecomment-767448085
@lmccay - I tested it with different topologies come with different signature algorithms (HS256 and RS 256) and it was working as expected. Could you please give me another review and let me know if I can merge this PR?
HS256 test
----------
Created a 256 bit HMAC secret:
`bin/knoxcli.sh create-alias gateway.signing.hmac.secret --value 6w9zAB-E_H-McQfTjWnZr4u7x_A7D8F-`
```
curl -iku admin:admin-password https://localhost:8443/gateway/tokenhs256/knoxtoken/api/v1/token
{"access_token":"eyJhbG...RI2hKM",...,"token_type":"Bearer","expires_in":1611689480150}
```
gateway.log:
`2021-01-26 10:35:24,158 INFO service.knoxtoken (TokenResource.java:getAuthenticationToken(407)) - Knox Token service (tokenhs256) issued token eyJhbG...RI2hKM (7794004b-c3e8-4aea-9d69-226786948ace)`
Testing the acquired token: `curl -ik -H "Authorization: Bearer eyJhbG...RI2hKM" https://localhost:8443/gateway/tokenbased/hdfs`
gateway-audit.log:
```
21/01/26 10:37:27 ||8e4bcfe0-4a26-4f0c-a74b-857217ffeae7|audit|[0:0:0:0:0:0:0:1]|HDFSUI||||access|uri|/gateway/tokenbased/hdfs|unavailable|Request method: GET
21/01/26 10:37:27 ||8e4bcfe0-4a26-4f0c-a74b-857217ffeae7|audit|[0:0:0:0:0:0:0:1]|HDFSUI|admin|||authentication|uri|/gateway/tokenbased/hdfs|success|
```
RS256 test
----------
`curl -iku admin:admin-password https://localhost:8443/gateway/tokenrs256/knoxtoken/api/v1/token`
`{"access_token":"eyJhbG...xukCHw",...,"token_type":"Bearer","expires_in":1611690064630}`
gateway.log:
`2021-01-26 10:41:04,638 INFO service.knoxtoken (TokenResource.java:getAuthenticationToken(407)) - Knox Token service (tokenrs256) issued token eyJhbG...xukCHw (fffd212d-dba7-40c5-8325-3512203ffde2)`
Testing the acquired token: `curl -ik -H "Authorization: Bearer eyJhbG...xukCHw" https://localhost:8443/gateway/tokenbased/hdfs`
gateway-audit.log:
```
21/01/26 10:44:35 ||1bac63c9-1c15-4673-a592-2072f9235d53|audit|[0:0:0:0:0:0:0:1]|HDFSUI||||access|uri|/gateway/tokenbased/hdfs|unavailable|Request method: GET
21/01/26 10:44:45 ||1bac63c9-1c15-4673-a592-2072f9235d53|audit|[0:0:0:0:0:0:0:1]|HDFSUI|admin|||authentication|uri|/gateway/tokenbased/hdfs|success|
```
The only difference between the `tokenhs256` and tokenrs256` topologies is the configured signature algorithm:
```
<service>
<role>KNOXTOKEN</role>
<param>
<name>knox.token.ttl</name>
<value>36000000</value>
</param>
<param>
<name>knox.token.audiences</name>
<value>tokenbased</value>
</param>
<param>
<name>knox.token.target.url</name>
<value>https://localhost:8443/gateway/tokenbased</value>
</param>
<param>
<name>knox.token.sigalg</name>
<value>HS256</value>
</param>
</service>
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #397: KNOX-2527 - Added support for HMAC signature/verification in JWT token authority
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #397:
URL: https://github.com/apache/knox/pull/397#discussion_r561935873
##########
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 makes sense; I fixed the code.
----------------------------------------------------------------
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:
users@infra.apache.org