{"thread":{"id":"41395","subject":"[PATCH] Implement https public key pinning","startedAt":"2016-02-11T22:54:37Z","lastAt":"2016-02-22T15:41:55Z","messageCount":16,"participants":["Christoph Egger","Daniel Stenberg","Thomas Gummerer","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"277977","messageId":"20160211225437.GA33955@mitoraj.siccegge.de","threadId":"41395","inReplyTo":null,"subject":"[PATCH] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-11T22:54:37Z","receivedAt":"2016-02-11T22:54:37Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Add the http.pinnedpubkey configuration option for public key\npinning. It allows any string supported by libcurl --\nbase64(sha256(pubkey)) or filename of the full public key.\n\nSigned-off-by: Christoph Egger \n---\n\n For some more sensitive repositories I'd like to properly pin the\n public key of the https service. libcURL properly supports this since\n 7.44.0, some parts earlier, the option just needs to be exposed by\n git.\n\n There seem to be no test regressions.\n\n Documentation/config.txt | 6 ++++++\n http.c | 11 +++++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 27f02be..35b4495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,6 +1727,12 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the 'GIT_SSL_CAPATH' environment variable.\n \n+http.pinnedpubkey::\n+\tPublic key of the https service. It may either be the filename of\n+\ta PEM or DER encoded public key file or a string starting with\n+\t'sha256//' followed by the base64 encoded sha256 hash of the\n+\tpublic key. See also libcurl 'CURLOPT_PINNEDPUBLICKEY'.\n+\n http.sslTry::\n \tAttempt to use AUTH SSL/TLS and encrypted data transfers\n \twhen connecting via regular FTP protocol. This might be needed\ndiff --git a/http.c b/http.c\nindex dfc53c1..60776cc 100644\n--- a/http.c\n+++ b/http.c\n@@ -57,6 +57,9 @@ static const char *ssl_key;\n #if LIBCURL_VERSION_NUM >= 0x070908\n static const char *ssl_capath;\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+static const char *ssl_pinnedkey;\n+#endif\n static const char *ssl_cainfo;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n@@ -239,6 +242,10 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (!strcmp(\"http.pinnedpubkey\", var))\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#endif\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -499,6 +506,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x074400\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n-- \n2.7.0\n\n\n-- \n"},{"id":"277978","messageId":"alpine.DEB.2.20.1602120030120.5268@tvnag.unkk.fr","threadId":"41395","inReplyTo":"20160211225437.GA33955@mitoraj.siccegge.de","subject":"Re: [PATCH] Implement https public key pinning","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2016-02-11T23:30:57Z","receivedAt":"2016-02-11T23:30:57Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 11 Feb 2016, Christoph Egger wrote:\n\n> +#if LIBCURL_VERSION_NUM >= 0x074400\n\nThat should probably be 0x072c00 ...\n\n-- \n\n / daniel.haxx.se\n"},{"id":"277986","messageId":"87oabmg24u.fsf@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"alpine.DEB.2.20.1602120030120.5268@tvnag.unkk.fr","subject":"Re: [PATCH] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-12T01:15:29Z","receivedAt":"2016-02-12T01:15:29Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Daniel Stenberg writes:\n> On Thu, 11 Feb 2016, Christoph Egger wrote:\n>> +#if LIBCURL_VERSION_NUM >= 0x074400\n>\n> That should probably be 0x072c00 ...\n\nThis is, of course, right.\n\nI used 7.44 / 0x072c00 as base because it has robust support for this\nfeature (including the sha256// variant). One could lower that depending\non the compromises one is willing to take FWIW\n\n Added in 7.39.0 for OpenSSL, GnuTLS and GSKit. Added in 7.43.0 for NSS\n and wolfSSL/CyaSSL. Added for mbedtls in 7.47.0, sha256 support added\n in 7.44.0 for OpenSSL, GnuTLS, NSS and wolfSSL/CyaSSL. Other SSL\n backends not supported.\n\nAlso some people suggested that git should fail if this option is\nrequested in the config but not supported by the libcurl version instead\nof falling back to just not pin the key. I'm undecided about that.\n\n Christoph\n"},{"id":"277987","messageId":"20160212011834.GA3322@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"87oabmg24u.fsf@mitoraj.siccegge.de","subject":"[PATCH v2] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-12T01:18:34Z","receivedAt":"2016-02-12T01:18:34Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Add the http.pinnedpubkey configuration option for public key\npinning. It allows any string supported by libcurl --\nbase64(sha256(pubkey)) or filename of the full public key.\n\nSigned-off-by: Christoph Egger \n---\n Documentation/config.txt | 6 ++++++\n http.c | 11 +++++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 27f02be..35b4495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,6 +1727,12 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the 'GIT_SSL_CAPATH' environment variable.\n \n+http.pinnedpubkey::\n+\tPublic key of the https service. It may either be the filename of\n+\ta PEM or DER encoded public key file or a string starting with\n+\t'sha256//' followed by the base64 encoded sha256 hash of the\n+\tpublic key. See also libcurl 'CURLOPT_PINNEDPUBLICKEY'.\n+\n http.sslTry::\n \tAttempt to use AUTH SSL/TLS and encrypted data transfers\n \twhen connecting via regular FTP protocol. This might be needed\ndiff --git a/http.c b/http.c\nindex dfc53c1..5549fe5 100644\n--- a/http.c\n+++ b/http.c\n@@ -57,6 +57,9 @@ static const char *ssl_key;\n #if LIBCURL_VERSION_NUM >= 0x070908\n static const char *ssl_capath;\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+static const char *ssl_pinnedkey;\n+#endif\n static const char *ssl_cainfo;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n@@ -239,6 +242,10 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (!strcmp(\"http.pinnedpubkey\", var))\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#endif\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -499,6 +506,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n-- \n2.7.0\n\n\n-- \n"},{"id":"278003","messageId":"20160212100226.GB13775@hank","threadId":"41395","inReplyTo":"87oabmg24u.fsf@mitoraj.siccegge.de","subject":"Re: [PATCH] Implement https public key pinning","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-02-12T10:02:26Z","receivedAt":"2016-02-12T10:02:26Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/12, Christoph Egger wrote:\n> Daniel Stenberg writes:\n> > On Thu, 11 Feb 2016, Christoph Egger wrote:\n> >> +#if LIBCURL_VERSION_NUM >= 0x074400\n> >\n> > That should probably be 0x072c00 ...\n>\n> This is, of course, right.\n>\n> I used 7.44 / 0x072c00 as base because it has robust support for this\n> feature (including the sha256// variant). One could lower that depending\n> on the compromises one is willing to take FWIW\n>\n> Added in 7.39.0 for OpenSSL, GnuTLS and GSKit. Added in 7.43.0 for NSS\n> and wolfSSL/CyaSSL. Added for mbedtls in 7.47.0, sha256 support added\n> in 7.44.0 for OpenSSL, GnuTLS, NSS and wolfSSL/CyaSSL. Other SSL\n> backends not supported.\n>\n> Also some people suggested that git should fail if this option is\n> requested in the config but not supported by the libcurl version instead\n> of falling back to just not pin the key. I'm undecided about that.\n\nThis seems to have been suggested off list (or at least I can't find\nthe message). FWIW I do agree with failing or as a bare minimum\nwarning the user if the config option is set, but not supported by the\nlibcurl version. Otherwise we risk giving the user a false sense of\nsecurity when the option is set, which is arguably worse than not\nhaving the security option at all.\n\n> Christoph\n"},{"id":"278025","messageId":"20160212183710.GC19973@sigill.intra.peff.net","threadId":"41395","inReplyTo":"20160212100226.GB13775@hank","subject":"Re: [PATCH] Implement https public key pinning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-12T18:37:10Z","receivedAt":"2016-02-12T18:37:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 12, 2016 at 11:02:26AM +0100, Thomas Gummerer wrote:\n\n> > Also some people suggested that git should fail if this option is\n> > requested in the config but not supported by the libcurl version instead\n> > of falling back to just not pin the key. I'm undecided about that.\n> \n> This seems to have been suggested off list (or at least I can't find\n> the message). FWIW I do agree with failing or as a bare minimum\n> warning the user if the config option is set, but not supported by the\n> libcurl version. Otherwise we risk giving the user a false sense of\n> security when the option is set, which is arguably worse than not\n> having the security option at all.\n\nWe can't do this perfectly, because older versions of git do not yet\nknow about the option, and will therefore just silently ignore it. And\nfor consistency there, we usually do the same for features that we know\nabout but are unsupported.\n\nBut I agree for something with security implications like this, we are\nbetter off warning when we know support is not built in. That's not\nperfect, but it's better than nothing.\n\nI wonder if there are other options which should get the same treatment.\nMost of the obvious ones I could think of (e.g., http.sslverify) do not\nneed it, because either they always have support built, or they\nfail-closed, or both.\n\n-Peff\n"},{"id":"278161","messageId":"87y4amcby5.fsf@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"20160212183710.GC19973@sigill.intra.peff.net","subject":"Re: [PATCH] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-15T13:58:26Z","receivedAt":"2016-02-15T13:58:26Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Jeff King writes:\n> We can't do this perfectly, because older versions of git do not yet\n> know about the option, and will therefore just silently ignore it. And\n> for consistency there, we usually do the same for features that we know\n> about but are unsupported.\n\nJep that's why I originally did it this way. But if I (the user) just\nhave to check the git version to know I'm fine (and not also check which\nversion of curl it is linked with) to be sure I'd assume that's an\nimprovement still.\n\n> But I agree for something with security implications like this, we are\n> better off warning when we know support is not built in. That's not\n> perfect, but it's better than nothing.\n\nI'll add an updated patch taking this into account\n\n> I wonder if there are other options which should get the same treatment.\n> Most of the obvious ones I could think of (e.g., http.sslverify) do not\n> need it, because either they always have support built, or they\n> fail-closed, or both.\n\ndoes CURLOPT_CAPATH add to CURLOPT_CAINFO or replace it? The\ndocumentation [0] is inconclusive to me in this regard.\n\n Christoph\n\n[0] https://curl.haxx.se/libcurl/c/CURLOPT_CAPATH.html\n"},{"id":"278162","messageId":"20160215140422.GA1747@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"87y4amcby5.fsf@mitoraj.siccegge.de","subject":"[PATCH +warn] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-15T14:04:22Z","receivedAt":"2016-02-15T14:04:22Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Add the http.pinnedpubkey configuration option for public key\npinning. It allows any string supported by libcurl --\nbase64(sha256(pubkey)) or filename of the full public key.\n\nIf cURL does not support pinning (is too old) output a warning to the\nuser.\n\nSigned-off-by: Christoph Egger \n---\n\n This version of the patch adds a warning to the user if the option is\n not supported.\n\n Documentation/config.txt | 8 ++++++++\n http.c | 14 ++++++++++++++\n 2 files changed, 22 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 27f02be..0f2643b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,6 +1727,14 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the 'GIT_SSL_CAPATH' environment variable.\n \n+http.pinnedpubkey::\n+\tPublic key of the https service. It may either be the filename of\n+\ta PEM or DER encoded public key file or a string starting with\n+\t'sha256//' followed by the base64 encoded sha256 hash of the\n+\tpublic key. See also libcurl 'CURLOPT_PINNEDPUBLICKEY'. git will\n+\texit with an error if this option is set but not supported by\n+\tcURL.\n+\n http.sslTry::\n \tAttempt to use AUTH SSL/TLS and encrypted data transfers\n \twhen connecting via regular FTP protocol. This might be needed\ndiff --git a/http.c b/http.c\nindex dfc53c1..0bb9237 100644\n--- a/http.c\n+++ b/http.c\n@@ -57,6 +57,9 @@ static const char *ssl_key;\n #if LIBCURL_VERSION_NUM >= 0x070908\n static const char *ssl_capath;\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+static const char *ssl_pinnedkey;\n+#endif\n static const char *ssl_cainfo;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n@@ -239,6 +242,13 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n+\tif (!strcmp(\"http.pinnedpubkey\", var))\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#else\n+\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n+\t\treturn 0;\n+#endif\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -499,6 +509,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n-- \n2.7.0\n\n\n-- \n"},{"id":"278244","messageId":"xmqqr3gdo8sz.fsf@gitster.mtv.corp.google.com","threadId":"41395","inReplyTo":"20160215140422.GA1747@mitoraj.siccegge.de","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-15T23:25:32Z","receivedAt":"2016-02-15T23:25:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. This, when applied on top of 2.7.1, however seems to break\nat least t5541 and t5551.\n"},{"id":"278257","messageId":"20160216012210.GB11961@sigill.intra.peff.net","threadId":"41395","inReplyTo":"xmqqr3gdo8sz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-16T01:22:11Z","receivedAt":"2016-02-16T01:22:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 03:25:32PM -0800, Junio C Hamano wrote:\n\n> Thanks. This, when applied on top of 2.7.1, however seems to break\n> at least t5541 and t5551.\n\nHrm. I cannot see how the new code can possibly do anything unless\nhttp.pinnedpubkey is set, and our tests don't do that. Neither t5541 nor\nt5551 fails for me with the patch on top of v2.7.1 (or current \"pu\", for\nthat matter).\n\nWhat does the failure look like?\n\n-Peff\n"},{"id":"278265","messageId":"xmqqziv1mjf8.fsf@gitster.mtv.corp.google.com","threadId":"41395","inReplyTo":"20160216012210.GB11961@sigill.intra.peff.net","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-16T03:19:07Z","receivedAt":"2016-02-16T03:19:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 15, 2016 at 03:25:32PM -0800, Junio C Hamano wrote:\n>\n>> Thanks. This, when applied on top of 2.7.1, however seems to break\n>> at least t5541 and t5551.\n>\n> Hrm. I cannot see how the new code can possibly do anything unless\n> http.pinnedpubkey is set, and our tests don't do that. Neither t5541 nor\n> t5551 fails for me with the patch on top of v2.7.1 (or current \"pu\", for\n> that matter).\n\n> What does the failure look like?\n\nIn t5541, #17 \"push (chunked)\" fails.\n\nThe test expects to see \"POST git-receive-pack (chunked)\" in the\nerror output, but here is what I see in $TRASH/test_repo_clone/err:\n\n Pushing to http://127.0.0.1:5541/smart/test_repo.git\n POST git-receive-pack (467 bytes)\n To http://127.0.0.1:5541/smart/test_repo.git\n 8598732..09a7db2 master -> master\n updating local tracking ref 'refs/remotes/origin/master'\n\n\"git reset --hard HEAD^\" to get rid of this patch before retesting\nmakes the same test pass, so even though I cannot see how this could\nmake any difference, it apparently is making some difference.\n\n#define LIBCURL_VERSION_NUM 0x072300\n\nI suspect that \"#else\" is too agressive to bail out or something\nsilly like that.\n\nOh, I think I found it.\n\n@@ -216,6 +219,13 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n+\tif (!strcmp(\"http.pinnedpubkey\", var))\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#else\n+\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n+\t\treturn 0;\n+#endif\n\nWe are not writing in Python. Indenting the second line the same\nway does not make it part of the block. Of course by inserting the\nnew config in the earlier part of the function, it broke everything\nthat comes after.\n\n\n\n\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -415,6 +425,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n"},{"id":"278267","messageId":"20160216032818.GB19954@sigill.intra.peff.net","threadId":"41395","inReplyTo":"xmqqziv1mjf8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-16T03:28:19Z","receivedAt":"2016-02-16T03:28:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 07:19:07PM -0800, Junio C Hamano wrote:\n\n> I suspect that \"#else\" is too agressive to bail out or something\n> silly like that.\n> \n> Oh, I think I found it.\n> \n> @@ -216,6 +219,13 @@ static int http_options(const char *var, const char *value, void *cb)\n> \tif (!strcmp(\"http.sslcapath\", var))\n> \t\treturn git_config_pathname(&ssl_capath, var, value);\n> #endif\n> +\tif (!strcmp(\"http.pinnedpubkey\", var))\n> +#if LIBCURL_VERSION_NUM >= 0x072c00\n> +\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n> +#else\n> +\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n> +\t\treturn 0;\n> +#endif\n> \n> We are not writing in Python. Indenting the second line the same\n> way does not make it part of the block. Of course by inserting the\n> new config in the earlier part of the function, it broke everything\n> that comes after.\n\nOof. Good eyes. I suspected something funny with the #if, but after\nstarting at it for minutes, couldn't see it.\n\nThat makes sense, and the fix is obvious.\n\n-Peff\n"},{"id":"278295","messageId":"20160216111921.GA67844@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"20160216032818.GB19954@sigill.intra.peff.net","subject":"[PATCH +warn2] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-16T11:19:21Z","receivedAt":"2016-02-16T11:19:21Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Add the http.pinnedpubkey configuration option for public key\npinning. It allows any string supported by libcurl --\nbase64(sha256(pubkey)) or filename of the full public key.\n\nIf cURL does not support pinning (is too old) output a warning to the\nuser.\n\nSigned-off-by: Christoph Egger \n---\n\n Now tested again both with and without a \"new enough\" cURL version.\n Passes tests in both configurations and is obviously more correct.\n\n Documentation/config.txt | 8 ++++++++\n http.c | 15 +++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 27f02be..0f2643b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,6 +1727,14 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the 'GIT_SSL_CAPATH' environment variable.\n \n+http.pinnedpubkey::\n+\tPublic key of the https service. It may either be the filename of\n+\ta PEM or DER encoded public key file or a string starting with\n+\t'sha256//' followed by the base64 encoded sha256 hash of the\n+\tpublic key. See also libcurl 'CURLOPT_PINNEDPUBLICKEY'. git will\n+\texit with an error if this option is set but not supported by\n+\tcURL.\n+\n http.sslTry::\n \tAttempt to use AUTH SSL/TLS and encrypted data transfers\n \twhen connecting via regular FTP protocol. This might be needed\ndiff --git a/http.c b/http.c\nindex dfc53c1..f640a8b 100644\n--- a/http.c\n+++ b/http.c\n@@ -57,6 +57,9 @@ static const char *ssl_key;\n #if LIBCURL_VERSION_NUM >= 0x070908\n static const char *ssl_capath;\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+static const char *ssl_pinnedkey;\n+#endif\n static const char *ssl_cainfo;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n@@ -239,6 +242,14 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n+\tif (!strcmp(\"http.pinnedpubkey\", var)) {\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#else\n+\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n+\t\treturn 0;\n+#endif\n+\t}\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -499,6 +510,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n-- \n2.7.0\n\n\n-- \n"},{"id":"278389","messageId":"xmqqr3gcmjwm.fsf@gitster.mtv.corp.google.com","threadId":"41395","inReplyTo":"20160216032818.GB19954@sigill.intra.peff.net","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-16T21:20:57Z","receivedAt":"2016-02-16T21:20:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 15, 2016 at 07:19:07PM -0800, Junio C Hamano wrote:\n>\n>> I suspect that \"#else\" is too agressive to bail out or something\n>> silly like that.\n>> \n>> Oh, I think I found it.\n>> \n>> @@ -216,6 +219,13 @@ static int http_options(const char *var, const char *value, void *cb)\n>> \tif (!strcmp(\"http.sslcapath\", var))\n>> \t\treturn git_config_pathname(&ssl_capath, var, value);\n>> #endif\n>> +\tif (!strcmp(\"http.pinnedpubkey\", var))\n>> +#if LIBCURL_VERSION_NUM >= 0x072c00\n>> +\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n>> +#else\n>> +\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n>> +\t\treturn 0;\n>> +#endif\n>> \n>> We are not writing in Python. Indenting the second line the same\n>> way does not make it part of the block. Of course by inserting the\n>> new config in the earlier part of the function, it broke everything\n>> that comes after.\n>\n> Oof. Good eyes. I suspected something funny with the #if, but after\n> starting at it for minutes, couldn't see it.\n>\n> That makes sense, and the fix is obvious.\n\nYup. I think I sent a patch already and I followed the \"when adding\na new thing to a set, add it at the end unless there is other reason\nto place it elsewhere\".\n\nThanks.\n"},{"id":"278507","messageId":"xmqqlh6jhwu0.fsf@gitster.mtv.corp.google.com","threadId":"41395","inReplyTo":"20160215140422.GA1747@mitoraj.siccegge.de","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-17T21:05:11Z","receivedAt":"2016-02-17T21:05:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christoph Egger writes:\n\n> Add the http.pinnedpubkey configuration option for public key\n> pinning. It allows any string supported by libcurl --\n> base64(sha256(pubkey)) or filename of the full public key.\n>\n> If cURL does not support pinning (is too old) output a warning to the\n> user.\n>\n> Signed-off-by: Christoph Egger \n> ---\n\nI needed this fix to unbreak it for those with older versions of\ncURL.\n\n\n http.c | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex a6b8076..3475040 100644\n--- a/http.c\n+++ b/http.c\n@@ -219,13 +219,6 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslcapath\", var))\n \t\treturn git_config_pathname(&ssl_capath, var, value);\n #endif\n-\tif (!strcmp(\"http.pinnedpubkey\", var))\n-#if LIBCURL_VERSION_NUM >= 0x072c00\n-\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n-#else\n-\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n-\t\treturn 0;\n-#endif\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n@@ -283,6 +276,14 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.useragent\", var))\n \t\treturn git_config_string(&user_agent, var, value);\n \n+\tif (!strcmp(\"http.pinnedpubkey\", var)) {\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#else\n+\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n+\t\treturn 0;\n+#endif\n+\t}\n \t/* Fall back on the default ones */\n \treturn git_default_config(var, value, cb);\n }\n"},{"id":"278882","messageId":"8760xg7nwc.fsf@mitoraj.siccegge.de","threadId":"41395","inReplyTo":"xmqqlh6jhwu0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH +warn] Implement https public key pinning","fromName":"Christoph Egger","fromEmail":"christoph@christoph-egger.org","sentAt":"2016-02-22T15:41:55Z","receivedAt":"2016-02-22T15:41:55Z","isPatch":true,"sender":{"key":"christoph@christoph-egger.org","avatar":"https://avatars.githubusercontent.com/u/405064?v=4"},"body":"Hi!\n\nJunio C Hamano writes:\n> Christoph Egger writes:\n>\n>> Add the http.pinnedpubkey configuration option for public key\n>> pinning. It allows any string supported by libcurl --\n>> base64(sha256(pubkey)) or filename of the full public key.\n>>\n>> If cURL does not support pinning (is too old) output a warning to the\n>> user.\n>>\n>> Signed-off-by: Christoph Egger \n>> ---\n>\n> I needed this fix to unbreak it for those with older versions of\n> cURL.\n\nJep sorry about that. should have run a second test with old libcurl.\nI've attached a consolidated patch.\n\n Christoph\n\n\n\n>From be8112d695de534629bcb3411634d101a74021a7 Mon Sep 17 00:00:00 2001\nFrom: Christoph Egger \nDate: Thu, 11 Feb 2016 23:28:20 +0100\nSubject: [PATCH] Implement https public key pinning\n\nAdd the http.pinnedpubkey configuration option for public key\npinning. It allows any string supported by libcurl --\nbase64(sha256(pubkey)) or filename of the full public key.\n\nIf cURL does not support pinning (is too old) output a warning to the\nuser.\n\nSigned-off-by: Christoph Egger \n---\n Documentation/config.txt | 8 ++++++++\n http.c | 16 ++++++++++++++++\n 2 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 27f02be..0f2643b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,6 +1727,14 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the 'GIT_SSL_CAPATH' environment variable.\n \n+http.pinnedpubkey::\n+\tPublic key of the https service. It may either be the filename of\n+\ta PEM or DER encoded public key file or a string starting with\n+\t'sha256//' followed by the base64 encoded sha256 hash of the\n+\tpublic key. See also libcurl 'CURLOPT_PINNEDPUBLICKEY'. git will\n+\texit with an error if this option is set but not supported by\n+\tcURL.\n+\n http.sslTry::\n \tAttempt to use AUTH SSL/TLS and encrypted data transfers\n \twhen connecting via regular FTP protocol. This might be needed\ndiff --git a/http.c b/http.c\nindex dfc53c1..1c295dd 100644\n--- a/http.c\n+++ b/http.c\n@@ -57,6 +57,9 @@ static const char *ssl_key;\n #if LIBCURL_VERSION_NUM >= 0x070908\n static const char *ssl_capath;\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+static const char *ssl_pinnedkey;\n+#endif\n static const char *ssl_cainfo;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n@@ -299,6 +302,15 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.useragent\", var))\n \t\treturn git_config_string(&user_agent, var, value);\n \n+\tif (!strcmp(\"http.pinnedpubkey\", var)) {\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n+#else\n+\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n+\t\treturn 0;\n+#endif\n+\t}\n+\n \t/* Fall back on the default ones */\n \treturn git_default_config(var, value, cb);\n }\n@@ -499,6 +511,10 @@ static CURL *get_curl_handle(void)\n \tif (ssl_capath != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);\n #endif\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\tif (ssl_pinnedkey != NULL)\n+\t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n+#endif\n \tif (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n-- \n2.7.0\n\n\n\n\n> http.c | 15 ++++++++-------\n> 1 file changed, 8 insertions(+), 7 deletions(-)\n>\n> diff --git a/http.c b/http.c\n> index a6b8076..3475040 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -219,13 +219,6 @@ static int http_options(const char *var, const char *value, void *cb)\n> \tif (!strcmp(\"http.sslcapath\", var))\n> \t\treturn git_config_pathname(&ssl_capath, var, value);\n> #endif\n> -\tif (!strcmp(\"http.pinnedpubkey\", var))\n> -#if LIBCURL_VERSION_NUM >= 0x072c00\n> -\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n> -#else\n> -\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n> -\t\treturn 0;\n> -#endif\n> \tif (!strcmp(\"http.sslcainfo\", var))\n> \t\treturn git_config_pathname(&ssl_cainfo, var, value);\n> \tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n> @@ -283,6 +276,14 @@ static int http_options(const char *var, const char *value, void *cb)\n> \tif (!strcmp(\"http.useragent\", var))\n> \t\treturn git_config_string(&user_agent, var, value);\n> \n> +\tif (!strcmp(\"http.pinnedpubkey\", var)) {\n> +#if LIBCURL_VERSION_NUM >= 0x072c00\n> +\t\treturn git_config_pathname(&ssl_pinnedkey, var, value);\n> +#else\n> +\t\twarning(_(\"Public key pinning not supported with cURL < 7.44.0\"));\n> +\t\treturn 0;\n> +#endif\n> +\t}\n> \t/* Fall back on the default ones */\n> \treturn git_default_config(var, value, cb);\n> }\n\n-- \n"}]}