Re: [PATCH +warn] Implement https public key pinning

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2016, 03:19 UTC
Message-ID: <xmqqziv1mjf8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160216012210.GB11961@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> On Mon, Feb 15, 2016 at 03:25:32PM -0800, Junio C Hamano wrote:
>
>> Thanks.  This, when applied on top of 2.7.1, however seems to break
>> at least t5541 and t5551.
>
> Hrm. I cannot see how the new code can possibly do anything unless
> http.pinnedpubkey is set, and our tests don't do that. Neither t5541 nor
> t5551 fails for me with the patch on top of v2.7.1 (or current "pu", for
> that matter).

> What does the failure look like?

In t5541, #17 "push (chunked)" fails.

The test expects to see "POST git-receive-pack (chunked)" in the error output, but here is what I see in $TRASH/test_repo_clone/err:

    Pushing to http://127.0.0.1:5541/smart/test_repo.git
    POST git-receive-pack (467 bytes)
    To http://127.0.0.1:5541/smart/test_repo.git
       8598732..09a7db2  master -> master
    updating local tracking ref 'refs/remotes/origin/master'

"git reset --hard HEAD^" to get rid of this patch before retesting makes the same test pass, so even though I cannot see how this could make any difference, it apparently is making some difference.

#define LIBCURL_VERSION_NUM 0x072300

I suspect that "#else" is too agressive to bail out or something silly like that.

Oh, I think I found it.

@@ -216,6 +219,13 @@ static int http_options(const char *var, const char *value, void *cb)
 	if (!strcmp("http.sslcapath", var))
 		return git_config_pathname(&ssl_capath, var, value);
 #endif
+	if (!strcmp("http.pinnedpubkey", var))
+#if LIBCURL_VERSION_NUM >= 0x072c00
+		return git_config_pathname(&ssl_pinnedkey, var, value);
+#else
+		warning(_("Public key pinning not supported with cURL < 7.44.0"));
+		return 0;
+#endif

We are not writing in Python.  Indenting the second line the same
way does not make it part of the block.  Of course by inserting the
new config in the earlier part of the function, it broke everything
that comes after.




 	if (!strcmp("http.sslcainfo", var))
 		return git_config_pathname(&ssl_cainfo, var, value);
 	if (!strcmp("http.sslcertpasswordprotected", var)) {
@@ -415,6 +425,10 @@ static CURL *get_curl_handle(void)
 	if (ssl_capath != NULL)
 		curl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);
 #endif
+#if LIBCURL_VERSION_NUM >= 0x072c00
+	if (ssl_pinnedkey != NULL)
+		curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);
+#endif
 	if (ssl_cainfo != NULL)
 		curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);