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

Refactor MakeTLSConfig and support mTLS on server #739

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

tie
Copy link
Contributor

@tie tie commented Nov 26, 2023

Proposed changes

This PR adds tests for and refactors tools.MakeTLSConfig and then adds support for enabling mutual TLS authentication for Centrifugo servers by setting ClientCAs and ClientAuth in tls.Config.

@tie tie force-pushed the mutual-tls branch 2 times, most recently from 7b88e8e to bf9ba76 Compare November 27, 2023 08:05
@FZambia
Copy link
Member

FZambia commented Nov 29, 2023

Many thanks! Could you please fix lint warnings? Otherwise, seems great – I'll take a closer look one more time soon. One question, to understand the impact - in your use case, for which Centrifugo component you want to use mTLS?

tie added 3 commits November 29, 2023 10:49
This change is a preparation for refactoring MakeTLSConfig and adds
tests to ensure that we keep the current behavior. To make testing
easier, we pass os.ReadFile as an argument and use fstest.MapFS.ReadFile
in tests. In particular, this also means avoiding tls.LoadX509KeyPair.
This change refactors TLS configuration builder and improves error
messages to contain configuration key that caused an error.
This change adds support for enabling mutual TLS authentication for
Centrifugo servers.
@tie
Copy link
Contributor Author

tie commented Nov 29, 2023

@FZambia, fixed linter warnings.

for which Centrifugo component you want to use mTLS

We are using Centrifugo behind a load balancer and with mTLS to secure traffic between load balancers and Centrifugo instances. Currently, we are using Ghostunnel to proxy TLS connections to Centrifugo running on localhost, however, we find this less reliable than exposing mTLS-authenticated Centrifugo directly.

@FZambia FZambia self-requested a review December 1, 2023 08:30
Copy link
Member

@FZambia FZambia left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@FZambia FZambia merged commit 6d3dd3f into centrifugal:master Dec 5, 2023
2 checks passed
@tie tie deleted the mutual-tls branch March 28, 2024 14:54
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.

2 participants