[PATCH v4] http: add http.sslVerifyStatus to check stapled OCSP responses

[graysongordon-gl <graysongordon1@gmail.com>]

From: Grayson Gordon <graysongordon1@gmail.com>

git asks libcurl to verify the peer certificate and the hostname, but it
never sets CURLOPT_SSL_VERIFYSTATUS, so the "Certificate Status Request"
TLS extension is never requested and any stapled OCSP response the server
does send is ignored.

On an OpenSSL-linked build this is silent. OpenSSL hands the stapled
response to the application and takes no view on it:
SSL_CTX_set_tlsext_status_cb(3) says the callback "should determine
whether the returned OCSP response(s) are acceptable or not", and libcurl
only installs that callback when CURLOPT_SSL_VERIFYSTATUS is set. So git
will fetch from a server whose own staple says its certificate has been
revoked.

A GnuTLS-linked build behaves differently, and the difference does not
come from curl. GnuTLS consults a stapled response inside
gnutls_certificate_verify_peers(), so the failure surfaces through the
verifypeer branch of curl's GnuTLS backend (lib/vtls/gtls.c) whether or
not CURLOPT_SSL_VERIFYSTATUS was ever set. The same git, against the same
server, therefore enforces revocation or not depending only on how its
libcurl was built. That difference is documented here rather than papered
over: this option turns the check on where the backend needs asking, and
setting it to false does not turn the check off on GnuTLS.

Add an http.sslVerifyStatus boolean that sets CURLOPT_SSL_VERIFYSTATUS.
Because http_options() is the collect_fn of a urlmatch config, the
per-URL form works with no further changes:

    git config http.https://example.com/.sslVerifyStatus true

It defaults to false, and has to. The option is fail-closed: libcurl fails
verification when the server staples nothing at all, so turning this on
globally would break every remote that does not staple.

Leaving the default to libcurl is not an option either. The same
complaint was raised there in https://github.com/curl/curl/issues/15483
and closed as intentional ("Marked as enhancement since this was done on
purpose"), with the observation that stapling is expected to see less use
as Let's Encrypt drops OCSP support. If the check is to be reachable at
all, the lever has to come from the application.

If the TLS backend cannot check the staple, curl_easy_setopt() returns
CURLE_NOT_BUILT_IN. Fail loudly there rather than carrying on, since
silently not checking is precisely what this option exists to prevent.

CURLOPT_SSL_VERIFYSTATUS has been available since libcurl 7.41.0, well
below the 7.61.0 floor documented in INSTALL, so no version guard is
needed.

The tests go in t5551 and run only in its https pass, which t5559 provides
by sourcing t5551 with LIB_HTTPD_SSL set; that is the only https server
the suite has. They exercise the fail-closed path, which needs no CA and
no OCSP responder: lib-httpd's server staples nothing, so enabling the
option has to turn a working fetch into a failing one. Verified against an
unpatched build, where exactly the two assertions that depend on the new
option fail and the two controls still pass, and against OpenSSL, GnuTLS
and mbedTLS-linked builds of libcurl.

Signed-off-by: Grayson Gordon <graysongordon1@gmail.com>
---

v4: drop the new test script.  The four tests now live in t5551, keyed on
$HTTPD_PROTO so they run in the https pass (t5559) only.

To answer the question I skipped past in v3: no, we do not need a new
script.  t5559 is t5551 run with LIB_HTTPD_SSL set, and it is the only
https server the suite has, so a new script would have had to stand up a
second one to reach the same place.  Nothing is left over from t5567 or
t5568 and no test number is spent.

That also means the t/meson.build hunk is not squashed in.  t5551 is
already listed there.  Adding t5568 to the list now would break configure
the other way round, since the list is checked against ls in both
directions and errors with "Test files configured, but not found".

On the lost exit status: changed, but to test_might_fail rather than a
bare &&.  The ls-remote in the prerequisite is expected to fail, that is
the premise of the test, so && short-circuits on the expected failure and
leaves the prerequisite unsatisfied.  Run against the https server both
ways:

    bare &&              ok 51 # skip http.sslVerifyStatus=true fails
                                 without a staple (missing SSL_VERIFYSTATUS)
    test_might_fail      ok 51 - http.sslVerifyStatus=true fails
                                 without a staple

The first still reports "passed all 61 test(s)", which is the failure mode
the prerequisite was written to avoid.  test_might_fail keeps the chain
intact and says the status is ignored on purpose.

Also dropped the "ls-remote succeeds with http.sslVerifyStatus unset"
test.  It was a control for the standalone script, and in t5551 the
surrounding tests already exercise that URL throughout.

The rationale that sat in an in-code comment in v3 is in the log now, per
the earlier review.

Verified: t5551 over plain http and t5559 over https both pass all 61
tests, with the four new ones skipping on the former and running on the
latter.
 Documentation/config/http.adoc | 17 +++++++++++++++++
 http.c                         | 10 ++++++++++
 t/t5551-http-fetch-smart.sh    | 29 +++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc
index 792a71b413..40b849bf7f 100644
--- a/Documentation/config/http.adoc
+++ b/Documentation/config/http.adoc
@@ -196,6 +196,23 @@ http.sslVerify::
 	over HTTPS. Defaults to true. Can be overridden by the
 	`GIT_SSL_NO_VERIFY` environment variable.
 
+http.sslVerifyStatus::
+	Whether to check the revocation status of the server
+	certificate using the stapled OCSP response supplied during
+	the TLS handshake ("OCSP stapling"). Defaults to false.
++
+This is fail-closed: if the server staples no response, verification
+fails. Set it per remote, e.g.
+`http.https://example.com/.sslVerifyStatus`, rather than globally.
++
+What it changes depends on the TLS backend libcurl was built against.
+An OpenSSL-linked build ignores a stapled response unless this is set.
+A GnuTLS-linked build consults the staple during ordinary certificate
+verification, so it already rejects a revoked certificate under
+`http.sslVerify` alone, and setting this to `false` does not disable
+that. Where a backend cannot check the staple at all, git fails with an
+error rather than continuing unchecked.
+
 http.sslCert::
 	File containing the SSL certificate when fetching or pushing
 	over HTTPS. Can be overridden by the `GIT_SSL_CERT` environment
diff --git a/http.c b/http.c
index caccf2108e..94f8dd817a 100644
--- a/http.c
+++ b/http.c
@@ -44,6 +44,7 @@ static CURL *curl_default;
 char curl_errorstr[CURL_ERROR_SIZE];
 
 static int curl_ssl_verify = -1;
+static int curl_ssl_verify_status;
 static int curl_ssl_try;
 static char *curl_http_version;
 static char *ssl_cert;
@@ -400,6 +401,10 @@ static int http_options(const char *var, const char *value,
 		curl_ssl_verify = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp("http.sslverifystatus", var)) {
+		curl_ssl_verify_status = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp("http.sslcipherlist", var))
 		return git_config_string(&ssl_cipherlist, var, value);
 	if (!strcmp("http.sslversion", var))
@@ -1133,6 +1138,11 @@ static CURL *get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2L);
 	}
 
+	if (curl_ssl_verify_status &&
+	    curl_easy_setopt(result, CURLOPT_SSL_VERIFYSTATUS, 1L) != CURLE_OK)
+		die(_("http.sslVerifyStatus is set, but the TLS backend of "
+		      "this libcurl cannot verify certificate status"));
+
     if (curl_http_version) {
 		long opt;
 		if (!get_curl_http_version_opt(curl_http_version, &opt)) {
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 805bec025c..c11e96c1ac 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -680,6 +680,35 @@ test_expect_success 'passing hostname resolution information works' '
 	git -c "http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1" ls-remote "$BOGUS_HTTPD_URL/smart/repo.git" >/dev/null
 '
 
+test_lazy_prereq SSL_VERIFYSTATUS '
+	test "$HTTPD_PROTO" = "https" &&
+	test_might_fail git -c http.sslVerifyStatus=true \
+		ls-remote "$HTTPD_URL/smart/repo.git" 2>err &&
+	! grep "cannot verify certificate status" err
+'
+
+test_expect_success SSL_VERIFYSTATUS 'http.sslVerifyStatus=true fails without a staple' '
+	test_must_fail git -c http.sslVerifyStatus=true \
+		ls-remote "$HTTPD_URL/smart/repo.git"
+'
+
+test_expect_success SSL_VERIFYSTATUS 'http.sslVerifyStatus=false is a no-op' '
+	git -c http.sslVerifyStatus=false \
+		ls-remote "$HTTPD_URL/smart/repo.git" >actual &&
+	test_line_count -gt 0 actual
+'
+
+test_expect_success SSL_VERIFYSTATUS 'per-URL sslVerifyStatus applies to a matching URL' '
+	test_must_fail git -c "http.$HTTPD_URL/.sslVerifyStatus=true" \
+		ls-remote "$HTTPD_URL/smart/repo.git"
+'
+
+test_expect_success SSL_VERIFYSTATUS 'per-URL sslVerifyStatus is not applied to other URLs' '
+	git -c "http.https://example.com/.sslVerifyStatus=true" \
+		ls-remote "$HTTPD_URL/smart/repo.git" >actual &&
+	test_line_count -gt 0 actual
+'
+
 # here user%40host is the URL-encoded version of user@host,
 # which is our intentionally-odd username to catch parsing errors
 url_user=$HTTPD_URL_USER/auth/smart/repo.git
-- 
2.50.1 (Apple Git-155)