Skip to content

Conversation

@michaeljmarshall
Copy link
Member

Fixes #19175 #19587

Motivation

In #15824, we added the subject alternative name to the certs for some tests. This is problematic for some tests that were expecting hostname verification to fail due to missing SANs. This PR adds a new cert without the SANs to ensure correct coverage.

One nuance is that I updated my /etc/ssl/openssl.cnf file to include the following config:

[ v3_ca ]
basicConstraints = critical,CA:TRUE
subjectKeyIdentifier = hash
authorityKeyIdentifier = keyid:always,issuer:always

I also had trouble with the -text output (not sure why), so I updated how the certs are written to files. That is likely dependent on the version of openssl.

Modifications

  • Update the certificates for ProxyWithAuthorizationTest to ensure correct test coverage

Verifying this change

This is a test fix.

Documentation

  • doc

This change includes updates to relevant comments for internal testing documentation.

Matching PR in forked repository

PR in forked repository: skipping because tests pass locally

@eolivelli
Copy link
Contributor

@nicoloboschi is this related to your latest PR?

@nicoloboschi
Copy link
Contributor

@nicoloboschi is this related to your latest PR?

I don't think so. The issues were created a lot of times before that regression

@nicoloboschi nicoloboschi merged commit f292bad into apache:master Feb 22, 2023
@michaeljmarshall michaeljmarshall deleted the fix-proxy-auth-test branch February 22, 2023 17:10
michaeljmarshall added a commit that referenced this pull request Feb 22, 2023
michaeljmarshall added a commit that referenced this pull request Feb 22, 2023
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Feb 22, 2023
…ache#19594)

(cherry picked from commit f292bad)
(cherry picked from commit 6576231)
(cherry picked from commit 14152fc)
michaeljmarshall added a commit to datastax/pulsar that referenced this pull request Feb 23, 2023
…ache#19594)

(cherry picked from commit f292bad)
(cherry picked from commit 6576231)
(cherry picked from commit 14152fc)
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Feb 23, 2023
…ache#19594)

(cherry picked from commit f292bad)
(cherry picked from commit 6576231)
(cherry picked from commit 14152fc)
(cherry picked from commit fbf4c17)
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Feb 23, 2023
…ache#19594)

(cherry picked from commit f292bad)
(cherry picked from commit 6576231)
(cherry picked from commit 14152fc)
(cherry picked from commit fbf4c17)
@michaeljmarshall
Copy link
Member Author

In talking with @lhotari offline, this PR's motivation might not have been clearly explained. Here is my justification:

The tests that I updated to use the certs without subject alternative names are tests that ensure the client does not connect to a proxy/broker when the cert fails hostname verification. When localhost and 127.0.0.1 were added as subject alternative names, these tests failed on personal machines (meaning connections were made), but appear to have passed (most of the time) on the CI machines (meaning connections were closed). In the local environment, the tests failed because the certs correctly had a hostname that matched the SAN.

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.

Flaky-test: ProxyWithAuthorizationTest.testTlsHostVerificationProxyToBroker Flaky-test: ProxyWithAuthorizationTest.testProxyAuthorization

3 participants