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

[PIP-60] Add TLS SNI support for cpp and python clients #8957

Merged
merged 2 commits into from
Dec 24, 2020

Conversation

deonvdv
Copy link
Contributor

@deonvdv deonvdv commented Dec 14, 2020

Motivation

Implementation of PIP-60
A proxy server is a go‑between or intermediary server that forwards requests from multiple clients to different servers across the Internet. The proxy server can act as a “traffic cop,” in both forward and reverse proxy scenarios, and adds various benefits in your system such as load balancing, performance, security, auto-scaling, etc.. There are already many proxy servers already available in the market which are fast, scalable and more importantly covers various essential security aspects that are needed by the large organization to securely share their confidential data over the network. Pulsar already provides proxy implementation PIP-1 which acts as a reverse proxy and creates a gateway in front of brokers. However, pulsar doesn’t provide support to use other proxies such as Apache traffic server (ATS), HAProxy, Nginx, Envoy those are more scalable and secured. Most of these proxy-servers support SNI ROUTING which can route traffic to a destination without having to terminate the SSL connection. Routing at layer 4 gives greater transparency because the outbound connection is determined by examining the destination address in the client TCP packets.
Netty supports sending SNI header on TLS handshake and this PR uses that Netty feature to send SNI header while connecting to proxy.

Modification

https://github.com/apache/pulsar/wiki/PIP-60:-Support-Proxy-server-with-SNI-routing:-Support-Proxy-server-with-SNI-routing#changes

@deonvdv deonvdv changed the title Add TLS SNI support for cpp and python clients [PIP-60] Add TLS SNI support for cpp and python clients Dec 14, 2020
@sijie
Copy link
Member

sijie commented Dec 14, 2020

@BewareMyPower Can you review this pull request?

@BewareMyPower
Copy link
Contributor

For the format issues, you should format your code by clang-format 5.0. Besides, Pulsar C++ client uses camel case but not snake case, though there's no related check like format.

@BewareMyPower
Copy link
Contributor

Also, I think a unit test is required for verification, like #6566 did.

By the way, the PR description should not just copy from the PIP 60

Netty supports sending SNI header on TLS handshake and this PR uses that Netty feature to send SNI header while connecting to proxy.

Netty is for Java client.

@BewareMyPower
Copy link
Contributor

/pulsarbot run-failure-checks

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@sijie
Copy link
Member

sijie commented Dec 23, 2020

@jiazhai @BewareMyPower Can you review this PR?

@sijie sijie added the doc-required Your PR changes impact docs and you will update later. label Dec 24, 2020
@sijie sijie merged commit f018892 into apache:master Dec 24, 2020
@Anonymitaet
Copy link
Member

@deonvdv thanks for your great work. Would you like to add docs accordingly? Then I can help review, thanks

@Anonymitaet Anonymitaet removed the doc-required Your PR changes impact docs and you will update later. label Apr 9, 2021
@eolivelli
Copy link
Contributor

@BewareMyPower @merlimat @sijie
is it safe to pick this to 2.7.2 ?

zymap pushed a commit that referenced this pull request Apr 14, 2021
* Add TLS SNI support for cpp and python clients

(cherry picked from commit f018892)
@zymap zymap added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Apr 14, 2021
@rdhabalia
Copy link
Contributor

I am confused here. How does this change support SNI routing in client side.
To support SNI proxy routing, client needs two URL: sni-proxy-url + actual broker service URL as we have added in Java client library: #6566

ClientBuilder clientBuilder = PulsarClient.builder()
		.serviceUrl(brokerServiceUrl)
        .proxyServiceUrl(proxyUrl, ProxyProtocol.SNI);

I am really confused what exactly we are trying to do here and how is it related to SNI proxy?

I also checked CPP + SNI doc PR which is also kind of misleading the usage: https://github.com/apache/pulsar/pull/9937/files

can someone please help me to understand how does it actually perform SNI routing?

@rdhabalia
Copy link
Contributor

I see multiple approvals, can someone please explain what exactly this PR is doing? this PR has just added an error log and moving lines. ?

@BewareMyPower
Copy link
Contributor

this PR has just added an error log and moving lines. ?

No. The core change of this PR is that SSL_set_tlsext_host_name is called to set the server's hostname for the SNI routing. See https://www.openssl.org/docs/man1.1.1/man3/SSL_set_tlsext_host_name.html

@BewareMyPower
Copy link
Contributor

To support SNI proxy routing, client needs two URL: sni-proxy-url + actual broker service URL as we have added in Java client library

While I agree the implementation of this PR is different with the original PIP-60, the actual broker service URL is retrieved from the lookup response.

https://github.com/apache/pulsar-client-cpp/blob/b242e1a0d8f316730ed5bfdd792c471ab2b6680c/lib/ClientImpl.cc#L537

The 2nd argument passed to ConnectionPool::getConnectionAsync is the serviceUrl in this PR. So in C++ client, users don't need to configure the actual broker service URL explicitly. Instead, this URL is returned according to the advertisedListeners or advertisedAddress config in broker.

@rdhabalia
Copy link
Contributor

rdhabalia commented Sep 19, 2023

@BewareMyPower
Thank you for your reply but unfortunately SNI doesn't work that way. In SNI routing, user provides SNI host to the proxy and proxy routes the request to that host and creates a tunnel between user and the actual host. as we have already explained into PIP-60 in detail with TCP routing protocol.

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

Successfully merging this pull request may close these issues.

None yet

9 participants