Merge branch 'jc/push-cert'
authorJunio C Hamano <gitster@pobox.com>
Mon, 20 Apr 2015 22:28:31 +0000 (15:28 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 20 Apr 2015 22:28:31 +0000 (15:28 -0700)
The "git push --signed" protocol extension did not limit what the
"nonce" that is a server-chosen string can contain or how long it
can be, which was unnecessarily lax. Limit both the length and the
alphabet to a reasonably small space that can still have enough
entropy.

* jc/push-cert:
push --signed: tighten what the receiving end can ask to sign

1  2 
send-pack.c
diff --combined send-pack.c
index 189bdde0c29b1aa62c9628786fa7b8c1b4083fb3,22498080271a5702a7e79a72af1f1f8491d15121..2e07ac3339bce870b12e0023dc985929a277ebef
@@@ -47,9 -47,8 +47,9 @@@ static int pack_objects(int fd, struct 
                NULL,
                NULL,
                NULL,
 +              NULL,
        };
 -      struct child_process po;
 +      struct child_process po = CHILD_PROCESS_INIT;
        int i;
  
        i = 4;
@@@ -61,8 -60,7 +61,8 @@@
                argv[i++] = "-q";
        if (args->progress)
                argv[i++] = "--progress";
 -      memset(&po, 0, sizeof(po));
 +      if (is_repository_shallow())
 +              argv[i++] = "--shallow";
        po.argv = argv;
        po.in = -1;
        po.out = args->stateless_rpc ? -1 : fd;
@@@ -193,13 -191,10 +193,13 @@@ static void advertise_shallow_grafts_bu
        for_each_commit_graft(advertise_shallow_grafts_cb, sb);
  }
  
 -static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
 +#define CHECK_REF_NO_PUSH -1
 +#define CHECK_REF_STATUS_REJECTED -2
 +#define CHECK_REF_UPTODATE -3
 +static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args)
  {
        if (!ref->peer_ref && !args->send_mirror)
 -              return 0;
 +              return CHECK_REF_NO_PUSH;
  
        /* Check for statuses set by set_ref_status_for_push() */
        switch (ref->status) {
        case REF_STATUS_REJECT_NEEDS_FORCE:
        case REF_STATUS_REJECT_STALE:
        case REF_STATUS_REJECT_NODELETE:
 +              return CHECK_REF_STATUS_REJECTED;
        case REF_STATUS_UPTODATE:
 -              return 0;
 +              return CHECK_REF_UPTODATE;
        default:
 -              return 1;
 +              return 0;
        }
  }
  
@@@ -238,15 -232,15 +238,15 @@@ static int generate_push_cert(struct st
                              const char *push_cert_nonce)
  {
        const struct ref *ref;
 -      char stamp[60];
        char *signing_key = xstrdup(get_signing_key());
        const char *cp, *np;
        struct strbuf cert = STRBUF_INIT;
        int update_seen = 0;
  
 -      datestamp(stamp, sizeof(stamp));
        strbuf_addf(&cert, "certificate version 0.1\n");
 -      strbuf_addf(&cert, "pusher %s %s\n", signing_key, stamp);
 +      strbuf_addf(&cert, "pusher %s ", signing_key);
 +      datestamp(&cert);
 +      strbuf_addch(&cert, '\n');
        if (args->url && *args->url) {
                char *anon_url = transport_anonymize_url(args->url);
                strbuf_addf(&cert, "pushee %s\n", anon_url);
        strbuf_addstr(&cert, "\n");
  
        for (ref = remote_refs; ref; ref = ref->next) {
 -              if (!ref_update_to_be_sent(ref, args))
 +              if (check_to_send_update(ref, args) < 0)
                        continue;
                update_seen = 1;
                strbuf_addf(&cert, "%s %s %s\n",
@@@ -285,29 -279,28 +285,51 @@@ free_return
        return update_seen;
  }
  
 +
 +static int atomic_push_failure(struct send_pack_args *args,
 +                             struct ref *remote_refs,
 +                             struct ref *failing_ref)
 +{
 +      struct ref *ref;
 +      /* Mark other refs as failed */
 +      for (ref = remote_refs; ref; ref = ref->next) {
 +              if (!ref->peer_ref && !args->send_mirror)
 +                      continue;
 +
 +              switch (ref->status) {
 +              case REF_STATUS_EXPECTING_REPORT:
 +                      ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
 +                      continue;
 +              default:
 +                      break; /* do nothing */
 +              }
 +      }
 +      return error("atomic push failed for ref %s. status: %d\n",
 +                   failing_ref->name, failing_ref->status);
 +}
 +
+ #define NONCE_LEN_LIMIT 256
+ static void reject_invalid_nonce(const char *nonce, int len)
+ {
+       int i = 0;
+       if (NONCE_LEN_LIMIT <= len)
+               die("the receiving end asked to sign an invalid nonce <%.*s>",
+                   len, nonce);
+       for (i = 0; i < len; i++) {
+               int ch = nonce[i] & 0xFF;
+               if (isalnum(ch) ||
+                   ch == '-' || ch == '.' ||
+                   ch == '/' || ch == '+' ||
+                   ch == '=' || ch == '_')
+                       continue;
+               die("the receiving end asked to sign an invalid nonce <%.*s>",
+                   len, nonce);
+       }
+ }
  int send_pack(struct send_pack_args *args,
              int fd[], struct child_process *conn,
              struct ref *remote_refs,
        int use_sideband = 0;
        int quiet_supported = 0;
        int agent_supported = 0;
 +      int use_atomic = 0;
 +      int atomic_supported = 0;
        unsigned cmds_sent = 0;
        int ret;
        struct async demux;
                agent_supported = 1;
        if (server_supports("no-thin"))
                args->use_thin_pack = 0;
 +      if (server_supports("atomic"))
 +              atomic_supported = 1;
        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"));
+               reject_invalid_nonce(push_cert_nonce, len);
                push_cert_nonce = xmemdupz(push_cert_nonce, len);
        }
  
                        "Perhaps you should specify a branch such as 'master'.\n");
                return 0;
        }
 +      if (args->atomic && !atomic_supported)
 +              die(_("the receiving end does not support --atomic push"));
 +
 +      use_atomic = atomic_supported && args->atomic;
  
        if (status_report)
                strbuf_addstr(&cap_buf, " report-status");
                strbuf_addstr(&cap_buf, " side-band-64k");
        if (quiet_supported && (args->quiet || !args->progress))
                strbuf_addstr(&cap_buf, " quiet");
 +      if (use_atomic)
 +              strbuf_addstr(&cap_buf, " atomic");
        if (agent_supported)
                strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized());
  
         * the pack data.
         */
        for (ref = remote_refs; ref; ref = ref->next) {
 -              if (!ref_update_to_be_sent(ref, args))
 +              switch (check_to_send_update(ref, args)) {
 +              case 0: /* no error */
 +                      break;
 +              case CHECK_REF_STATUS_REJECTED:
 +                      /*
 +                       * When we know the server would reject a ref update if
 +                       * we were to send it and we're trying to send the refs
 +                       * atomically, abort the whole operation.
 +                       */
 +                      if (use_atomic)
 +                              return atomic_push_failure(args, remote_refs, ref);
 +                      /* Fallthrough for non atomic case. */
 +              default:
                        continue;
 -
 +              }
                if (!ref->deletion)
                        need_pack_data = 1;
  
                if (args->dry_run || args->push_cert)
                        continue;
  
 -              if (!ref_update_to_be_sent(ref, args))
 +              if (check_to_send_update(ref, args) < 0)
                        continue;
  
                old_hex = sha1_to_hex(ref->old_sha1);