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

Client certificate chain is now sent #4894

Open
wants to merge 9 commits into
base: dev_kip848_mock_handler_and_integration_tests
Choose a base branch
from

Conversation

emasab
Copy link
Contributor

@emasab emasab commented Nov 6, 2024

when using ssl.certificate.pem or ssl_certificate or ssl.keystore.location.
Without that, broker must explicitly add any intermediate certification
authority certificate to its truststore to be able to accept client
certificate.
Happens since: 1.x

@emasab emasab requested a review from a team as a code owner November 6, 2024 18:43
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@airlock-confluentinc airlock-confluentinc bot force-pushed the dev_ssl_send_full_certificate_chain branch from c77c7e4 to ec21967 Compare December 4, 2024 19:52
@emasab emasab changed the base branch from master to dev_kip848_mock_handler_and_integration_tests December 4, 2024 19:55
@airlock-confluentinc airlock-confluentinc bot force-pushed the dev_ssl_send_full_certificate_chain branch from ec21967 to 7ec52aa Compare December 4, 2024 20:03
@airlock-confluentinc airlock-confluentinc bot force-pushed the dev_ssl_send_full_certificate_chain branch from 6cf59f0 to d64f4f8 Compare December 5, 2024 17:28
Copy link
Contributor

@milindl milindl left a comment

Choose a reason for hiding this comment

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

Reviewed the non-test files, looking at tests.

if (sk_X509_num(ca) > 0)
cert->chain = ca;
else
sk_X509_pop_free(cert->chain,
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this need to be sk_X509_pop_free(ca, instead? because cert->chain isn't assigned to yet.

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