Skip to content

Test client authentication, add https-mtls server #16923

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

Closed
wants to merge 3 commits into from

Conversation

yedayak
Copy link
Contributor

@yedayak yedayak commented Apr 2, 2025

In order to test using mutual TLS (mtls), this adds a new server type, https-mtls, which turns on the verifyChain in stunnel, which forces client to authenticate using a certificate signed by the CA specified by CAfile.
Also generate a client certificate to use when authenticating.

In addition, this removes the CApath config since it requires CA files being named with the hash of the subject [1], which isn't done here (if I understand correctly it didn't even point to the certs/ directory).

[1] https://www.stunnel.org/static/stunnel.html#CApath-CA_DIRECTORY

Refs #16804 #16686

@github-actions github-actions bot added the tests label Apr 2, 2025
@yedayak yedayak marked this pull request as draft April 2, 2025 14:11
@testclutch
Copy link

Analysis of PR #16923 at de80d163:

Test 417 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 65 different CI jobs (the link just goes to one of them).

Test 2088 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 83 different CI jobs (the link just goes to one of them).

Test 1194 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 2202 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

@yedayak
Copy link
Contributor Author

yedayak commented Apr 3, 2025

It seems test 417 fails because when stunnel has the CAfile, it sends it in the chain to the client, which is not required. This means test 417 gets two certificates which the test doesn't expect.
I'll move the CAfile definition to the mtls branch, unless someone thinks 417 should be changed for this.

@yedayak yedayak force-pushed the test-client-auth branch 3 times, most recently from baf858d to f9f243e Compare April 3, 2025 19:41
yedayak added a commit to yedayak/curl that referenced this pull request Apr 3, 2025
SIZE_MAX is an very overkill size for certificates or keys, lower it to
100KiB for both certificate and keys. The default max size of openssl is
100KiB for the entire chain [1], and it seems firefox fails at ~60kb
[2].

Found by curl#16923

[0] https://docs.openssl.org/3.2/man3/SSL_CTX_set_max_cert_list/#notes
[2] https://0x00.cl/blog/2024/exploring-tls-certs/
@yedayak
Copy link
Contributor Author

yedayak commented Apr 3, 2025

Well this seems to have already found a failure in rustls, opened #16951

yedayak added a commit to yedayak/curl that referenced this pull request Apr 3, 2025
SIZE_MAX is an very overkill size for certificates or keys, lower it to
100KiB for both certificate and keys. The default max size of openssl is
100KiB for the entire chain [1], and it seems firefox fails at ~60kb
[2].

Found by curl#16923

[0] https://docs.openssl.org/3.2/man3/SSL_CTX_set_max_cert_list/#notes
[2] https://0x00.cl/blog/2024/exploring-tls-certs/
yedayak added a commit to yedayak/curl that referenced this pull request Apr 3, 2025
SIZE_MAX is an very overkill size for certificates or keys, lower it to
100KiB for both certificate and keys. The default max size of openssl is
100KiB for the entire chain [1], and it seems firefox fails at ~60kb
[2].

Found by curl#16923

[0] https://docs.openssl.org/3.2/man3/SSL_CTX_set_max_cert_list/#notes
[2] https://0x00.cl/blog/2024/exploring-tls-certs/
bagder pushed a commit that referenced this pull request Apr 3, 2025
SIZE_MAX is an very overkill size for certificates or keys, lower it to
100KiB for both certificate and keys. The default max size of openssl is
100KiB for the entire chain [1], and it seems firefox fails at ~60kb
[2].

Found by #16923

[0] https://docs.openssl.org/3.2/man3/SSL_CTX_set_max_cert_list/#notes
[2] https://0x00.cl/blog/2024/exploring-tls-certs/

Closes #16951
@yedayak yedayak force-pushed the test-client-auth branch from f9f243e to 5bac069 Compare April 3, 2025 21:37
@yedayak yedayak marked this pull request as ready for review April 3, 2025 22:13
yedayak added 3 commits April 4, 2025 13:25
It wasn't used, and didn't do anything since the folder it got didn't
have files with names of the hash of the subjects. [1]

[1] https://www.stunnel.org/static/stunnel.html#CApath-CA_DIRECTORY
This adds a new certificate to generate which has the clientAuth key
usage enabled, and uses it to connect to a https-mtls server.
@yedayak yedayak force-pushed the test-client-auth branch from 5bac069 to 880322b Compare April 4, 2025 10:25
@bagder bagder closed this in 0f201d4 Apr 7, 2025
bagder pushed a commit that referenced this pull request Apr 7, 2025
- test2088 verifies that mutual tls works

This adds a new certificate to generate which has the clientAuth key
usage enabled, and uses it to connect to a https-mtls server.

Closes #16923
@bagder
Copy link
Member

bagder commented Apr 7, 2025

Thanks!

nbaws pushed a commit to nbaws/curl that referenced this pull request Apr 26, 2025
It wasn't used, and didn't do anything since the folder it got didn't
have files with names of the hash of the subjects. [1]

[1] https://www.stunnel.org/static/stunnel.html#CApath-CA_DIRECTORY

Closes curl#16923
nbaws pushed a commit to nbaws/curl that referenced this pull request Apr 26, 2025
- test2088 verifies that mutual tls works

This adds a new certificate to generate which has the clientAuth key
usage enabled, and uses it to connect to a https-mtls server.

Closes curl#16923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants