signed push: fortify against replay attacks

In order to prevent a valid push certificate for pushing into an repository from getting replayed in a different push operation, send a nonce string from the receive-pack process and have the signer include it in the push certificate. The receiving end uses an HMAC hash of the path to the repository it serves and the current time stamp, hashed with a secret seed (the secret seed does not have to be per-repository but can be defined in /etc/gitconfig) to generate the nonce, in order to ensure that a random third party cannot forge a nonce that looks like it originated from it. The original nonce is exported as GIT_PUSH_CERT_NONCE for the hooks to examine and match against the value on the "nonce" header in the certificate to notice a replay, but returned "nonce" header in the push certificate is examined by receive-pack and the result is exported as GIT_PUSH_CERT_NONCE_STATUS, whose value would be "OK" if the nonce recorded in the certificate matches what we expect, so that the hooks can more easily check. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Aug 21, 2014 at 16:45 UTC b89363e4a5277038629491f8765c0598f366326c
7 files changed +187 -29
Documentation/config.txt
+6 -6
@@ -2038,17 +2038,17 @@ rebase.autostash::
2038 successful rebase might result in non-trivial conflicts.
2039 Defaults to false.
2040
2041 -receive.acceptpushcert::
2042 - By default, `git receive-pack` will advertise that it
2043 - accepts `git push --signed`. Setting this variable to
2044 - false disables it (this is a tentative variable that
2045 - will go away at the end of this series).
2046 -
2041 receive.autogc::
2042 By default, git-receive-pack will run "git-gc --auto" after
2043 receiving data from git-push and updating refs. You can stop
2044 it by setting this variable to false.
2045
2046 +receive.certnonceseed::
2047 + By setting this variable to a string, `git receive-pack`
2048 + will accept a `git push --signed` and verifies it by using
2049 + a "nonce" protected by HMAC using this string as a secret
2050 + key.
2051 +
2052 receive.fsckObjects::
2053 If it is set to true, git-receive-pack will check all received
2054 objects. It will abort in the case of a malformed object or a
Documentation/git-receive-pack.txt
+19
@@ -72,6 +72,24 @@ the following environment variables:
72 using the same mnemonic as used in `%G?` format of `git log`
73 family of commands (see linkgit:git-log[1]).
74
75 +`GIT_PUSH_CERT_NONCE`::
76 + The nonce string the process asked the signer to include
77 + in the push certificate. If this does not match the value
78 + recorded on the "nonce" header in the push certificate, it
79 + may indicate that the certificate is a valid one that is
80 + being replayed from a separate "git push" session.
81 +
82 +`GIT_PUSH_CERT_NONCE_STATUS`::
83 +`UNSOLICITED`;;
84 + "git push --signed" sent a nonce when we did not ask it to
85 + send one.
86 +`MISSING`;;
87 + "git push --signed" did not send any nonce header.
88 +`BAD`;;
89 + "git push --signed" sent a bogus nonce.
90 +`OK`;;
91 + "git push --signed" sent the nonce we asked it to send.
92 +
93 This hook is called before any refname is updated and before any
94 fast-forward checks are performed.
95
@@ -147,6 +165,7 @@ service:
165 if test -n "${GIT_PUSH_CERT-}" && test ${GIT_PUSH_CERT_STATUS} = G
166 then
167 (
168 + echo expected nonce is ${GIT_PUSH_NONCE}
169 git cat-file blob ${GIT_PUSH_CERT}
170 ) | mail -s "push certificate from $GIT_PUSH_CERT_SIGNER" push-log@mydomain
171 fi
Documentation/technical/pack-protocol.txt
+6
@@ -485,6 +485,7 @@ references.
485 PKT-LINE("certificate version 0.1" LF)
486 PKT-LINE("pusher" SP ident LF)
487 PKT-LINE("pushee" SP url LF)
488 + PKT-LINE("nonce" SP nonce LF)
489 PKT-LINE(LF)
490 *PKT-LINE(command LF)
491 *PKT-LINE(gpg-signature-lines LF)
@@ -533,6 +534,11 @@ Currently, the following header fields are defined:
534 authentication material) the user who ran `git push`
535 intended to push into.
536
537 +`nonce` nonce::
538 + The 'nonce' string the receiving repository asked the
539 + pushing user to include in the certificate, to prevent
540 + replay attacks.
541 +
542 The GPG signature lines are a detached signature for the contents
543 recorded in the push certificate before the signature block begins.
544 The detached signature is used to certify that the commands were
Documentation/technical/protocol-capabilities.txt
+4 -3
@@ -251,10 +251,11 @@ If the upload-pack server advertises this capability, fetch-pack may
251 send "want" lines with SHA-1s that exist at the server but are not
252 advertised by upload-pack.
253
254 -push-cert
255 ----------
254 +push-cert=<nonce>
255 +-----------------
256
257 The receive-pack server that advertises this capability is willing
258 -to accept a signed push certificate. A send-pack client MUST NOT
258 +to accept a signed push certificate, and asks the <nonce> to be
259 +included in the push certificate. A send-pack client MUST NOT
260 send a push-cert packet unless the receive-pack server advertises
261 this capability.
builtin/receive-pack.c
+124 -8
@@ -48,10 +48,17 @@ static void *head_name_to_free;
48 static int sent_capabilities;
49 static int shallow_update;
50 static const char *alt_shallow_file;
51 -static int accept_push_cert = 1;
51 static struct strbuf push_cert = STRBUF_INIT;
52 static unsigned char push_cert_sha1[20];
53 static struct signature_check sigcheck;
54 +static const char *push_cert_nonce;
55 +static const char *cert_nonce_seed;
56 +
57 +static const char *NONCE_UNSOLICITED = "UNSOLICITED";
58 +static const char *NONCE_BAD = "BAD";
59 +static const char *NONCE_MISSING = "MISSING";
60 +static const char *NONCE_OK = "OK";
61 +static const char *nonce_status;
62
63 static enum deny_action parse_deny_action(const char *var, const char *value)
64 {
@@ -135,10 +142,8 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
142 return 0;
143 }
144
138 - if (strcmp(var, "receive.acceptpushcert") == 0) {
139 - accept_push_cert = git_config_bool(var, value);
140 - return 0;
141 - }
145 + if (strcmp(var, "receive.certnonceseed") == 0)
146 + return git_config_string(&cert_nonce_seed, var, value);
147
148 return git_default_config(var, value, cb);
149 }
@@ -157,8 +162,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
162 "report-status delete-refs side-band-64k quiet");
163 if (prefer_ofs_delta)
164 strbuf_addstr(&cap, " ofs-delta");
160 - if (accept_push_cert)
161 - strbuf_addstr(&cap, " push-cert");
165 + if (push_cert_nonce)
166 + strbuf_addf(&cap, " push-cert=%s", push_cert_nonce);
167 strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
168 packet_write(1, "%s %s%c%s\n",
169 sha1_to_hex(sha1), path, 0, cap.buf);
@@ -271,6 +276,110 @@ static int copy_to_sideband(int in, int out, void *arg)
276 return 0;
277 }
278
279 +#define HMAC_BLOCK_SIZE 64
280 +
281 +static void hmac_sha1(unsigned char out[20],
282 + const char *key_in, size_t key_len,
283 + const char *text, size_t text_len)
284 +{
285 + unsigned char key[HMAC_BLOCK_SIZE];
286 + unsigned char k_ipad[HMAC_BLOCK_SIZE];
287 + unsigned char k_opad[HMAC_BLOCK_SIZE];
288 + int i;
289 + git_SHA_CTX ctx;
290 +
291 + /* RFC 2104 2. (1) */
292 + memset(key, '\0', HMAC_BLOCK_SIZE);
293 + if (HMAC_BLOCK_SIZE < key_len) {
294 + git_SHA1_Init(&ctx);
295 + git_SHA1_Update(&ctx, key_in, key_len);
296 + git_SHA1_Final(key, &ctx);
297 + } else {
298 + memcpy(key, key_in, key_len);
299 + }
300 +
301 + /* RFC 2104 2. (2) & (5) */
302 + for (i = 0; i < sizeof(key); i++) {
303 + k_ipad[i] = key[i] ^ 0x36;
304 + k_opad[i] = key[i] ^ 0x5c;
305 + }
306 +
307 + /* RFC 2104 2. (3) & (4) */
308 + git_SHA1_Init(&ctx);
309 + git_SHA1_Update(&ctx, k_ipad, sizeof(k_ipad));
310 + git_SHA1_Update(&ctx, text, text_len);
311 + git_SHA1_Final(out, &ctx);
312 +
313 + /* RFC 2104 2. (6) & (7) */
314 + git_SHA1_Init(&ctx);
315 + git_SHA1_Update(&ctx, k_opad, sizeof(k_opad));
316 + git_SHA1_Update(&ctx, out, sizeof(out));
317 + git_SHA1_Final(out, &ctx);
318 +}
319 +
320 +static char *prepare_push_cert_nonce(const char *path, unsigned long stamp)
321 +{
322 + struct strbuf buf = STRBUF_INIT;
323 + unsigned char sha1[20];
324 +
325 + strbuf_addf(&buf, "%s:%lu", path, stamp);
326 + hmac_sha1(sha1, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));;
327 + strbuf_release(&buf);
328 +
329 + /* RFC 2104 5. HMAC-SHA1-80 */
330 + strbuf_addf(&buf, "%lu-%.*s", stamp, 20, sha1_to_hex(sha1));
331 + return strbuf_detach(&buf, NULL);
332 +}
333 +
334 +/*
335 + * NEEDSWORK: reuse find_commit_header() from jk/commit-author-parsing
336 + * after dropping "_commit" from its name and possibly moving it out
337 + * of commit.c
338 + */
339 +static char *find_header(const char *msg, size_t len, const char *key)
340 +{
341 + int key_len = strlen(key);
342 + const char *line = msg;
343 +
344 + while (line && line < msg + len) {
345 + const char *eol = strchrnul(line, '\n');
346 +
347 + if ((msg + len <= eol) || line == eol)
348 + return NULL;
349 + if (line + key_len < eol &&
350 + !memcmp(line, key, key_len) && line[key_len] == ' ') {
351 + int offset = key_len + 1;
352 + return xmemdupz(line + offset, (eol - line) - offset);
353 + }
354 + line = *eol ? eol + 1 : NULL;
355 + }
356 + return NULL;
357 +}
358 +
359 +static const char *check_nonce(const char *buf, size_t len)
360 +{
361 + char *nonce = find_header(buf, len, "nonce");
362 + const char *retval = NONCE_BAD;
363 +
364 + if (!nonce) {
365 + retval = NONCE_MISSING;
366 + goto leave;
367 + } else if (!push_cert_nonce) {
368 + retval = NONCE_UNSOLICITED;
369 + goto leave;
370 + } else if (!strcmp(push_cert_nonce, nonce)) {
371 + retval = NONCE_OK;
372 + goto leave;
373 + }
374 +
375 + /* returned nonce MUST match what we gave out earlier */
376 + retval = NONCE_BAD;
377 +
378 +leave:
379 + free(nonce);
380 + return retval;
381 +}
382 +
383 static void prepare_push_cert_sha1(struct child_process *proc)
384 {
385 static int already_done;
@@ -305,6 +414,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)
414
415 strbuf_release(&gpg_output);
416 strbuf_release(&gpg_status);
417 + nonce_status = check_nonce(push_cert.buf, bogs);
418 }
419 if (!is_null_sha1(push_cert_sha1)) {
420 argv_array_pushf(&env, "GIT_PUSH_CERT=%s", sha1_to_hex(push_cert_sha1));
@@ -313,7 +423,10 @@ static void prepare_push_cert_sha1(struct child_process *proc)
423 argv_array_pushf(&env, "GIT_PUSH_CERT_KEY=%s",
424 sigcheck.key ? sigcheck.key : "");
425 argv_array_pushf(&env, "GIT_PUSH_CERT_STATUS=%c", sigcheck.result);
316 -
426 + if (push_cert_nonce) {
427 + argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE=%s", push_cert_nonce);
428 + argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE_STATUS=%s", nonce_status);
429 + }
430 proc->env = env.argv;
431 }
432 }
@@ -1296,6 +1409,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
1409 die("'%s' does not appear to be a git repository", dir);
1410
1411 git_config(receive_pack_config, NULL);
1412 + if (cert_nonce_seed)
1413 + push_cert_nonce = prepare_push_cert_nonce(dir, time(NULL));
1414
1415 if (0 <= transfer_unpack_limit)
1416 unpack_limit = transfer_unpack_limit;
@@ -1340,5 +1455,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
1455 packet_flush(1);
1456 sha1_array_clear(&shallow);
1457 sha1_array_clear(&ref);
1458 + free((void *)push_cert_nonce);
1459 return 0;
1460 }
send-pack.c
+14 -4
@@ -228,7 +228,8 @@ static const char *next_line(const char *line, size_t len)
228 static int generate_push_cert(struct strbuf *req_buf,
229 const struct ref *remote_refs,
230 struct send_pack_args *args,
231 - const char *cap_string)
231 + const char *cap_string,
232 + const char *push_cert_nonce)
233 {
234 const struct ref *ref;
235 char stamp[60];
@@ -245,6 +246,8 @@ static int generate_push_cert(struct strbuf *req_buf,
246 strbuf_addf(&cert, "pushee %s\n", anon_url);
247 free(anon_url);
248 }
249 + if (push_cert_nonce[0])
250 + strbuf_addf(&cert, "nonce %s\n", push_cert_nonce);
251 strbuf_addstr(&cert, "\n");
252
253 for (ref = remote_refs; ref; ref = ref->next) {
@@ -295,6 +298,7 @@ int send_pack(struct send_pack_args *args,
298 unsigned cmds_sent = 0;
299 int ret;
300 struct async demux;
301 + const char *push_cert_nonce = NULL;
302
303 /* Does the other end support the reporting? */
304 if (server_supports("report-status"))
@@ -311,8 +315,14 @@ int send_pack(struct send_pack_args *args,
315 agent_supported = 1;
316 if (server_supports("no-thin"))
317 args->use_thin_pack = 0;
314 - if (args->push_cert && !server_supports("push-cert"))
315 - die(_("the receiving end does not support --signed push"));
318 + if (args->push_cert) {
319 + int len;
320 +
321 + push_cert_nonce = server_feature_value("push-cert", &len);
322 + if (!push_cert_nonce)
323 + die(_("the receiving end does not support --signed push"));
324 + push_cert_nonce = xmemdupz(push_cert_nonce, len);
325 + }
326
327 if (!remote_refs) {
328 fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -343,7 +353,7 @@ int send_pack(struct send_pack_args *args,
353
354 if (!args->dry_run && args->push_cert)
355 cmds_sent = generate_push_cert(&req_buf, remote_refs, args,
346 - cap_buf.buf);
356 + cap_buf.buf, push_cert_nonce);
357
358 /*
359 * Clear the status for each ref and see if we need to send
t/t5534-push-signed.sh
+14 -8
@@ -50,7 +50,6 @@ test_expect_success 'unsigned push does not send push certificate' '
50 test_expect_success 'talking with a receiver without push certificate support' '
51 prepare_dst &&
52 mkdir -p dst/.git/hooks &&
53 - git -C dst config receive.acceptpushcert no &&
53 write_script dst/.git/hooks/post-receive <<-\EOF &&
54 # discard the update list
55 cat >/dev/null
@@ -68,7 +67,6 @@ test_expect_success 'talking with a receiver without push certificate support' '
67 test_expect_success 'push --signed fails with a receiver without push certificate support' '
68 prepare_dst &&
69 mkdir -p dst/.git/hooks &&
71 - git -C dst config receive.acceptpushcert no &&
70 test_must_fail git push --signed dst noop ff +noff 2>err &&
71 test_i18ngrep "the receiving end does not support" err
72 '
@@ -89,6 +87,7 @@ test_expect_success GPG 'no certificate for a signed push with no update' '
87 test_expect_success GPG 'signed push sends push certificate' '
88 prepare_dst &&
89 mkdir -p dst/.git/hooks &&
90 + git -C dst config receive.certnonceseed sekrit &&
91 write_script dst/.git/hooks/post-receive <<-\EOF &&
92 # discard the update list
93 cat >/dev/null
@@ -102,17 +101,24 @@ test_expect_success GPG 'signed push sends push certificate' '
101 SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
102 KEY=${GIT_PUSH_CERT_KEY-nokey}
103 STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
104 + NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
105 + NONCE=${GIT_PUSH_CERT_NONCE-nononce}
106 E_O_F
107
108 EOF
109
109 - cat >expect <<-\EOF &&
110 - SIGNER=C O Mitter <committer@example.com>
111 - KEY=13B6F51ECDDE430D
112 - STATUS=G
113 - EOF
114 -
110 git push --signed dst noop ff +noff &&
111 +
112 + (
113 + cat <<-\EOF &&
114 + SIGNER=C O Mitter <committer@example.com>
115 + KEY=13B6F51ECDDE430D
116 + STATUS=G
117 + NONCE_STATUS=OK
118 + EOF
119 + sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
120 + ) >expect &&
121 +
122 grep "$(git rev-parse noop ff) refs/heads/ff" dst/push-cert &&
123 grep "$(git rev-parse noop noff) refs/heads/noff" dst/push-cert &&
124 test_cmp expect dst/push-cert-status