Skip to content

Ensure all tests pass with rustls backend - #14317

Closed
ctz wants to merge 6 commits into
curl:masterfrom
ctz:jbp-rustls-test-updates
Closed

Ensure all tests pass with rustls backend#14317
ctz wants to merge 6 commits into
curl:masterfrom
ctz:jbp-rustls-test-updates

Conversation

@ctz

@ctz ctz commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

The goal of this PR is to remove the rustls stanza in tests/data/DISABLED that skips some tests.

That involves supporting CRLs -- we've had upstream support for a little while, but needs some plumbing in here.

This is my first curl PR, so please shout if I've made any beginner errors: but I have run the tests and run make checksrc.

ctz added 5 commits July 30, 2024 14:23
@ctz
rustls supports IP addresses in certs since 0.21
4c06a71 
Remove workaround, and re-enable tests.
@ctz
rustls: expose support for CRLs
b987db1 
This enables support for `CURLOPT_CRLFILE` in the rustls
TLS backend.
@ctz
rustls: fix leak of rustls_root_cert_store
85f0131 
`rustls_web_pki_server_cert_verifier_builder_new` does not
take ownership of its argument.
@ctz
rustls: fix leak of rustls_server_cert_verifier
4f30a61 
`rustls_client_config_builder_set_server_verifier` has
refcount semantics on its argument.
@ctz
rustls: re-enable CRL test
1938e4a
@github-actions github-actions Bot added the tests label Jul 30, 2024
cpu
cpu reviewed Jul 30, 2024
View reviewed changes

@cpu cpu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! The diff looks good to me, but my experience in this repo is about the same as yours :-)

bagder
bagder approved these changes Jul 30, 2024
View reviewed changes

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome!

@bagder

bagder commented Jul 30, 2024

Copy link
Copy Markdown
Member

Our rustls CI job uses rustls-version: 0.13.0. That looks really outdated, right?

@cpu

cpu commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

Our rustls CI job uses rustls-version: 0.13.0. That looks really outdated, right?

That's the rustls-ffi version (the C bindings for rustls) and is current.

@ctz

ctz commented Jul 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Our rustls CI job uses rustls-version: 0.13.0. That looks really outdated, right?

That is the current version, which underneath uses rustls 0.23.4 (released ~March 2024). So it's pretty current. ^snap

@cpu

cpu commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

Our rustls CI job uses rustls-version: 0.13.0. That looks really outdated, right?

That is the current version, which underneath uses rustls 0.23.4 (released ~March 2024). So it's pretty current. ^snap

Sorry, thought I was safe replying based on time zones but we raced! 😃

@bagder

bagder commented Jul 30, 2024

Copy link
Copy Markdown
Member

Ah, that makes sense. Thanks!

@bagder bagder closed this in dd95a49 Jul 31, 2024
@bagder

bagder commented Jul 31, 2024

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bagder bagder bagder approved these changes
@MarcelRaad MarcelRaad MarcelRaad approved these changes
+1 more reviewer
@cpu cpu cpu left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tests

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ctz @bagder @cpu @MarcelRaad