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
schannel: offer a "best effort" revocation check mode #4981
Conversation
c029416
to
549dc5f
Compare
(I'll just note that this is too late for getting into 7.69.0) |
Okay :-) I'll update the version numbers once we have a working regression test... |
We have a solution to this already, --ssl-no-revoke. I'm against soft revocation, I think it's misleading. That said I suggest calling this --ssl-soft-revoke instead of best effort. Best effort could be interpreted as the best way to revoke. "soft" is clearly less. Or split it up --ssl-ignore-revoke-missing --ssl-ignore-revoke-offline. Or how about George did something like that in #4078... would --ssl-ignore-revoke-missing not be enough? (Note a revoke missing error technically means revocation could not be performed, not necessarily because there is no revocation point.) |
Unfortunately, this is not a solution at all... I would not respond so forcefully to your claim if I had not had to field my share of support requests. With This is not really acceptable for a vast majority of our users. And if I had to field my share of support requests, the GitHub Desktop team had to field a dozen time as many. To make things utterly clear: the situation is so bad with the unfortunate lack of a best effort strategy in cURL that, should you dig in and defend your stance despite all of the user complaints to the contrary, you will force me to ship a modified cURL in Git for Windows. I don't want to fork from cURL. I really don't. I hope you don't make me.
It has nothing to do with soft, please do not misrepresent what this does. It is a best effort. Don't take my word for it, take the word of another Microsoftie (if you don't trust me): #264 (comment). In other words, this is not a soft revocation (what would that be, anyway? "soft" would suggest that you say "well, this is revoked, but this one time we let it pass anyway"?). This is a best effort. And it is the common practice of web browsers out there. Above-quoted Eric Lawrence is a Microsoft Edge developer. You might want to assume that he really knows what he is talking about.
I can do that. It will make it a bit harder to follow the lead of web browsers to implement a best effort strategy, but if that's what it takes to get this use case supported, so be it. More importantly, could you guide me how to implement a test case for this? |
I don't understand why doing it as two separate options would make anything better. I think @dscho makes a compelling argument for this feature and I don't see how adding two options for this is what anyone wants nor how it helps anyone - but instead just makes it more complicated. Revocations is a difficult and unfortunately mostly broken area of TLS and certificates. I don't think we do our users any benefits by refusing this PR. |
549dc5f
to
cdcc286
Compare
@niik pointed out that I forgot the case where the certificates specified via Speaking of tests, @niik also came up with a better test (i.e. one that does not try to access a proxy with a public IP) by copy-editing 310 to our need. I verified locally (with a newly-created ,MSYS2-packaged |
cdcc286
to
7d85e6a
Compare
I changed the patch tentatively to refer to 7.70.0 instead. Of course, I understand that 7.69.0 is due March 4th, so I hope that this PR will be rebase-merged soon thereafter :-) |
58153e2
to
1c75019
Compare
This is soft revocation. The revocation check for whatever reason can't be performed and so it fails. You want to soft-fail it. Hence soft revocation. You are making assumptions about what has caused the two errors you want to ignore. If an attacker can make the client ignore the revocation check it is in my opinion not worth checking. I have discussed this already in the issues. I feel I've treated everyone fairly on this. |
I feel reminded of the "WinSSL" vs "Secure Channel" issue: insisting on calling it something different than literally everybody else is just confusing. I'd rather refrain from confusing users. Same reason I'd like this to be one cURL flag instead of two, with a name that talks about the effect, rather than using names that must be less than obvious to occasional users. |
@@ -351,6 +354,8 @@ static CURLcode dohprobe(struct Curl_easy *data, | |||
} | |||
if(data->set.ssl.no_revoke) | |||
ERROR_CHECK_SETOPT(CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); | |||
if(data->set.ssl.revoke_best_effort) | |||
ERROR_CHECK_SETOPT(CURLOPT_SSL_OPTIONS, CURLSSLOPT_REVOKE_BEST_EFFORT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the proxy one in doh will need to be changed to else if
because otherwise best effort overrides no revoke, think of it like this
curl_easy_setopt(curl, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);
curl_easy_setopt(curl, CURLOPT_SSL_OPTIONS, CURLSSLOPT_REVOKE_BEST_EFFORT);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course!
40e8fdf
to
a2b0730
Compare
With this change, we rebase the two patches we have on top of cURL to the newest cURL version, and we also add the (still tentative) support for the "best effort" strategy for dealing with revoke problems in the Secure Channel backend. This "best effort" strategy is currently under review at curl/curl#4981. We will use this feature in Git for Windows, to turn on "best-effort" by default (using `CURLSSLOPT_REVOKE_BEST_EFFORT` as a tell-tale whether cURL supports it). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Hrm. I tried to detect Msys so that we can avoid testing 2070 there (it would always fail with "curl: (4) schannel: CA cert support not built in", there's nothing we can do about it), but apparently this precheck does not catch it: curl -V | grep -q pc-mingw32 && echo "MINGW has no CA cert support" Any ideas? Also, the FreeBSD CI is failing due to some sort of relatively obvious version check mismatch. What is not obvious to me is what I can do about it. Lastly, there seem to be a ton of CI failures on Windows that however appear not to be caused by this PR. Any guidance? |
This should be fixed via 691b71b in master now.
These should be fixed via 9aaca09. Please rebase. Also AppVeyor may still randomly fail some tests for currently unknown reason, then just retry by re-pushing or asking one of us to re-run the CI. |
Maybe do the check like the testsuite itself does? Line 2721 in 3c1b914
|
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The native Windows HTTPS backend is based on Secure Channel which lets the caller decide how to handle revocation checking problems caused by missing information in the certificate or offline CRL distribution points. Unfortunately, cURL chose to handle these problems differently than OpenSSL by default: while OpenSSL happily ignores those problems (essentially saying "¯\_(ツ)_/¯"), the Secure Channel backend will error out instead. As a remedy, the "no revoke" mode was introduced, which turns off revocation checking altogether. This is a bit heavy-handed. We support this via the `http.schannelCheckRevoke` setting. In curl/curl#4981, we contributed an opt-in "best effort" strategy that emulates what OpenSSL seems to do. In Git for Windows, we actually want this to be the default. This patch makes it so, introducing it as a new value for the `http.schannelCheckRevoke" setting, which now becmes a tristate: it accepts the values "false", "true" or "best-effort" (defaulting to the last one). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Network is locked so i cant change anything on it plus dms are also flakey. How to preceed? |
@thniig you're commenting on a PR that's been closed for months. It won't help you even if we would understand what you're talking about, which I don't. If you have a (lib)curl bug, file an issue. If you have a question, post it on the suitable curl mailing list. |
When running e.g. with Fiddler, the schannel backend fails with an unhelpful error message:
Unfortunately, it affects not only debugging tools like Fiddler, but a large number of enterprise setups where MITM proxies are the rule rather than the exception. So it's not like this is a niche problem.
This has been discussed in plenty of issues, e.g. in #3727 or in #264.
It has also been discussed plenty of times in the context of Git for Windows and of GitHub Desktop, as many of our users ran straight into these issues with proxies and/or certificates lacking distribution points for CRLs.
Hopefully we will get this (as far as cURL is concerned, still opt-in) behavior, at long last?
/cc @georgeok @jay @niik
@jay, I basically copy-edited the test case straight from georgeok@bd69551 and am fairly certain that this is not what we want. If I am reading it correctly, this test requires an internet connection and a not-further-specified proxy behind a public IP. I would love to fix this, but I don't know how. Essentially, what I want is a test that verifies that schannel works even when using a proxy with a certificate that is missing a CRL distribution point. Would you kindly help me out here?