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/04/20 18:03:40 UTC

[GitHub] [knox] smolnar82 opened a new pull request #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

smolnar82 opened a new pull request #437:
URL: https://github.com/apache/knox/pull/437


   ## What changes were proposed in this pull request?
   
   Saving the `token.id` claim (aka. tokenId or passcode) in a hashed form in the database instead of plain text.
   
   ## How was this patch tested?
   
   Updated JUnit tests using Derby DB. I also executed the following manual steps with a Postgres DB configured as the token management backend. During my tests, I did not declare hashing algorithm in `gateway-site.xml` so that the default `HmacSha256` was used so I needed to configure an appropriate hashing key.
   
   0. Configuration:
   ```
   bin/knoxcli.sh create-alias gateway_database_hash_key --value sPj8FCgQhCEi6G18kBfpswxYSki33plbelGLs0hMSbk
   ```
   
   1. Fetched a token using our tokengen application (passcode/token.id=b1e78313-3635-4236-bd77-71886ef9a707)
   ```
   2021-04-20 19:38:11,818 DEBUG knox.gateway (GatewayFilter.java:doFilter(116)) - Received request: GET /knoxtoken/api/v1/token
   2021-04-20 19:38:11,907 INFO  service.knoxtoken (TokenResource.java:getAuthenticationToken(421)) - Knox Token service (homepage) issued token eyJhbG...B9Ze9A (b1e78313...71886ef9a707)
   2021-04-20 19:38:11,920 DEBUG token.state (JDBCTokenStateService.java:addToken(82)) - Token b1e78313...71886ef9a707 has been saved in the database; id = 4448b054-3467-4c29-9468-d4bddfdf5e00
   2021-04-20 19:38:11,920 DEBUG token.state (DefaultTokenStateService.java:addToken(144)) - Added token b1e78313...71886ef9a707, expiration 2021-04-21T17:38:11.900Z
   2021-04-20 19:38:11,927 DEBUG token.state (JDBCTokenStateService.java:addMetadata(213)) - Updated metadata for b1e78313...71886ef9a707 in the database
   2021-04-20 19:38:11,927 DEBUG service.knoxtoken (TokenResource.java:getAuthenticationToken(448)) - Knox Token service (homepage) stored state for token eyJhbG...B9Ze9A (b1e78313...71886ef9a707)
   
   postgres=# select * FROM KNOX_TOKENS;
                     id                  |                        token_id                         |  issue_time   |  expiration   | max_lifetime  | username | comment 
   --------------------------------------+---------------------------------------------------------+---------------+---------------+---------------+----------+---------
    4448b054-3467-4c29-9468-d4bddfdf5e00 | [\x1A�\x15�w���\x04f\x7F+�S\x1D��\x06��\x1B�(3�س�\x1F�G | 1618940291911 | 1619026691900 | 1619545091911 | admin    | 
   (1 row)
   ```
   
   2. Renewed the token
   ```
   $ curl -ku admin:admin-password -d "@token.txt" -X POST https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/renew
   {
     "renewed": "true",
     "expires": "1619026870461"
   }
   
   2021-04-20 19:41:10,445 DEBUG knox.gateway (GatewayFilter.java:doFilter(116)) - Received request: POST /knoxtoken/api/v1/token/renew
   2021-04-20 19:41:10,446 INFO  knox.gateway (KnoxLdapRealm.java:getUserDn(688)) - Computed userDn: uid=admin,ou=people,dc=hadoop,dc=apache,dc=org using dnTemplate for principal: admin
   2021-04-20 19:41:10,476 DEBUG token.state (JDBCTokenStateService.java:updateExpiration(135)) - Updated expiration for b1e78313...71886ef9a707 in the database to 1,619,026,870,461
   2021-04-20 19:41:10,477 DEBUG token.state (DefaultTokenStateService.java:renewToken(219)) - Renewed token b1e78313...71886ef9a707, expiration 2021-04-21T17:41:10.461Z
   2021-04-20 19:41:10,477 INFO  service.knoxtoken (TokenResource.java:renew(282)) - Knox Token service (sandbox) renewed the expiration for token eyJhbG...B9Ze9A (b1e78313...71886ef9a707) (renewer=admin)
   
   postgres=# select * FROM KNOX_TOKENS;
                     id                  |                        token_id                         |  issue_time   |  expiration   | max_lifetime  | username | comment 
   --------------------------------------+---------------------------------------------------------+---------------+---------------+---------------+----------+---------
    4448b054-3467-4c29-9468-d4bddfdf5e00 | [\x1A�\x15�w���\x04f\x7F+�S\x1D��\x06��\x1B�(3�س�\x1F�G | 1618940291911 | 1619026870461 | 1619545091911 | admin    | 
   (1 row)
   ```
   
   3. Used the token successfully (issued a WEBHDFS list command)
   
   4. Revoked the token
   ```
   $ curl -ku admin:admin-password -d "@token.txt" -X POST https://localhost:8443/gateway/sandbox/knoxtoken/api/v1/token/revoke
   {
     "revoked": "true"
   }
   
   
   2021-04-20 19:44:32,405 DEBUG knox.gateway (GatewayFilter.java:doFilter(116)) - Received request: POST /knoxtoken/api/v1/token/revoke
   2021-04-20 19:44:32,406 INFO  knox.gateway (KnoxLdapRealm.java:getUserDn(688)) - Computed userDn: uid=admin,ou=people,dc=hadoop,dc=apache,dc=org using dnTemplate for principal: admin
   2021-04-20 19:44:32,440 DEBUG token.state (DefaultTokenStateService.java:removeTokenState(290)) - Removed state for tokens b1e78313...71886ef9a707
   2021-04-20 19:44:32,440 DEBUG token.state (JDBCTokenStateService.java:removeToken(186)) - Token b1e78313...71886ef9a707 has been removed from the database
   2021-04-20 19:44:32,441 DEBUG token.state (DefaultTokenStateService.java:revokeToken(244)) - Revoked token b1e78313...71886ef9a707
   2021-04-20 19:44:32,443 INFO  service.knoxtoken (TokenResource.java:revoke(329)) - Knox Token service (sandbox) revoked token eyJhbG...B9Ze9A (b1e78313...71886ef9a707) (renewer=admin)
   
   postgres=# select * FROM KNOX_TOKENS;
    id | token_id | issue_time | expiration | max_lifetime | username | comment 
   ----+----------+------------+------------+--------------+----------+---------
   (0 rows)
   ```
   
   5. Confirmed it cannot be used anymore (repeated step 3.):
   ```
   $ curl -ku Passcode:b1e78313-3635-4236-bd77-71886ef9a707 https://localhost:8443/gateway/tokenbased/webhdfs/v1?op=LISTSTATUS
   <html>
   <head>
   <meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
   <title>Error 401 Unknown token: b1e78313...71886ef9a707</title>
   </head>
   <body><h2>HTTP ERROR 401 Unknown token: b1e78313...71886ef9a707</h2>
   <table>
   <tr><th>URI:</th><td>/gateway/tokenbased/webhdfs/v1</td></tr>
   <tr><th>STATUS:</th><td>401</td></tr>
   <tr><th>MESSAGE:</th><td>Unknown token: b1e78313...71886ef9a707</td></tr>
   <tr><th>SERVLET:</th><td>tokenbased-knox-gateway-servlet</td></tr>
   </table>
   
   </body>
   </html>
   
   
   2021-04-20 19:47:02,751 DEBUG knox.gateway (GatewayFilter.java:doFilter(116)) - Received request: GET /webhdfs/v1
   2021-04-20 19:47:02,761 ERROR token.state (DefaultTokenStateService.java:validateToken(321)) - Unknown token b1e78313...71886ef9a707
   2021-04-20 19:47:02,772 ERROR token.state (DefaultTokenStateService.java:validateToken(321)) - Unknown token b1e78313...71886ef9a707
   2021-04-20 19:47:02,772 WARN  federation.jwt (AbstractJWTFilter.java:validateToken(404)) - Unable to verify token expiration: org.apache.knox.gateway.services.security.token.UnknownTokenException: Unknown token: b1e78313...71886ef9a707
   org.apache.knox.gateway.services.security.token.UnknownTokenException: Unknown token: b1e78313...71886ef9a707
   	at org.apache.knox.gateway.services.token.impl.DefaultTokenStateService.validateToken(DefaultTokenStateService.java:322)
   	at org.apache.knox.gateway.services.token.impl.JDBCTokenStateService.getTokenExpiration(JDBCTokenStateService.java:110)
   	at org.apache.knox.gateway.services.token.impl.DefaultTokenStateService.getTokenExpiration(DefaultTokenStateService.java:175)
   	at org.apache.knox.gateway.provider.federation.jwt.filter.AbstractJWTFilter.getServerManagedStateExpiration(AbstractJWTFilter.java:197)
   	at org.apache.knox.gateway.provider.federation.jwt.filter.AbstractJWTFilter.tokenIsStillValid(AbstractJWTFilter.java:190)
   	at org.apache.knox.gateway.provider.federation.jwt.filter.AbstractJWTFilter.validateToken(AbstractJWTFilter.java:391)
   	at org.apache.knox.gateway.provider.federation.jwt.filter.JWTFederationFilter.doFilter(JWTFederationFilter.java:121)
   ```
   


-- 
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 #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r617711538



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {
+  static final String KNOX_TOKEN_HASH_KEY_ALIAS_NAME = "knox.token.hash.key";
+
+  private final Mac tokedMac;
+
+  TokenMAC(GatewayConfig config, AliasService aliasService) throws ServiceLifecycleException {
+    if (aliasService == null) {
+      throw new ServiceLifecycleException("Missing alias service while configuring Knox Token MAC.");
+    }
+
+    try {
+      final String algorithm = config.getKnoxTokenHashAlgorithm();
+      final char[] knoxTokenHashKey = aliasService.getPasswordFromAliasForGateway(KNOX_TOKEN_HASH_KEY_ALIAS_NAME);
+      if (knoxTokenHashKey != null) {
+        final SecretKey key = new SecretKeySpec(new String(knoxTokenHashKey).getBytes(StandardCharsets.UTF_8), algorithm);
+        tokedMac = Mac.getInstance(algorithm);
+        tokedMac.init(key);
+      } else {
+        throw new ServiceLifecycleException("Missing " + KNOX_TOKEN_HASH_KEY_ALIAS_NAME + " alias from Gateway's credential store");

Review comment:
       The master secret cannot be used as key length matters.




-- 
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 #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r617713911



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {
+  static final String KNOX_TOKEN_HASH_KEY_ALIAS_NAME = "knox.token.hash.key";
+
+  private final Mac tokedMac;
+
+  TokenMAC(GatewayConfig config, AliasService aliasService) throws ServiceLifecycleException {
+    if (aliasService == null) {
+      throw new ServiceLifecycleException("Missing alias service while configuring Knox Token MAC.");
+    }
+
+    try {
+      final String algorithm = config.getKnoxTokenHashAlgorithm();
+      final char[] knoxTokenHashKey = aliasService.getPasswordFromAliasForGateway(KNOX_TOKEN_HASH_KEY_ALIAS_NAME);
+      if (knoxTokenHashKey != null) {
+        final SecretKey key = new SecretKeySpec(new String(knoxTokenHashKey).getBytes(StandardCharsets.UTF_8), algorithm);
+        tokedMac = Mac.getInstance(algorithm);
+        tokedMac.init(key);
+      } else {
+        throw new ServiceLifecycleException("Missing " + KNOX_TOKEN_HASH_KEY_ALIAS_NAME + " alias from Gateway's credential store");
+      }
+    } catch (AliasServiceException | NoSuchAlgorithmException | InvalidKeyException e) {
+      throw new ServiceLifecycleException("Error while initiating Knox Token MAC: " + e, e);

Review comment:
       +1




-- 
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] pzampino commented on a change in pull request #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r630379211



##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
##########
@@ -141,9 +142,21 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         }
       } else if (TokenType.Passcode.equals(tokenType)) {
         // Validate the token based on the server-managed metadata
-        if (validateToken((HttpServletRequest) request, (HttpServletResponse) response, chain, tokenValue)) {
+        // The received token value must be a Base64 encoded value of Base64(tokenId)::Base64(rawPasscode)
+        String tokenId = null, passcode = null;

Review comment:
       nit: This is not typical Knox coding style

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -316,6 +334,35 @@ protected long convertCharArrayToLong(char[] charArray) {
     return Long.parseLong(new String(charArray));
   }
 
+  @Override
+  public long getTokenIssueTime(String tokenId) throws UnknownTokenException {
+    // Check the in-memory collection first, to avoid costly keystore access when possible
+    try {
+      // check the in-memory cache first
+      return super.getTokenIssueTime(tokenId);
+    } catch (UnknownTokenException e) {
+      // It's not in memory
+    }
+
+    // If there is no associated state in the in-memory cache, proceed to check the alias service
+    long issueTime = 0;
+    try {
+      char[] issueTimeStr = getPasswordUsingAliasService(tokenId + TOKEN_ISSUE_TIME_POSTFIX);
+      if (issueTimeStr == null) {
+        throw new UnknownTokenException(tokenId);
+      }
+      issueTime = convertCharArrayToLong(issueTimeStr);
+      // Update the in-memory cache to avoid subsequent keystore look-ups for the same state
+      super.setIssueTime(tokenId, issueTime);

Review comment:
       Should use the setIssueTimeInMemory() method here?




-- 
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 #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
smolnar82 commented on pull request #437:
URL: https://github.com/apache/knox/pull/437#issuecomment-823910539


   > I think it would be good to generalize the token ID hashing facility such that it could be applied to token state journaling, alias keys, etc...
   
   Yes, it's a good idea. I've just re-submitted my changes with a new class called `TokenMAC` that can be re-used.


-- 
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 #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #437:
URL: https://github.com/apache/knox/pull/437


   


-- 
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] pzampino commented on a change in pull request #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r618465498



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {

Review comment:
       +1




-- 
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 #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r617633095



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateDatabase.java
##########
@@ -28,7 +28,7 @@
 
 public class TokenStateDatabase {
   static final String TOKENS_TABLE_NAME = "KNOX_TOKENS";
-  private static final String ADD_TOKEN_SQL = "INSERT INTO " + TOKENS_TABLE_NAME + "(token_id, issue_time, expiration, max_lifetime) VALUES(?, ?, ?, ?)";
+  private static final String ADD_TOKEN_SQL = "INSERT INTO " + TOKENS_TABLE_NAME + "(id, token_id, issue_time, expiration, max_lifetime) VALUES(?, ?, ?, ?, ?)";

Review comment:
       I was thinking that we would leave this as token.id and add a passcode col that was a hash.

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {
+  static final String KNOX_TOKEN_HASH_KEY_ALIAS_NAME = "knox.token.hash.key";
+
+  private final Mac tokedMac;
+
+  TokenMAC(GatewayConfig config, AliasService aliasService) throws ServiceLifecycleException {
+    if (aliasService == null) {
+      throw new ServiceLifecycleException("Missing alias service while configuring Knox Token MAC.");
+    }
+
+    try {
+      final String algorithm = config.getKnoxTokenHashAlgorithm();
+      final char[] knoxTokenHashKey = aliasService.getPasswordFromAliasForGateway(KNOX_TOKEN_HASH_KEY_ALIAS_NAME);
+      if (knoxTokenHashKey != null) {
+        final SecretKey key = new SecretKeySpec(new String(knoxTokenHashKey).getBytes(StandardCharsets.UTF_8), algorithm);
+        tokedMac = Mac.getInstance(algorithm);
+        tokedMac.init(key);
+      } else {
+        throw new ServiceLifecycleException("Missing " + KNOX_TOKEN_HASH_KEY_ALIAS_NAME + " alias from Gateway's credential store");
+      }
+    } catch (AliasServiceException | NoSuchAlgorithmException | InvalidKeyException e) {
+      throw new ServiceLifecycleException("Error while initiating Knox Token MAC: " + e, e);
+    }
+  }
+
+  String hash(String toBeHashed) {
+    return new String(tokedMac.doFinal(toBeHashed.getBytes(StandardCharsets.UTF_8)), StandardCharsets.UTF_8);

Review comment:
       Possible NPE?

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {

Review comment:
       It seems like this should be something added to the CryptoService rather than a separate class.

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {
+  static final String KNOX_TOKEN_HASH_KEY_ALIAS_NAME = "knox.token.hash.key";
+
+  private final Mac tokedMac;
+
+  TokenMAC(GatewayConfig config, AliasService aliasService) throws ServiceLifecycleException {
+    if (aliasService == null) {
+      throw new ServiceLifecycleException("Missing alias service while configuring Knox Token MAC.");
+    }
+
+    try {
+      final String algorithm = config.getKnoxTokenHashAlgorithm();
+      final char[] knoxTokenHashKey = aliasService.getPasswordFromAliasForGateway(KNOX_TOKEN_HASH_KEY_ALIAS_NAME);
+      if (knoxTokenHashKey != null) {
+        final SecretKey key = new SecretKeySpec(new String(knoxTokenHashKey).getBytes(StandardCharsets.UTF_8), algorithm);

Review comment:
       I think we need to use salt here as well. Is this implemented somewhere else?
   Otherwise, a hash of the same token will always result in the same hash value. Since these are UUID's that is less likely but if all the passcodes are hashed with the same key then the entire set can be attacked at once rather than individually with a random salt per passcode.

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenMAC.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+
+public class TokenMAC {
+  static final String KNOX_TOKEN_HASH_KEY_ALIAS_NAME = "knox.token.hash.key";
+
+  private final Mac tokedMac;

Review comment:
       tokeDmac?




-- 
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