Honor R_LIBCURL_SSL_REVOKE_BEST_EFFORT #1624
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
R_LIBCURL_SSL_REVOKE_BEST_EFFORT was introduced in R on R-4.2 as documented in wch/r-source@f1ec503. If the environment variable is set to TRUE, then curl relaxes the certificate revocation checks. This is needed on Windows systems using curl with the schannel backend on environments with man in the middle https certificates (typically corporate environments), since those certificate revocation checks otherwise fail. To avoid duplicating custom configurations for all packages, I suggest to honor that environment variable here as well. I tested the command: curl.exe -X GET https://cloud.r-project.org/src/contrib/PACKAGES on a system where it failed for the reasons described above. I tested the proposed solution and it worked. curl.exe -X GET --ssl-revoke-best-effort https://cloud.r-project.org/src/contrib/PACKAGES I also considered that a user may have set the '--ssl-revoke-best-effort' parameter manually, and with my patch the user would be passing the parameter twice. I tested passing that option twice and curl did not care, so no backwards issue is expected. In other words, this worked: curl.exe -X GET --ssl-revoke-best-effort --ssl-revoke-best-effort https://cloud.r-project.org/src/contrib/PACKAGES
kevinushey
approved these changes
Aug 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
R_LIBCURL_SSL_REVOKE_BEST_EFFORTwas introduced in R on R-4.2 at wch/r-source@f1ec503.If that environment variable is set to
TRUE, then curl relaxes the certificate revocation checks. This is needed on Windows systems using curl with the schannel backend on environments with man in the middle https certificates (typically corporate environments), since those certificate revocation checks otherwise fail.The
curlpackage already disables those checks on windows as discussed at jeroen/curl#306.To avoid having to declare a custom configuration for each package, I suggest to honor that same environment variable in
renvas well.I manually verified (on a Windows system with a MITM certificate authority) that renv was giving certificate revocation check errors and that with this fix the errors are gone. Besides, I did the following command line curl tests on such system:
The command:
failed for the reasons described above (certificate revocation check failed).
The proposed solution:
worked without any issue.
I also considered that a user may have set the
--ssl-revoke-best-effortparameter manually for renv, and with this patch the user would be passing the parameter twice. Therefore I tested passing that option to curl twice. curl did not care, so no backwards compatibility issue is expected. In other words, this worked:Thanks for your consideration reviewing this PR and for your time working on renv. It is really useful.