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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2026, 19:28 UTC
Message-ID: <xmqq8q6c1mpi.fsf@gitster.g>
In-Reply-To: <20260811170200.43097-1-ggordon@gitlab.com>

graysongordon-gl <graysongordon1@gmail.com> writes:

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

Good to see that the author paid extra attention to compatibility.

Show 11 quoted lines

> +	/*
> +	 * Ask the TLS backend to check the certificate's revocation
> +	 * status via the stapled OCSP response. libcurl defaults this
> +	 * off, and no backend except GnuTLS consults the staple on its
> +	 * own, so without this git will happily accept a certificate
> +	 * whose own staple says it has been revoked.
> +	 *
> +	 * Off by default because it is fail-closed: a server that
> +	 * staples nothing fails verification outright, so enabling it
> +	 * globally would break every remote that does not staple.
> +	 */

The comment may not be telling any lies per se, but it is dubious that this belongs here as an in-code comment. Developers hunting a bug they suspect this setting might have caused will need access to this information, and they can access it by running 'git blame' to locate the commit that introduced the code. As long as a solid commit log message explains how you arrived at various design decisions (such as 'off by default because'), they can use that as a starting point. For other developers hunting different bugs or trying to add their own enhancements, the comment is a mere distraction.

Show 5 quoted lines

> +	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"));
> +

Thanks.