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 <gits...@pobox.com>
---
 Documentation/git-receive-pack.txt                |  8 +++++
 Documentation/technical/pack-protocol.txt         |  5 +--
 Documentation/technical/protocol-capabilities.txt |  7 ++--
 builtin/receive-pack.c                            | 42 ++++++++++++++++++++---
 send-pack.c                                       | 19 +++++++---
 t/t5534-push-signed.sh                            | 17 +++++----
 6 files changed, 78 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index 60151a6..983b24e 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -72,6 +72,13 @@ GIT_PUSH_CERT_STATUS::
        using the same mnemonic as used in `%G?` format of `git log`
        family of commands (see linkgit:git-log[1]).
 
+GIT_PUSH_CERT_NONCE::
+       The nonce string the process asked the signer to include
+       in the push certificate.  If this does not match the value
+       recorded on the "nonce" header in the push certificate, it
+       may indicate that the certificate is a valid one that is
+       being replayed from a separate "git push" session.
+
 This hook is called before any refname is updated and before any
 fast-forward checks are performed.
 
@@ -147,6 +154,7 @@ service:
        if test -n "${GIT_PUSH_CERT-}" && test ${GIT_PUSH_CERT_STATUS} = G
        then
                (
+                       echo expected nonce is ${GIT_PUSH_NONCE}
                        git cat-file blob ${GIT_PUSH_CERT}
                ) | mail -s "push from $GIT_PUSH_CERT_SIGNER" push-log@mydomain
        fi
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index b86580b..67dd3c9 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -483,10 +483,11 @@ references.
 
   push-cert         = PKT-LINE("push-cert" NUL capability-list LF)
                      PKT-LINE("certificate version 0.1" LF)
-                     PKT-LINE("pusher" ident LF)
+                     PKT-LINE("pusher" SP ident LF)
+                     PKT-LINE("nonce" SP nonce LF)
                      PKT-LINE(LF)
                      *PKT-LINE(command LF)
-                     *PKT-LINE(GPG signature lines LF)
+                     *PKT-LINE(detached GPG signature lines LF)
                      PKT-LINE("push-cert-end" LF)
 
   pack-file         = "PACK" 28*(OCTET)
diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index a478cc4..0c92dee 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -251,10 +251,11 @@ If the upload-pack server advertises this capability, 
fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
-push-cert
----------
+push-cert=<nonce>
+-----------------
 
 The receive-pack server that advertises this capability is willing
-to accept a signed push certificate.  A send-pack client MUST NOT
+to accept a signed push certificate, and asks the <nonce> to be
+included in the push certificate.  A send-pack client MUST NOT
 send a push-cert packet unless the receive-pack server advertises
 this capability.
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
@@ -51,6 +51,7 @@ static const char *alt_shallow_file;
 static struct strbuf push_cert = STRBUF_INIT;
 static unsigned char push_cert_sha1[20];
 static struct signature_check sigcheck;
+static const char *push_cert_nonce;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -142,15 +143,18 @@ static void show_ref(const char *path, const unsigned 
char *sha1)
        if (ref_is_hidden(path))
                return;
 
-       if (sent_capabilities)
+       if (sent_capabilities) {
                packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
-       else
-               packet_write(1, "%s %s%c%s%s agent=%s\n",
+       } else {
+               packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
                             sha1_to_hex(sha1), path, 0,
-                            " report-status delete-refs side-band-64k quiet 
push-cert",
+                            " report-status delete-refs side-band-64k quiet",
                             prefer_ofs_delta ? " ofs-delta" : "",
+                            push_cert_nonce ? " push-cert=" : "",
+                            push_cert_nonce ? push_cert_nonce : "",
                             git_user_agent_sanitized());
-       sent_capabilities = 1;
+               sent_capabilities = 1;
+       }
 }
 
 static int show_ref_cb(const char *path, const unsigned char *sha1, int flag, 
void *unused)
@@ -294,6 +298,8 @@ static void prepare_push_cert_sha1(struct child_process 
*proc)
                argv_array_pushf(&env, "GIT_PUSH_CERT_SIGNER=%s", 
sigcheck.signer);
                argv_array_pushf(&env, "GIT_PUSH_CERT_KEY=%s", sigcheck.key);
                argv_array_pushf(&env, "GIT_PUSH_CERT_STATUS=%c", 
sigcheck.result);
+               if (push_cert_nonce)
+                       argv_array_pushf(&env, "GIT_PUSH_CERT_NONCE=%s", 
push_cert_nonce);
 
                proc->env = env.argv;
        }
@@ -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];
+               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);
+       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 <commit...@example.com>
-       KEY=13B6F51ECDDE430D
-       STATUS=G
-       EOF
-
        git push --signed dst noop ff +noff &&
+
+       (
+               cat <<-\EOF &&
+               SIGNER=C O Mitter <commit...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to