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

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

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];

+       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