[PATCH js/daemon-log] receive-pack: do not send error details to the client

[Johannes Sixt <j6t@kdbg.org>]

If the objects that a client pushes to the server cannot be processed for
any reason, an error is reported back to the client via the git protocol.
We used to send quite detailed information if a system call failed if
unpack-objects is run. This can be regarded as an information leak. Now we
do not send any error details like we already do in the case where
index-pack failed.

Errors in system calls as well as the exit code of unpack-objects and
index-pack are now reported to stderr; in the case of a local push or via
ssh these messages still go to the client, but that is OK since these forms
of access to the server assume that the client can be trusted. If
receive-pack is run from git-daemon, then the daemon should put the error
messages into the syslog.

With this reasoning a new status report is added for the post-update-hook;
untrusted (i.e. daemon's) clients cannot observe its status anyway, others
may want to know failure details.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Does this make sense?

 The motivation of this change is to ultimately get rid of error codes
 for system call failures in start/finish/run_command functions and call
 error() in these functions.

 -- Hannes

 builtin-receive-pack.c |   53 +++++++++++++++++++----------------------------
 1 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 33d345d..6ec1d05 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -123,27 +123,27 @@ static struct command *commands;
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
-static int hook_status(int code, const char *hook_name)
+static int run_status(int code, const char *cmd_name)
 {
 	switch (code) {
 	case 0:
 		return 0;
 	case -ERR_RUN_COMMAND_FORK:
-		return error("hook fork failed");
+		return error("fork of %s failed", cmd_name);
 	case -ERR_RUN_COMMAND_EXEC:
-		return error("hook execute failed");
+		return error("execute of %s failed", cmd_name);
 	case -ERR_RUN_COMMAND_PIPE:
-		return error("hook pipe failed");
+		return error("pipe failed");
 	case -ERR_RUN_COMMAND_WAITPID:
 		return error("waitpid failed");
 	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
 		return error("waitpid is confused");
 	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-		return error("%s died of signal", hook_name);
+		return error("%s died of signal", cmd_name);
 	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
-		return error("%s died strangely", hook_name);
+		return error("%s died strangely", cmd_name);
 	default:
-		error("%s exited with error code %d", hook_name, -code);
+		error("%s exited with error code %d", cmd_name, -code);
 		return -code;
 	}
 }
@@ -174,7 +174,7 @@ static int run_receive_hook(const char *hook_name)
 
 	code = start_command(&proc);
 	if (code)
-		return hook_status(code, hook_name);
+		return run_status(code, hook_name);
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string) {
 			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
@@ -186,7 +186,7 @@ static int run_receive_hook(const char *hook_name)
 		}
 	}
 	close(proc.in);
-	return hook_status(finish_command(&proc), hook_name);
+	return run_status(finish_command(&proc), hook_name);
 }
 
 static int run_update_hook(struct command *cmd)
@@ -203,7 +203,7 @@ static int run_update_hook(struct command *cmd)
 	argv[3] = sha1_to_hex(cmd->new_sha1);
 	argv[4] = NULL;
 
-	return hook_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |
+	return run_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |
 					RUN_COMMAND_STDOUT_TO_STDERR),
 			update_hook);
 }
@@ -394,7 +394,7 @@ static char update_post_hook[] = "hooks/post-update";
 static void run_update_post_hook(struct command *cmd)
 {
 	struct command *cmd_p;
-	int argc;
+	int argc, status;
 	const char **argv;
 
 	for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
@@ -417,8 +417,9 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
-		| RUN_COMMAND_STDOUT_TO_STDERR);
+	status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
+			| RUN_COMMAND_STDOUT_TO_STDERR);
+	run_status(status, update_post_hook);
 }
 
 static void execute_commands(const char *unpacker_error)
@@ -534,24 +535,10 @@ static const char *unpack(void)
 		unpacker[i++] = hdr_arg;
 		unpacker[i++] = NULL;
 		code = run_command_v_opt(unpacker, RUN_GIT_CMD);
-		switch (code) {
-		case 0:
+		if (!code)
 			return NULL;
-		case -ERR_RUN_COMMAND_FORK:
-			return "unpack fork failed";
-		case -ERR_RUN_COMMAND_EXEC:
-			return "unpack execute failed";
-		case -ERR_RUN_COMMAND_WAITPID:
-			return "waitpid failed";
-		case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
-			return "waitpid is confused";
-		case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-			return "unpacker died of signal";
-		case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
-			return "unpacker died strangely";
-		default:
-			return "unpacker exited with error code";
-		}
+		run_status(code, unpacker[0]);
+		return "unpack-objects abnormal exit";
 	} else {
 		const char *keeper[7];
 		int s, status, i = 0;
@@ -574,8 +561,11 @@ static const char *unpack(void)
 		ip.argv = keeper;
 		ip.out = -1;
 		ip.git_cmd = 1;
-		if (start_command(&ip))
+		status = start_command(&ip);
+		if (status) {
+			run_status(status, keeper[0]);
 			return "index-pack fork failed";
+		}
 		pack_lockfile = index_pack_lockfile(ip.out);
 		close(ip.out);
 		status = finish_command(&ip);
@@ -583,6 +573,7 @@ static const char *unpack(void)
 			reprepare_packed_git();
 			return NULL;
 		}
+		run_status(status, keeper[0]);
 		return "index-pack abnormal exit";
 	}
 }
-- 
1.6.3.17.g1665f