Commit Graph

29 Commits

Author SHA1 Message Date
Johannes Schindelin 5450428491 schannel: add "best effort" revocation check option
- Implement new option CURLSSLOPT_REVOKE_BEST_EFFORT and
  --ssl-revoke-best-effort to allow a "best effort" revocation check.

A best effort revocation check ignores errors that the revocation check
was unable to take place. The reasoning is described in detail below and
discussed further in the PR.

---

When running e.g. with Fiddler, the schannel backend fails with an
unhelpful error message:

	Unknown error (0x80092012) - The revocation function was unable
	to check revocation for the certificate.

Sadly, many enterprise users who are stuck behind MITM proxies suffer
the very same problem.

This has been discussed in plenty of issues:
https://github.com/curl/curl/issues/3727,
https://github.com/curl/curl/issues/264, for example.

In the latter, a Microsoft Edge developer even made the case that the
common behavior is to ignore issues when a certificate has no recorded
distribution point for revocation lists, or when the server is offline.
This is also known as "best effort" strategy and addresses the Fiddler
issue.

Unfortunately, this strategy was not chosen as the default for schannel
(and is therefore a backend-specific behavior: OpenSSL seems to happily
ignore the offline servers and missing distribution points).

To maintain backward-compatibility, we therefore add a new flag
(`CURLSSLOPT_REVOKE_BEST_EFFORT`) and a new option
(`--ssl-revoke-best-effort`) to select the new behavior.

Due to the many related issues Git for Windows and GitHub Desktop, the
plan is to make this behavior the default in these software packages.

The test 2070 was added to verify this behavior, adapted from 310.

Based-on-work-by: georgeok <giorgos.n.oikonomou@gmail.com>
Co-authored-by: Markus Olsson <j.markus.olsson@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Closes https://github.com/curl/curl/pull/4981
2020-03-18 03:23:39 -04:00
Pedro Monreal 4b6fd29f1a cleanup: fix typos and wording in docs and comments
Closes #4869
Reviewed-by: Emil Engler and Daniel Gustafsson
2020-02-02 18:43:01 +01:00
Daniel Stenberg 4940bb8568
doh: make it behave when built without proxy support
Reported-by: Marcel Raad
Bug: https://github.com/curl/curl/pull/4692#issuecomment-564115734

Closes #4704
2019-12-12 09:26:08 +01:00
Jay Satiro 9c1806ae46 build: Disable Visual Studio warning "conditional expression is constant"
- Disable warning C4127 "conditional expression is constant" globally
  in curl_setup.h for when building with Microsoft's compiler.

This mainly affects building with the Visual Studio project files found
in the projects dir.

Prior to this change the cmake and winbuild build systems already
disabled 4127 globally for when building with Microsoft's compiler.
Also, 4127 was already disabled for all build systems in the limited
circumstance of the WHILE_FALSE macro which disabled the warning
specifically for while(0). This commit removes the WHILE_FALSE macro and
all other cruft in favor of disabling globally in curl_setup.

Background:

We have various macros that cause 0 or 1 to be evaluated, which would
cause warning C4127 in Visual Studio. For example this causes it:

    #define Curl_resolver_asynch() 1

Full behavior is not clearly defined and inconsistent across versions.
However it is documented that since VS 2015 Update 3 Microsoft has
addressed this somewhat but not entirely, not warning on while(true) for
example.

Prior to this change some C4127 warnings occurred when I built with
Visual Studio using the generated projects in the projects dir.

Closes https://github.com/curl/curl/pull/4658
2019-12-01 19:01:02 -05:00
Niall 8acfad38ca
doh: use dedicated probe slots
... to easier allow additional DNS transactions.

Closes #4629
2019-11-28 12:49:44 +01:00
Niall b6a53fff6c
doh: improced both encoding and decoding
Improved estimation of expected_len and updated related comments;
increased strictness of QNAME-encoding, adding error detection for empty
labels and names longer than the overall limit; avoided treating DNAME
as unexpected;

updated unit test 1655 with more thorough set of proofs and tests

Closes #4598
2019-11-16 16:15:03 +01:00
Daniel Stenberg dcd7e37c3a
url: make Curl_close() NULLify the pointer too
This is the common pattern used in the code and by a unified approach we
avoid mistakes.

Closes #4534
2019-10-30 09:36:21 +01:00
Paul Dreik a5bf6a36c5
doh: allow only http and https in debug mode
Otherwise curl may be told to use for instance pop3 to
communicate with the doh server, which most likely
is not what you want.

Found through fuzzing.

Closes #4406
2019-09-23 22:38:23 +02:00
Paul Dreik bb74201804
doh: return early if there is no time left
Closes #4406
2019-09-23 22:38:15 +02:00
Niall O'Reilly 0d59addff6
doh: avoid truncating DNS QTYPE to lower octet
Closes #4381
2019-09-19 22:50:38 +02:00
Daniel Stenberg 9bc44ff64d
doh: clean up dangling DOH handles and memory on easy close
If you set the same URL for target as for DoH (and it isn't a DoH
server), like "https://example.com" in both, the easy handles used for
the DoH requests could be left "dangling" and end up not getting freed.

Reported-by: Paul Dreik
Closes #4366
2019-09-16 17:31:56 +02:00
Paul Dreik dda418266c
doh: fix undefined behaviour and open up for gcc and clang optimization
The undefined behaviour is annoying when running fuzzing with
sanitizers. The codegen is the same, but the meaning is now not up for
dispute. See https://cppinsights.io/s/516a2ff4

By incrementing the pointer first, both gcc and clang recognize this as
a bswap and optimizes it to a single instruction.  See
https://godbolt.org/z/994Zpx

Closes #4350
2019-09-15 23:27:45 +02:00
Paul Dreik b766602729
doh: fix (harmless) buffer overrun
Added unit test case 1655 to verify.
Close #4352

the code correctly finds the flaws in the old code,
if one temporarily restores doh.c to the old version.
2019-09-15 23:25:24 +02:00
Marcel Raad 10db3ef21e
lib: reduce variable scopes
Fixes Codacy/CppCheck warnings.

Closes https://github.com/curl/curl/pull/3872
2019-05-20 08:51:11 +02:00
Daniel Stenberg ae4adae954
doh: CURL_DISABLE_DOH 2019-05-13 08:17:09 +02:00
Jay Satiro 9e6af114ea doh: inherit some SSL options from user's easy handle
- Inherit SSL options for the doh handle but not SSL client certs,
  SSL ALPN/NPN, SSL engine, SSL version, SSL issuer cert,
  SSL pinned public key, SSL ciphers, SSL id cache setting,
  SSL kerberos or SSL gss-api settings.

- Fix inheritance of verbose setting.

- Inherit NOSIGNAL.

There is no way for the user to set options for the doh (DNS-over-HTTPS)
handles and instead we inherit some options from the user's easy handle.

My thinking for the SSL options not inherited is they are most likely
not intended by the user for the DOH transfer. I did inherit insecure
because I think that should still be in control of the user.

Prior to this change doh did not work for me because CAINFO was not
inherited. Also verbose was set always which AFAICT was a bug (#3660).

Fixes https://github.com/curl/curl/issues/3660
Closes https://github.com/curl/curl/pull/3661
2019-03-11 01:50:12 -04:00
Rikard Falkeborn fa2d6ba84d
printf: fix format specifiers
Closes #3426
2019-01-04 23:50:48 +01:00
Daniel Stenberg d506df860f
doh: fix memory leak in OOM situation
Reviewed-by: Daniel Gustafsson
Closes #3342
2018-12-06 09:52:38 +01:00
Daniel Stenberg 027d66e5f1
doh: make it work for h2-disabled builds too
Reported-by: dtmsecurity at github
Fixes #3325
Closes #3336
2018-12-05 23:44:26 +01:00
Daniel Gustafsson fd1049554f doh: fix typo in infof call
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
2018-11-26 11:40:26 +01:00
Daniel Stenberg dcd6f81025
snprintf: renamed and we now only use msnprintf()
The function does not return the same value as snprintf() normally does,
so readers may be mislead into thinking the code works differently than
it actually does. A different function name makes this easier to detect.

Reported-by: Tomas Hoger
Assisted-by: Daniel Gustafsson
Fixes #3296
Closes #3297
2018-11-23 08:26:51 +01:00
Daniel Stenberg ad547fcf7b
travis: add build for "configure --disable-verbose"
Closes #3144
2018-10-18 14:51:49 +02:00
Daniel Stenberg 7f00146d00
doh: keep the IPv4 address in (original) network byte order
Ideally this will fix the reversed order shown in SPARC tests:

  resp 8: Expected 127.0.0.1 got 1.0.0.127

Closes #3091
2018-10-05 22:15:34 +02:00
Daniel Stenberg 8f2bb0e377
doh: make sure TTL isn't re-inited by second (discarded?) response
Closes #3092
2018-10-04 23:22:28 +02:00
Marcel Raad c1c092c0b4
doh: fix curl_easy_setopt argument type
CURLOPT_POSTFIELDSIZE is long. Fixes a compiler warning on 64-bit
MinGW.
2018-10-02 11:15:29 +02:00
Daniel Stenberg 570008c99d
doh: only build if h2 enabled
The DoH spec says "HTTP/2 [RFC7540] is the minimum RECOMMENDED version
of HTTP for use with DoH".

Reported-by: Marcel Raad
Closes #3066
2018-09-30 11:31:58 +02:00
Daniel Stenberg c515294cec
doh: minor edits to please Coverity
The gcc typecheck macros and coverity combined made it warn on the 2nd
argument for ERROR_CHECK_SETOPT(). Here's minor rearrange to please it.

Coverity CID 1439115 and CID 1439114.
2018-09-07 09:26:08 +02:00
Daniel Stenberg f2b1a18975
DOH: add test case 1650 and 2100 2018-09-06 09:17:25 +02:00
Daniel Stenberg abff183387
setopt: add CURLOPT_DOH_URL
Closes #2668
2018-09-06 09:17:17 +02:00