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

Change protocol name to TLS in QuarkusRestClientBuilder #24182

Merged
merged 1 commit into from Mar 9, 2022

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Mar 8, 2022

This PR only changes SSL to TLS; I haven't verified if in fact on Java 11 SSL still defaults to TLSv1.2 but even if does it is better to have TLS to avoid various scanners reporting false positives.

I think having TLS is the most flexible option because (quoting some lines from Java Crypto docs):

  • TLS 1.2 has been the default-enabled TLS protocol since Java 8
  • TLS protocols provide a built-in mechanism to negotiate the specific protocol version to use. When a client connects to a server, it announces the highest version it can support

So if we explicitly start with TLS1.2 then it may fail against the legacy servers which only implement TLS 1.0 or just don't correctly implement TLS version negotiation.

With TLS Quarkus client will start with TLSv1.2 but will be able to drop to TLSv1.0 if necessary.

Also CC @cescoffier

@sberyozkin sberyozkin marked this pull request as draft March 8, 2022 16:29
@sberyozkin sberyozkin marked this pull request as ready for review March 8, 2022 16:37
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 8, 2022

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building 957fbb4

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build ⚠️ Check → Logs Raw logs
Attach pull request number ⚠️ Check → Logs Raw logs
CI Sanity Check ⚠️ Check → Logs Raw logs

@gsmet
Copy link
Member

gsmet commented Mar 8, 2022

This will have to wait. CI is overloaded right now, cancelling the build.

@sberyozkin
Copy link
Member Author

No problems, it can wait

@gsmet gsmet merged commit a0dbd8f into quarkusio:main Mar 9, 2022
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Mar 9, 2022
@gsmet gsmet modified the milestones: 2.8 - main, 2.7.4.Final Mar 9, 2022
@sberyozkin sberyozkin deleted the quarkus_rest_client_builder_tls branch March 9, 2022 10:22
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.

None yet

3 participants