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

Add Config::add_root_certificate for trusting custom ca #239

Merged

Conversation

iamjpotts
Copy link
Contributor

Support connecting to registries with self-signed certificates and certificates signed by CAs that are not in the client system trust store.

Simply passes thru the certificate along to a reqwest builder method by the same name.

Adds two tests:

  1. Verify the client fails with an SSL error when the CA is not provided
  2. Verify the client connects without an SSL error when the CA is given

Both tests are a part of the test-mock feature.

Copy link
Collaborator

@PratikMahajan PratikMahajan left a comment

Choose a reason for hiding this comment

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

PR needs rebase

.gitignore Outdated
@@ -3,3 +3,4 @@ Cargo.lock
.envrc
default.nix
shell.nix
.idea
Copy link
Collaborator

Choose a reason for hiding this comment

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

this need not be in the commit. Depends on dev workflow, ide.
Can we worked around by using gobal .gitignore on dev system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@iamjpotts iamjpotts force-pushed the 20221107-certificate-authority branch 3 times, most recently from 38408c0 to 527690c Compare January 25, 2024 23:48
@iamjpotts
Copy link
Contributor Author

PR needs rebase

Surprised, what did you notice that made it look like it needed a rebase? It was rebased about 5 months ago.

It should be current with the default branch now in any case.

@iamjpotts
Copy link
Contributor Author

When this PR was originally authored 2 years ago, hyper 1.x had not been released.

Do you want me to upgrade this PR to hyper 1.x, or leave it on version 0.14?

@PratikMahajan
Copy link
Collaborator

can you upgrade to hyper 1.x and it'll be good to go!

@iamjpotts iamjpotts force-pushed the 20221107-certificate-authority branch from 527690c to b28ddc1 Compare January 31, 2024 01:40
@iamjpotts
Copy link
Contributor Author

can you upgrade to hyper 1.x and it'll be good to go!

Done.

Copy link
Collaborator

@PratikMahajan PratikMahajan left a comment

Choose a reason for hiding this comment

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

/lgtm

@iamjpotts
Copy link
Contributor Author

I assume you are ok with a MSRV increase with this PR?

Do you want it to be the oldest possible passing version, or a newer version?

@PratikMahajan
Copy link
Collaborator

looks like we need to move to v0.14.28
sorry about that

@iamjpotts iamjpotts force-pushed the 20221107-certificate-authority branch from b28ddc1 to 72789d2 Compare February 1, 2024 04:10
@iamjpotts iamjpotts force-pushed the 20221107-certificate-authority branch from 72789d2 to 5c7dedd Compare February 1, 2024 04:14
@iamjpotts
Copy link
Contributor Author

  • Back to hyper 0.14
  • Fixed cargo fmt issues

@PratikMahajan PratikMahajan merged commit f11746a into camallo:master Feb 1, 2024
6 of 16 checks passed
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