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

Security enhancement for the JTOpen library #200

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MarcelRomijn
Copy link

Added a security enhancement that allows setting a custom SSLSocketFactory for making secure connections to a host.
This enhancement is available for both JTOpen connections made with the SecureAS400 class and with the AS400JDBCDriver class.

I created another GitHub repository that contains tester code to test/demonstrate the security enhancement (in branch feature/security): https://github.com/MarcelRomijn/JTOpen_security_test/tree/feature/security

@nadiramra
Copy link
Member

nadiramra commented Oct 8, 2024

Not sure we will be going this way. I think our thought is giving users the ability to specify path to certificate store with a password, and the additional option to indicate whether or not all certificates should be trusted (in which case no certificate store needs to be specified).

@MarcelRomijn
Copy link
Author

@nadiramra, only specifying a path to a certificate store with a password will not work for us.
We use JTOpen in a server environment where a configuration is dynamically set. Through that configuration, a collection of trusted certificates are obtained. At runtime, an SSLSocketFactory is built, which is then handed over to JTOpen.

One option could be to implement both:

  1. The current implementation in the PR, which allows for maximum flexibility by dynamically creating an instance of `SSLSocketFactory'.
  2. An extra API, where the path to a certificate store with password can be provided. And maybe also a variant where an instance of KeyStore can be provided in case the user has already read it.

Obviously, that second extra API can make use of the first one.

@trknz
Copy link

trknz commented Oct 23, 2024

Not sure we will be going this way. I think our thought is giving users the ability to specify path to certificate store with a password, and the additional option to indicate whether or not all certificates should be trusted (in which case no certificate store needs to be specified).

@nadiramra, could you provide an estimated timeframe? Even a rough indication would be helpful.

@ThePrez
Copy link
Member

ThePrez commented Dec 26, 2024

@nadiramra, only specifying a path to a certificate store with a password will not work for us. We use JTOpen in a server environment where a configuration is dynamically set. Through that configuration, a collection of trusted certificates are obtained. At runtime, an SSLSocketFactory is built, which is then handed over to JTOpen.

One option could be to implement both:

1. The current implementation in the PR, which allows for maximum flexibility by dynamically creating an instance of `SSLSocketFactory'.

2. An extra API, where the path to a certificate store with password can be provided. And maybe also a variant where an instance of `KeyStore` can be provided in case the user has already read it.

Obviously, that second extra API can make use of the first one.

I understand the use case for both, and I think we will inevitably need both.

@ThePrez
Copy link
Member

ThePrez commented Dec 26, 2024

@MarcelRomijn as you can see I pushed in some changes that would honor both a custom SSLSocketFactory object or, alternatively, a custom truststore filename and password.

I did some basic testing and have more to do, but will you please verify that the latest version of this branch works for your needs?

@MarcelRomijn
Copy link
Author

@ThePrez,

I saw the update to be able to provide a truststore filename and password for the JDBC connections.
Also the extra option to provide "*ANY" and have a trust-all SSLSocketFactory.

I copied my JDBC test TestDatabaseAccessSecure to:

  • TestDatabaseAccessSecureExternal, which stores the retrieved truststore in a temporary file and uses the filename and password to create a secure JDBC connection.
  • TestDatabaseAccessSecureAny, which uses *ANY for the truststore filename and password to create a secure JDBC connection.
    Both these tests work well.

A small remark...
I saw that JDProperties expects the KeyStore file to be of type JKS. As of Java 9, the default KeyStore type changed to PKCS12.
Fortunately, when a KeyStore is created of type JKS, it uses a KeyStore SPI that accepts both JKS and PKCS12. So my test worked, even though I provided a KeyStore of type PKCS12 😉
I know that by far most KeyStores will be either of type JKS or PKCS12. But would it be an option to add another attribute to JDProperties to specify the KeyStore type, in case someone uses a different KeyStore type?

But even without an extra KeyStore type attribute in JDProperties, the current branch code-base works well for me... Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants