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

PAYARA-3997 Add TLS 1.3 support when using Zulu JDK 8 #4243

Merged
merged 5 commits into from Oct 11, 2019

Conversation

@MeroRai
Copy link
Contributor

MeroRai commented Oct 1, 2019

Description

This is a feature to add TLS 1.3 support when using Zulu JDK 1.8_222 and higher. This possible due to OpenJSEE project, OpenJSEE is a JSEE provider created by Azul to support TLS 1.3 on Java SE 8. You can enable TLS 1.3 support with the Java option --XX:+UseOpenJSSE. See TLS 1.3 Support in Zulu
8 with OpenJSSE
for more information.

Important Info

  • Implemented vendor specific JVM options, since TLS 1.3 is only supported in Zulu JDK. E.g. [Azul-1.8.0u222|1.8.0u500]-XX:+UseOpenJSSE this option will on included if the JDK vendor is Azul.

  • The checkbox to enable TLS 1.3 should not be visible if you are using Zulu JDK lower 1.8_222 or JDK 8 provided by other vendors.

  • You will need to add the following Java Options to the asadmin script for TLS 1.3 to work on the asadmin CLI:

    • -XX:+UseOpenJSSE flag makes OpenJSSE default TLS provider
    • -Dfish.payara.clientHttpsProtocol=TLSv1.3

Testing

New tests

None

Testing Performed

  • Manual testing
    • Enabled TLS 1.3 on admin-listener and enabled secure admin. Restarted the domain and checked if it is indeed using TLS 1.3 by going to developer tools in Chrome, clicking on the Security tab and view connection details. More information regarding this can be found here: https://support.cloudflare.com/hc/en-us/articles/227172348-Understanding-TLS-1-3.
    • Tested it on Windows and Elementary OS with Oracle, Azul Zulu JDK, AdoptOpenJDK, Red Hat OpenJDK and Amazon Corretto.
  • local build test

Testing Environment

Zulu JDK 1.8_222 on Elementary OS 0.4.1 Loki with Maven 3.5.4

Documentation

TODO

Notes for Reviewers

@MeroRai MeroRai requested a review from Pandrex247 Oct 1, 2019
@MeroRai MeroRai self-assigned this Oct 1, 2019
@MeroRai

This comment has been minimized.

Copy link
Contributor Author

MeroRai commented Oct 1, 2019

jenkins test please

@smillidge

This comment has been minimized.

Copy link
Contributor

smillidge commented Oct 1, 2019

Can you describe the testing performed rather than just "manual testing"

@MeroRai MeroRai requested a review from Cousjava Oct 2, 2019
@MeroRai MeroRai changed the title PAYARA-3545 Add TLS 1.3 support when using Zulu JDK 8 PAYARA-3997 Add TLS 1.3 support when using Zulu JDK 8 Oct 2, 2019
@smillidge

This comment has been minimized.

Copy link
Contributor

smillidge commented Oct 2, 2019

Have you tested the negative cases;
Does the option disappear when the underlying JVM does not support TLS 1.3?
Does the domain still start after your changes on a non-Zulu 8.x JVM?
Can you enable TLS 1.3 on JDK 11 OK?

Also
Is there a PR to backport to 4.x as this is targeting master?

MeroRai added 2 commits Oct 7, 2019
@MeroRai

This comment has been minimized.

Copy link
Contributor Author

MeroRai commented Oct 7, 2019

@smillidge, I have now tested all the test cases you mentioned. In regards, to backport to 4.x, I have made a PR in BitBucket.

@MeroRai

This comment has been minimized.

Copy link
Contributor Author

MeroRai commented Oct 8, 2019

jenkins test please

@Pandrex247

This comment has been minimized.

Copy link
Member

Pandrex247 commented Oct 8, 2019

Jenkins test please

Copy link
Member

Pandrex247 left a comment

One minor comment, but seems good to me. Gave it a little razz and seemed to work.
Noice.

@@ -1156,6 +1156,7 @@ ssl.tlsLabel=TLS:
ssl.tlsLabel11=TLS1.1:
ssl.tlsLabel12=TLS1.2:
ssl.tlsLabel13=TLS1.3:
ssl.tlsLabel13Help=<b>-Dfish.payara.clientHttpsProtocol=TLSv1.3</b> java option will need to be added to the asadmin script for TLS 1.3 to work with asadmin CLI. If you are using Zulu JDK 8, you will also need to add the following Java Option: <b>-XX:+UseOpenJSSE</b>, this option makes OpenJSSE default TLS provider.

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Oct 8, 2019

Member

I'd scrap the this option makes OpenJSSE default TLS provider bit, I think the sentence works fine without it.

@MeroRai

This comment has been minimized.

Copy link
Contributor Author

MeroRai commented Oct 9, 2019

jenkins test please

@smillidge

This comment has been minimized.

Copy link
Contributor

smillidge commented Oct 9, 2019

I have concerns about the changes to the parsing of the versioned JVM parameters I think this needs extensive testing on a number of setups and JVMs to ensure there is not a raft of bug reports when this goes out.

@MeroRai

This comment has been minimized.

Copy link
Contributor Author

MeroRai commented Oct 11, 2019

jenkins test please

@MeroRai MeroRai merged commit 052f0d2 into payara:master Oct 11, 2019
51 of 55 checks passed
51 of 55 checks passed
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) Test in progress
security/snyk - appserver/persistence/pom.xml (payara-ci) Test in progress
security/snyk - appserver/pom.xml (payara-ci) Test in progress
security/snyk - appserver/resources/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.