You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "michaeljmarshall (via GitHub)" <gi...@apache.org> on 2023/02/22 06:51:01 UTC

[GitHub] [pulsar] michaeljmarshall opened a new pull request, #19594: [fix][test] ProxyWithAuthorizationTest remove SAN from test certs

michaeljmarshall opened a new pull request, #19594:
URL: https://github.com/apache/pulsar/pull/19594

   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:
   
   ```cnf
   [ 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
   
   - [x] `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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] nicoloboschi merged pull request #19594: [fix][test] ProxyWithAuthorizationTest remove SAN from test certs

Posted by "nicoloboschi (via GitHub)" <gi...@apache.org>.
nicoloboschi merged PR #19594:
URL: https://github.com/apache/pulsar/pull/19594


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] michaeljmarshall commented on pull request #19594: [fix][test] ProxyWithAuthorizationTest remove SAN from test certs

Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #19594:
URL: https://github.com/apache/pulsar/pull/19594#issuecomment-1447170779

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] nicoloboschi commented on pull request #19594: [fix][test] ProxyWithAuthorizationTest remove SAN from test certs

Posted by "nicoloboschi (via GitHub)" <gi...@apache.org>.
nicoloboschi commented on PR #19594:
URL: https://github.com/apache/pulsar/pull/19594#issuecomment-1440443194

   > @nicoloboschi is this related to your latest PR?
   
   I don't think so. The issues were created a lot of times before that regression 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] eolivelli commented on pull request #19594: [fix][test] ProxyWithAuthorizationTest remove SAN from test certs

Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #19594:
URL: https://github.com/apache/pulsar/pull/19594#issuecomment-1439917258

   @nicoloboschi is this related to your latest PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org