Skip to content

Commit 0ea47f9

Browse files
committed
signed push: teach smart-HTTP to pass "git push --signed" around
The "--signed" option received by "git push" is first passed to the transport layer, which the native transport directly uses to notice that a push certificate needs to be sent. When the transport-helper is involved, however, the option needs to be told to the helper with set_helper_option(), and the helper needs to take necessary action. For the smart-HTTP helper, the "necessary action" involves spawning the "git send-pack" subprocess with the "--signed" option. Once the above all gets wired in, the smart-HTTP transport now can use the push certificate mechanism to authenticate its pushes. Add a test that is modeled after tests for the native transport in t5534-push-signed.sh to t5541-http-push-smart.sh. Update the test Apache configuration to pass GNUPGHOME environment variable through. As PassEnv would trigger warnings for an environment variable that is not set, export it from test-lib.sh set to a harmless value when GnuPG is not being used in the tests. Note that the added test is deliberately loose and does not check the nonce in this step. This is because the stateless RPC mode is inevitably flaky and a nonce that comes back in the actual push processing is one issued by a different process; if the two interactions with the server crossed a second boundary, the nonces will not match and such a check will fail. A later patch in the series will work around this shortcoming. Signed-off-by: Junio C Hamano <[email protected]>
1 parent b89363e commit 0ea47f9

File tree

6 files changed

+63
-3
lines changed

6 files changed

+63
-3
lines changed

builtin/send-pack.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
153153
args.verbose = 1;
154154
continue;
155155
}
156+
if (!strcmp(arg, "--signed")) {
157+
args.push_cert = 1;
158+
continue;
159+
}
156160
if (!strcmp(arg, "--progress")) {
157161
progress = 1;
158162
continue;

remote-curl.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ struct options {
2525
update_shallow : 1,
2626
followtags : 1,
2727
dry_run : 1,
28-
thin : 1;
28+
thin : 1,
29+
push_cert : 1;
2930
};
3031
static struct options options;
3132
static struct string_list cas_options = STRING_LIST_INIT_DUP;
@@ -106,6 +107,14 @@ static int set_option(const char *name, const char *value)
106107
else
107108
return -1;
108109
return 0;
110+
} else if (!strcmp(name, "pushcert")) {
111+
if (!strcmp(value, "true"))
112+
options.push_cert = 1;
113+
else if (!strcmp(value, "false"))
114+
options.push_cert = 0;
115+
else
116+
return -1;
117+
return 0;
109118
} else {
110119
return 1 /* unsupported */;
111120
}
@@ -872,6 +881,8 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
872881
argv_array_push(&args, "--thin");
873882
if (options.dry_run)
874883
argv_array_push(&args, "--dry-run");
884+
if (options.push_cert)
885+
argv_array_push(&args, "--signed");
875886
if (options.verbosity == 0)
876887
argv_array_push(&args, "--quiet");
877888
else if (options.verbosity > 1)

t/lib-httpd/apache.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ LockFile accept.lock
6868

6969
PassEnv GIT_VALGRIND
7070
PassEnv GIT_VALGRIND_OPTIONS
71+
PassEnv GNUPGHOME
7172

7273
Alias /dumb/ www/
7374
Alias /auth/dumb/ www/auth/dumb/

t/t5541-http-push-smart.sh

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ if test -n "$NO_CURL"; then
1212
fi
1313

1414
ROOT_PATH="$PWD"
15+
. "$TEST_DIRECTORY"/lib-gpg.sh
1516
. "$TEST_DIRECTORY"/lib-httpd.sh
1617
. "$TEST_DIRECTORY"/lib-terminal.sh
1718
start_httpd
@@ -323,5 +324,40 @@ test_expect_success 'push into half-auth-complete requires password' '
323324
test_cmp expect actual
324325
'
325326

327+
test_expect_success GPG 'push with post-receive to inspect certificate' '
328+
(
329+
cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
330+
mkdir -p hooks &&
331+
write_script hooks/post-receive <<-\EOF &&
332+
# discard the update list
333+
cat >/dev/null
334+
# record the push certificate
335+
if test -n "${GIT_PUSH_CERT-}"
336+
then
337+
git cat-file blob $GIT_PUSH_CERT >../push-cert
338+
fi &&
339+
cat >../push-cert-status <<E_O_F
340+
SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
341+
KEY=${GIT_PUSH_CERT_KEY-nokey}
342+
STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
343+
E_O_F
344+
EOF
345+
346+
git config receive.certnonceseed sekrit
347+
) &&
348+
cd "$ROOT_PATH/test_repo_clone" &&
349+
test_commit cert-test &&
350+
git push --signed "$HTTPD_URL/smart/test_repo.git" &&
351+
(
352+
cd "$HTTPD_DOCUMENT_ROOT_PATH" &&
353+
cat <<-\EOF
354+
SIGNER=C O Mitter <[email protected]>
355+
KEY=13B6F51ECDDE430D
356+
STATUS=G
357+
EOF
358+
) >expect &&
359+
test_cmp expect "$HTTPD_DOCUMENT_ROOT_PATH/push-cert-status"
360+
'
361+
326362
stop_httpd
327363
test_done

t/test-lib.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,8 @@ rm -fr "$TRASH_DIRECTORY" || {
813813
}
814814

815815
HOME="$TRASH_DIRECTORY"
816-
export HOME
816+
GNUPGHOME="$HOME/gnupg-home-not-used"
817+
export HOME GNUPGHOME
817818

818819
if test -z "$TEST_NO_CREATE_REPO"
819820
then

transport-helper.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,8 @@ static const char *unsupported_options[] = {
259259
static const char *boolean_options[] = {
260260
TRANS_OPT_THIN,
261261
TRANS_OPT_KEEP,
262-
TRANS_OPT_FOLLOWTAGS
262+
TRANS_OPT_FOLLOWTAGS,
263+
TRANS_OPT_PUSH_CERT
263264
};
264265

265266
static int set_helper_option(struct transport *transport,
@@ -835,6 +836,9 @@ static int push_refs_with_push(struct transport *transport,
835836
if (flags & TRANSPORT_PUSH_DRY_RUN) {
836837
if (set_helper_option(transport, "dry-run", "true") != 0)
837838
die("helper %s does not support dry-run", data->name);
839+
} else if (flags & TRANSPORT_PUSH_CERT) {
840+
if (set_helper_option(transport, TRANS_OPT_PUSH_CERT, "true") != 0)
841+
die("helper %s does not support --signed", data->name);
838842
}
839843

840844
strbuf_addch(&buf, '\n');
@@ -859,6 +863,9 @@ static int push_refs_with_export(struct transport *transport,
859863
if (flags & TRANSPORT_PUSH_DRY_RUN) {
860864
if (set_helper_option(transport, "dry-run", "true") != 0)
861865
die("helper %s does not support dry-run", data->name);
866+
} else if (flags & TRANSPORT_PUSH_CERT) {
867+
if (set_helper_option(transport, TRANS_OPT_PUSH_CERT, "true") != 0)
868+
die("helper %s does not support dry-run", data->name);
862869
}
863870

864871
if (flags & TRANSPORT_PUSH_FORCE) {

0 commit comments

Comments
 (0)