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

Rebased version of [WIP] Bundled cacert #4679 #5677

Closed
wants to merge 1 commit into from

Conversation

gvollant
Copy link
Contributor

I added on my fork https://github.com/gvollant/curl a rebased version of #4679
I invite @moparisthebest on my curl fork. He made the real work

I just wanted a rebased version of this PR to test it against current curl master

Validation : 397e11b [397e11b]
Parents : 4506607
Auteur : moparisthebest admin@moparisthebest.com
Date : vendredi 3 avril 2020 00:32:42
Auteur : moparisthebest
Date de validation : samedi 4 avril 2020 00:31:23
Add CURLOPT_CAINFO_PEM

@gvollant
Copy link
Contributor Author

this PR is probably exclusive with #5664

@nadrolinux
Copy link

Hello,
Any progress in this area? This is really nice feature.

Validation : 397e11b [397e11b]
Parents : 4506607
Auteur : moparisthebest <admin@moparisthebest.com>
Date : vendredi 3 avril 2020 00:32:42
Auteur : moparisthebest
Date de validation : samedi 4 avril 2020 00:31:23
Add CURLOPT_CAINFO_PEM
@gvollant
Copy link
Contributor Author

Hello,
Any progress in this area? This is really nice feature.

I just rebase it against current master

@bagder bagder mentioned this pull request Sep 20, 2020
@bagder
Copy link
Member

bagder commented Sep 20, 2020

Note that this still causes test failures (test 1119) which is one of the primary reasons I've not checked it closer yet.

return CURLE_SSL_CACERT_BADFILE;
}
/* Continue with a warning if no certificate verification is required. */
infof(data, "error setting certificate file, continuing anyway\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we really ignore the failure to load the cacert from memory when we are not verifying the peer certificate?

Would not it be better to fail anyway? Honest question, I don't have a specific opinion on this matter.

@bagder
Copy link
Member

bagder commented Oct 7, 2020

Closing due to inactivity.

@bagder bagder closed this Oct 7, 2020
@JCMais
Copy link
Contributor

JCMais commented Oct 9, 2020

The test failure seems to be related to the SYMBOLS_IN_VERSIONS documentation page.

@gvollant @moparisthebest are you guys available to take a look into this? This feature seems very interesting.

If not, I should be able to take a look at it later next week.

@gvollant
Copy link
Contributor Author

I rebased https://github.com/gvollant/curl/tree/gv_bundled_cacert but PR is closed...

@JCMais
Copy link
Contributor

JCMais commented Oct 13, 2020

@gvollant It was closed due to inactivity. It was not reviewed before because the tests were failing: https://github.com/curl/curl/runs/1098854111

@gvollant
Copy link
Contributor Author

The latest commit must fix test 1119 . How can we reopen ?

@JCMais
Copy link
Contributor

JCMais commented Oct 14, 2020

cc @bagder

@bagder
Copy link
Member

bagder commented Oct 14, 2020

You can't. If you're set to work on it, submit a new PR!

@JCMais
Copy link
Contributor

JCMais commented Oct 21, 2020

@gvollant let me know if have the time / is planning to open a new PR with the fixes. I'm eager for this feature, if no one is going to keep working on it I should be able to take ownership of the remaining changes needed.

@gvollant
Copy link
Contributor Author

@JCMais see #6109
I send you invitation on my fork

jay pushed a commit that referenced this pull request May 5, 2021
- New options CURLOPT_CAINFO_BLOB and CURLOPT_PROXY_CAINFO_BLOB to
  specify in-memory PEM certificates for OpenSSL, Schannel (Windows)
  and Secure Transport (Apple) SSL backends.

Prior to this change PEM certificates could only be imported from a file
and not from memory.

Co-authored-by: moparisthebest@users.noreply.github.com

Ref: #4679
Ref: #5677
Ref: #6109

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

Successfully merging this pull request may close these issues.

None yet

5 participants