Switch to rely on Netty for Hostname Verification

[michaeljmarshall]

Motivation

Currently, we perform hostname verification for the Java Client and the Proxy Java Client using a custom Pulsar hostname verifier. In order to simplify the code, I propose that we refactor these clients so they rely on a Netty, its SslHandler, and the JVM, to perform the hostname verification.

When HTTPS is configured as the endpoint verification algorithm, it uses RFC 2818 to perform hostname verification. This is defined by the Java Security Standard Algorithm Names documentation for JDK versions 8, 11, and 17. Here are the official docs:

• https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html
• https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html
• https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html

Modifications

• Update the Java Client so that it configures the SslHandler's SslEngine to use HTTPS for endpoint verification and remove unnecessary custom hostname verification logic.
• Update the Proxy's DirectProxyHandler class so that it configures the SslHandler to perform hostname verification and so that the proxy handler itself does not perform that verification.
• Make it possible to disable hostname verification checking in the HttpClient used by the HTTP Lookup Client code in the Java Client. Currently, it defaults to being always enabled.

Verifying this change

There are tests that already cover the changes, and I performed integration testing on a minikube cluster with Cert-Manager created certs.

Does this pull request potentially affect one of the following parts:

This change deprecates support for CN matching in the Pulsar Java Client. A future change should remove this support from the Pulsar Admin Client, which relies on Pulsar's verifier that still supports CN matching.

Documentation

There is no need for documentation. This is an internal change.

• doc-not-needed

[github-actions]

@michaeljmarshall:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

[github-actions]

@michaeljmarshall:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

[github-actions]

@michaeljmarshall:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

[eolivelli]

Lgtm

[eolivelli]

/pulsarbot rerun-failure-checks

[eolivelli]

/pulsarbot rerun-failure-checks

[nicoloboschi]

Lgtm

[nodece]

LGTM

I have a quest:

If a subjectAltName extension of type dNSName is present, that MUST
be used as the identity. Otherwise, the (most specific) Common Name
field in the Subject field of the certificate MUST be used. Although
the use of the Common Name is existing practice, it is deprecated and
Certification Authorities are encouraged to use the dNSName instead.

Do your changes follow up on this rule?

[lhotari]

There are 2 test failures:

• org.apache.pulsar.client.api.AuthenticationTlsHostnameVerificationTest ► testTlsSyncProducerAndConsumerWithInvalidBrokerHost
• org.apache.pulsar.proxy.server.ProxyWithAuthorizationTest ►
  testTlsHostVerificationProxyToClient

I debugged and fixed the first one. The test was invalid.
I pushed the commit to my fork: lhotari@7c9545ec

One observation is that when the handshake fails because of hostname verification, the client will keep on retrying for some time. Here's an example log: https://gist.github.com/lhotari/8455fc155e0c7526398f36e9bd0bd88c
(can be reproduced by running AuthenticationTlsHostnameVerificationTest with the above commit)

[lhotari]

I got both failing tests fixed. I pushed my change to my fork and I'm running tests there: lhotari#72

[lhotari]

@michaeljmarshall I'll rebase this PR and include my changes to fix the tests. I hope this is fine.

[lhotari]

@michaeljmarshall It seems that when you opened the PR you didn't grant access to others for collaborating in the PR. I pushed changes to my fork in master...lhotari:use-netty .

one way to make your branch to match the changes:

git pull https://github.com/lhotari/pulsar.git use-netty
git reset --hard FETCH_HEAD

[michaeljmarshall]

@lhotari thank you for finding and fixing those tests! I just gave maintainers access to collaborate. If you’re no longer available to push, I’ll get the commits in soon.

[github-actions]

@michaeljmarshall Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.