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

[improve][broker-web&websocket&proxy&function-worker] Full-support set ssl provider, ciphers and protocols #13740

Merged
merged 2 commits into from
May 1, 2022

Conversation

nodece
Copy link
Member

@nodece nodece commented Jan 13, 2022

Signed-off-by: Zixuan Liu [email protected]

Fixes #13734

Motivation

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when tlsEnabledWithKeyStore=false

Modifications

  • Add org.apache.pulsar.jetty.tls package in pulsar-broker-common for Jetty TLS support
  • Add a new webServiceTlsProvider=Conscrypt to broker and proxy config
  • Update Conscrypt as the tlsProvider value in websocket config

In the old version, we implicitly use the Conscrypt provider, now we need to set it explicitly.

Documentation

Need to update docs?

  • doc-required

Effected version

  • Pulsar 2.8.x
  • Pulsar 2.9.x

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 13, 2022
@nodece nodece marked this pull request as draft January 13, 2022 15:13
@nodece nodece force-pushed the fix_tls_config branch 2 times, most recently from 642124f to ea13113 Compare January 14, 2022 04:49
@nodece nodece marked this pull request as ready for review January 14, 2022 04:50
@nodece nodece force-pushed the fix_tls_config branch 3 times, most recently from 2aea92d to fccc371 Compare January 14, 2022 08:52
@nodece nodece marked this pull request as draft January 14, 2022 16:38
@nodece nodece force-pushed the fix_tls_config branch 2 times, most recently from 2581c78 to 74c462a Compare January 15, 2022 15:18
@nodece nodece marked this pull request as ready for review January 15, 2022 15:25
limbonicat
limbonicat previously approved these changes Jan 17, 2022
@nodece nodece changed the title [Broker] Fix config ciphers and protocols to web server [Broker] Fix handshake failure on web service Jan 17, 2022
@nodece nodece force-pushed the fix_tls_config branch 2 times, most recently from 2c82a67 to 4f36929 Compare January 17, 2022 03:32
@nodece nodece changed the title [Broker] Fix handshake failure on web service [Security] Fix handshake failure on web service Jan 17, 2022
@nodece
Copy link
Member Author

nodece commented Jan 17, 2022

/pulsarbot run-failure-checks

tuteng
tuteng previously approved these changes Jan 17, 2022
Copy link
Member

@tuteng tuteng left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

@nodece - would you please add a README.md file to the resources directories where the certificates and the .jks files are added explaining how they were created/generated? Adding this documentation will help any future troubleshooting that might be necessary for these tests, and it'll help someone verify the files, if they would like to do so. Thanks!

@BewareMyPower
Copy link
Contributor

where the certificates and the .jks files are added explaining how they were created/generated?

I've asked the question offline before. It looks like these binaries are copied from #13354 (I'm not sure why the sizes are a little different), whose documents will be added later. I'm just wondering that is there a good way to reference the same resources in two modules?

@nodece
Copy link
Member Author

nodece commented Apr 28, 2022

@nodece - would you please add a README.md file to the resources directories where the certificates and the .jks files are added explaining how they were created/generated? Adding this documentation will help any future troubleshooting that might be necessary for these tests, and it'll help someone verify the files, if they would like to do so. Thanks!

Thanks for your point!

I want to do this next PR, we have multiple public certificates and keystore file in Pulsar, these also need to improve.

@michaeljmarshall

@nodece nodece requested a review from michaeljmarshall April 28, 2022 06:19
@nodece
Copy link
Member Author

nodece commented Apr 28, 2022

@eolivelli Could you help review this PR?

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@michaeljmarshall
Copy link
Member

Thanks for clarifying @BewareMyPower and @nodece. It makes sense to add in another PR, especially if we have multiple resources to document.

@nodece
Copy link
Member Author

nodece commented Apr 29, 2022

Thanks for clarifying @BewareMyPower and @nodece. It makes sense to add in another PR, especially if we have multiple resources to document.

@michaeljmarshall Thanks, could you approve this PR? then I do these things.

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

@nodece
Copy link
Member Author

nodece commented Apr 30, 2022

/pulsarbot rerun-failure-checks

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

nodece added 2 commits April 30, 2022 18:19
…t ssl provider, ciphers and protocols

Signed-off-by: Zixuan Liu <[email protected]>
@nodece
Copy link
Member Author

nodece commented Apr 30, 2022

Forch-pushed for rebase the master branch.

@nodece
Copy link
Member Author

nodece commented Apr 30, 2022

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit bf15e83 into apache:master May 1, 2022
codelipenghui pushed a commit that referenced this pull request May 5, 2022
…t ssl provider, ciphers and protocols (#13740)

Fixes #13734

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when `tlsEnabledWithKeyStore=false`

- Add `org.apache.pulsar.jetty.tls` package in pulsar-broker-common for Jetty TLS support
- Add a new `webServiceTlsProvider=Conscrypt` to broker and proxy config
- Update `Conscrypt` as the `tlsProvider` value in websocket config

In the old version, we implicitly use the `Conscrypt` provider, now we need to set it explicitly.

(cherry picked from commit bf15e83)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label May 5, 2022
codelipenghui pushed a commit that referenced this pull request May 5, 2022
…t ssl provider, ciphers and protocols (#13740)

Fixes #13734

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when `tlsEnabledWithKeyStore=false`

- Add `org.apache.pulsar.jetty.tls` package in pulsar-broker-common for Jetty TLS support
- Add a new `webServiceTlsProvider=Conscrypt` to broker and proxy config
- Update `Conscrypt` as the `tlsProvider` value in websocket config

In the old version, we implicitly use the `Conscrypt` provider, now we need to set it explicitly.

(cherry picked from commit bf15e83)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label May 5, 2022
@Anonymitaet Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels May 5, 2022
codelipenghui pushed a commit that referenced this pull request May 20, 2022
…t ssl provider, ciphers and protocols (#13740)

Fixes #13734

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when `tlsEnabledWithKeyStore=false`

- Add `org.apache.pulsar.jetty.tls` package in pulsar-broker-common for Jetty TLS support
- Add a new `webServiceTlsProvider=Conscrypt` to broker and proxy config
- Update `Conscrypt` as the `tlsProvider` value in websocket config

In the old version, we implicitly use the `Conscrypt` provider, now we need to set it explicitly.

(cherry picked from commit bf15e83)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 23, 2022
…t ssl provider, ciphers and protocols (apache#13740)

Fixes apache#13734

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when `tlsEnabledWithKeyStore=false`

- Add `org.apache.pulsar.jetty.tls` package in pulsar-broker-common for Jetty TLS support
- Add a new `webServiceTlsProvider=Conscrypt` to broker and proxy config
- Update `Conscrypt` as the `tlsProvider` value in websocket config

In the old version, we implicitly use the `Conscrypt` provider, now we need to set it explicitly.

(cherry picked from commit bf15e83)
(cherry picked from commit fb0cb76)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jun 1, 2022
…t ssl provider, ciphers and protocols (apache#13740)

Fixes apache#13734

Pulsar doesn't set ssl provider, ciphers and protocols to the web, websocket and proxy service when `tlsEnabledWithKeyStore=false`

- Add `org.apache.pulsar.jetty.tls` package in pulsar-broker-common for Jetty TLS support
- Add a new `webServiceTlsProvider=Conscrypt` to broker and proxy config
- Update `Conscrypt` as the `tlsProvider` value in websocket config

In the old version, we implicitly use the `Conscrypt` provider, now we need to set it explicitly.

(cherry picked from commit bf15e83)
(cherry picked from commit b28f541)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-complete Your PR changes impact docs and the related docs have been already added. doc-not-needed Your PR changes do not impact docs release/2.8.4 release/2.9.3 release/2.10.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Websocket Client TLS not supported on Win7/8 (handshake failure)