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

Added functionality to enable CRL checking for CURL on linux; added tests for this new functionality. #3923

Conversation

LarryOsterman
Copy link
Member

@LarryOsterman LarryOsterman commented Sep 2, 2022

Added functionality to enable CRL checking for CURL on linux.

This one is somewhat unpleasant and much larger than expected.

This pull request enables two pieces of functionality:

  1. The ability to specify a known root certificate to the CURL HTTP transport (instead of a certificate file).
  2. The ability to enable CRL validation (normally this is disabled in libCURL).

Enabling CRL validation ended up pulling in a significant chunk of code from azure-c-shared-util which handled retrieving CRLs (I was unable to find code in libCURL to do this). Native LibCURL support for CRL validation is limited to the schannel SSL backend (Windows Only).

This change also adds logic to the CURL transport to enable the ability to ignore CRL retrieval errors (there doesn't seem to be a comparable way of doing this for WinHTTP so it is a CURL transport only option).

To verify the root certificate logic, an extremely simple client for the SDK Test Proxy was written and is used to "record" a request to the C++ SDK HTTP server.

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

  • C++ Guidelines
  • Doxygen docs
  • Unit tests
  • No unwanted commits/changes
  • Descriptive title/description
    • PR is single purpose
    • Related issue listed
  • Comments in source
  • No typos
  • Update changelog
  • Not work-in-progress
  • External references or docs updated
  • Self review of PR done
  • Any breaking changes?

@LarryOsterman LarryOsterman marked this pull request as ready for review September 7, 2022 00:16
@LarryOsterman
Copy link
Member Author

Ping :).

Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

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

For a long time, I was worried about this moment. The moment when our libcurl implementation would require orchestrating things beyond libcurl (like openssl, windows api, etc).
The long night has arrived!!! hahaha So glad we have Larry for this :)

sdk/core/azure-core/src/http/curl/curl.cpp Show resolved Hide resolved
@LarryOsterman LarryOsterman merged commit ceca1cf into Azure:feature/websockets Sep 19, 2022
@LarryOsterman LarryOsterman deleted the larryo/tlsrootcertificateoverride branch September 19, 2022 18:04
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.

4 participants