git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 19/19] signed push: fortify against replay attacks

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Aug 24, 2014, 03:29 UTC
Message-ID
<CAPig+cR=V+1VhHDU5pd0W-9oSwzULZsVPybGRz7WXrmKrO4+xA@mail.gmail.com>
In-Reply-To
<1408739424-31429-20-git-send-email-gitster@pobox.com>
On Fri, Aug 22, 2014 at 4:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> In order to prevent a valid push certificate for pushing into an
> repository from getting replayed to push to an unrelated one, send a
> nonce string from the receive-pack process and have the signer
> include it in the push certificate.  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.
>
> Because the built-in nonce generation may not be suitable for all
> situations, allow the server to invoke receive-pack with pregenerated
> nonce from the command line argument.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 991e417..8ad4d9b 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1226,12 +1232,28 @@ static int delete_only(struct command *commands)
>         return 1;
>  }
>
> +static char *prepare_push_cert_nonce(const char *sitename, const char *dir)
> +{
> +       struct strbuf buf = STRBUF_INIT;
> +       unsigned char sha1[20];
> +
> +       if (!sitename) {
> +               static char buf[1024];
Potentially confusing 'buf' shadows 'buf' in outer scope.
Show 5 quoted lines
> +               gethostname(buf, sizeof(buf));
> +               sitename = buf;
> +       }
> +       strbuf_addf(&buf, "%s:%s:%lu", sitename, dir, time(NULL));
> +       hash_sha1_file(buf.buf, buf.len, "blob", sha1);
strbuf_release(&buf);
Show 141 quoted lines
> +       return xstrdup(sha1_to_hex(sha1));
> +}
> +
>  int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>  {
>         int advertise_refs = 0;
>         int stateless_rpc = 0;
>         int i;
>         const char *dir = NULL;
> +       const char *sitename = NULL;
>         struct command *commands;
>         struct sha1_array shallow = SHA1_ARRAY_INIT;
>         struct sha1_array ref = SHA1_ARRAY_INIT;
> @@ -1261,6 +1283,13 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>                                 fix_thin = 0;
>                                 continue;
>                         }
> +                       if (skip_prefix(arg, "--sitename=", &sitename)) {
> +                               continue;
> +                       }
> +                       if (skip_prefix(arg, "--push-cert-nonce=", &push_cert_nonce)) {
> +                               push_cert_nonce = xstrdup(push_cert_nonce);
> +                               continue;
> +                       }
>
>                         usage(receive_pack_usage);
>                 }
> @@ -1277,6 +1306,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>                 die("'%s' does not appear to be a git repository", dir);
>
>         git_config(receive_pack_config, NULL);
> +       if (!push_cert_nonce)
> +               push_cert_nonce = prepare_push_cert_nonce(sitename, dir);
>
>         if (0 <= transfer_unpack_limit)
>                 unpack_limit = transfer_unpack_limit;
> @@ -1321,5 +1352,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>                 packet_flush(1);
>         sha1_array_clear(&shallow);
>         sha1_array_clear(&ref);
> +       free((void *)push_cert_nonce);
>         return 0;
>  }
> diff --git a/send-pack.c b/send-pack.c
> index 61f321d..349393a 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -228,7 +228,8 @@ static const char *next_line(const char *line, size_t len)
>  static int generate_push_cert(struct strbuf *req_buf,
>                               const struct ref *remote_refs,
>                               struct send_pack_args *args,
> -                             const char *cap_string)
> +                             const char *cap_string,
> +                             const char *push_cert_nonce)
>  {
>         const struct ref *ref;
>         char stamp[60];
> @@ -240,6 +241,8 @@ static int generate_push_cert(struct strbuf *req_buf,
>         datestamp(stamp, sizeof(stamp));
>         strbuf_addf(&cert, "certificate version 0.1\n");
>         strbuf_addf(&cert, "pusher %s %s\n", signing_key, stamp);
> +       if (push_cert_nonce[0])
> +               strbuf_addf(&cert, "nonce %s\n", push_cert_nonce);
>         strbuf_addstr(&cert, "\n");
>
>         for (ref = remote_refs; ref; ref = ref->next) {
> @@ -290,6 +293,8 @@ int send_pack(struct send_pack_args *args,
>         unsigned cmds_sent = 0;
>         int ret;
>         struct async demux;
> +       const char *push_cert_nonce = NULL;
> +
>
>         /* Does the other end support the reporting? */
>         if (server_supports("report-status"))
> @@ -306,8 +311,14 @@ int send_pack(struct send_pack_args *args,
>                 agent_supported = 1;
>         if (server_supports("no-thin"))
>                 args->use_thin_pack = 0;
> -       if (args->push_cert && !server_supports("push-cert"))
> -               die(_("the receiving end does not support --signed push"));
> +       if (args->push_cert) {
> +               int len;
> +
> +               push_cert_nonce = server_feature_value("push-cert", &len);
> +               if (!push_cert_nonce)
> +                       die(_("the receiving end does not support --signed push"));
> +               push_cert_nonce = xmemdupz(push_cert_nonce, len);
> +       }
>
>         if (!remote_refs) {
>                 fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
> @@ -338,7 +349,7 @@ int send_pack(struct send_pack_args *args,
>
>         if (!args->dry_run && args->push_cert)
>                 cmds_sent = generate_push_cert(&req_buf, remote_refs, args,
> -                                              cap_buf.buf);
> +                                              cap_buf.buf, push_cert_nonce);
>
>         /*
>          * Clear the status for each ref and see if we need to send
> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> index 659bca0..6db59ce 100755
> --- a/t/t5534-push-signed.sh
> +++ b/t/t5534-push-signed.sh
> @@ -58,17 +58,22 @@ test_expect_success GPG 'signed push sends push certificate' '
>         SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
>         KEY=${GIT_PUSH_CERT_KEY-nokey}
>         STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
> +       NONCE=${GIT_PUSH_CERT_NONCE-nononce}
>         E_O_F
>
>         EOF
>
> -       cat >expect <<-\EOF &&
> -       SIGNER=C O Mitter <committer@example.com>
> -       KEY=13B6F51ECDDE430D
> -       STATUS=G
> -       EOF
> -
>         git push --signed dst noop ff +noff &&
> +
> +       (
> +               cat <<-\EOF &&
> +               SIGNER=C O Mitter <committer@example.com>
> +               KEY=13B6F51ECDDE430D
> +               STATUS=G
> +               EOF
> +               sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
> +       ) >expect &&
> +
>         grep "$(git rev-parse noop ff) refs/heads/ff" dst/push-cert &&
>         grep "$(git rev-parse noop noff) refs/heads/noff" dst/push-cert &&
>         test_cmp expect dst/push-cert-status
> --
> 2.1.0-304-g950f846
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous: Junio C HamanoNext: Stefan Beller
Message 21 of 23 in “Signed push”
  1. 00/19 Signed pushJunio C Hamano, Aug 22, 2014
  2. 01/19 receive-pack: do not overallocate command structureJunio C Hamano, Aug 22, 2014
  3. 02/19 receive-pack: parse feature request a bit earlierJunio C Hamano, Aug 22, 2014
  4. 03/19 receive-pack: do not reuse old_sha1[] for other thingsJunio C Hamano, Aug 22, 2014
  5. 04/19 receive-pack: factor out queueing of commandJunio C Hamano, Aug 22, 2014
  6. 05/19 send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higherJunio C Hamano, Aug 22, 2014
  7. 06/19 send-pack: refactor decision to send update per refJunio C Hamano, Aug 22, 2014
  8. 07/19 send-pack: always send capabilitiesJunio C Hamano, Aug 22, 2014
  9. 08/19 send-pack: factor out capability string generationJunio C Hamano, Aug 22, 2014
  10. 09/19 send-pack: rename "new_refs" to "need_pack_data"Junio C Hamano, Aug 22, 2014
  11. 10/19 send-pack: refactor inspecting and resetting status and sending commandsJunio C Hamano, Aug 22, 2014
  12. 11/19 send-pack: clarify that cmds_sent is a booleanJunio C Hamano, Aug 22, 2014
  13. 12/19 gpg-interface: move parse_gpg_output() to where it should beJunio C Hamano, Aug 22, 2014
  14. 13/19 gpg-interface: move parse_signature() to where it should beJunio C Hamano, Aug 22, 2014
  15. 14/19 pack-protocol doc: typofix for PKT-LINEJunio C Hamano, Aug 22, 2014
  16. 15/19 the beginning of the signed pushJunio C Hamano, Aug 22, 2014
  17. 16/19 receive-pack: GPG-validate push certificatesJunio C Hamano, Aug 22, 2014
  18. 17/19 send-pack: send feature request on push-cert packetJunio C Hamano, Aug 22, 2014
  19. 18/19 signed push: remove duplicated protocol infoJunio C Hamano, Aug 22, 2014
  20. 19/19 signed push: fortify against replay attacksJunio C Hamano, Aug 22, 2014
  21. Eric SunshineAug 24, 2014
  22. Stefan BellerAug 30, 2014
  23. Junio C HamanoSep 2, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.