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

> 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.

> 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.

> diff --git a/http.c b/http.c
> index caccf2108e..94f8dd817a 100644
> --- a/http.c
> +++ b/http.c
> @@ -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"));

> 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
> +'