Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KNOX-1801 - Master secret is incorrectly assumed when a custom truststore is not specified when clientauth is enabled #63

Merged
merged 2 commits into from Mar 5, 2019

Conversation

rlevas
Copy link
Contributor

@rlevas rlevas commented Mar 5, 2019

What changes were proposed in this pull request?

Master secret is incorrectly assumed when a custom truststore is not specified when clientauth is enabled. 

Steps to reproduce

  1. Create custom TLS keystore for Knox with a custom keystore password (not the master secret)
  2. Specify the custom TLS keystore details in gateway-site.xml
    • gateway.tls.keystore.password.alias
    • gateway.tls.keystore.path
    • gateway.tls.keystore.type
    • gateway.tls.key.alias
    • gateway.tls.key.passphrase.alias (optional)
  3. Turn on client-auth
    • gateway.client.auth.needed : {{true}}
  4. Create password alias for the custom keystore using Knox CLI
    • bin/knoxcli.sh create-alias gateway-identity-keystore-password --value <password>
  5. (Re)Start the Gateway

The Gateway will fail to start with the following error in the gateway.log:

2019-03-04 11:03:15,921 FATAL knox.gateway (GatewayServer.java:main(168)) - Failed to start gateway: java.io.IOException: keystore password was incorrect
java.io.IOException: keystore password was incorrect
        at sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2059)
        at java.security.KeyStore.load(KeyStore.java:1445)
        at org.apache.knox.gateway.services.security.impl.JettySSLService.loadKeyStore(JettySSLService.java:257)
        at org.apache.knox.gateway.services.security.impl.JettySSLService.buildSslContextFactory(JettySSLService.java:222)
        at org.apache.knox.gateway.GatewayServer.createConnector(GatewayServer.java:373)
        at org.apache.knox.gateway.GatewayServer.start(GatewayServer.java:520)
        at org.apache.knox.gateway.GatewayServer.startGateway(GatewayServer.java:308)
        at org.apache.knox.gateway.GatewayServer.main(GatewayServer.java:161)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.knox.gateway.launcher.Invoker.invokeMainMethod(Invoker.java:68)
        at org.apache.knox.gateway.launcher.Invoker.invoke(Invoker.java:39)
        at org.apache.knox.gateway.launcher.Command.run(Command.java:99)
        at org.apache.knox.gateway.launcher.Launcher.run(Launcher.java:75)
        at org.apache.knox.gateway.launcher.Launcher.main(Launcher.java:52)
Caused by: java.security.UnrecoverableKeyException: failed to decrypt safe contents entry: javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
        ... 17 more

Solution
Lookup password for the truststore using the appropriate alias name, falling back to the master secret if an alias is not configured or not set.

While at it, cleaned up org.apache.knox.gateway.services.security.impl.JettySSLService to remove unneeded MasterService calls.

How was this patch tested?

Added new unit tests - org.apache.knox.gateway.services.security.impl.JettySSLServiceTest

mvn verify -Prelease,package -pl gateway-server
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 47.321 s
[INFO] Finished at: 2019-03-05T12:08:21-05:00
[INFO] Final Memory: 101M/1111M
[INFO] ------------------------------------------------------------------------

Manually tested various scenarios.

Please review Knox Contributing Process before opening a pull request.

…tore is not specified when clientauth is enabled
@rlevas
Copy link
Contributor Author

rlevas commented Mar 5, 2019

FYI @risdenk , @lmccay , @smolnar82 , @moresandeep , @pzampino

@risdenk risdenk self-requested a review March 5, 2019 17:15
Copy link
Contributor

@risdenk risdenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall really like the changes. Nice cleanup and thanks for adding the test. Minor questions/changes requested.

@risdenk risdenk self-assigned this Mar 5, 2019
…y.SSLService#buildSslContextFactory, Removed EasyMockSupport from org.apache.knox.gateway.services.security.impl.JettySSLServiceTest
@risdenk risdenk merged commit 1262230 into apache:master Mar 5, 2019
@rlevas rlevas deleted the KNOX-1801 branch March 11, 2019 19:15
twmarshall pushed a commit to twmarshall/knox that referenced this pull request Aug 27, 2019
…custom truststore is not specified when clientauth is enabled (apache#63)

Change-Id: Ia712fc08fab2b3d6bc256712481f9e58af1ca37f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants