[PATCH v3] ssh signing: don't detach the filename strbuf from key_file tempfile

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Detaching the filename string from the tempfile structure used to cause
delete_tempfile() to fail and the temporary file was not cleaned up.

While it's possible to get rid of the allocation and copy from
xstrdup(), it keeps the code symetric with the other branch since
interpolate_path() also allocates and ssh_signing_key_file is freed
in both cases.

The exisiting test was updated to check if the temporary files are
properly deleted. To prevent TMPDIR from leaking into the other tests, a
new subshell is created, however this prevents test_config from working.
The cleanup of the config changed in the subshell is done by
test_unconfig in a call to test_when_finished outside of it.

Helped-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
Helped-by: Patrick Steinhardt <ps@xxxxxx>
Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
Signed-off-by: redoste <redoste@xxxxxxxxxxx>
---
v1->v2:
 * add a test case that checks for temporary files after signing commits
 * add small explaination about the use of xstrdup() in the commit
   body
v2->v3:
 * merge the test with the previous ssh-agent test
 * export TMPDIR in a subshell to prevent the environment variable from
   leaking in the next tests
 * use test_must_be_empty instead of test_line_count

 gpg-interface.c              |  2 +-
 t/t7528-signed-commit-ssh.sh | 32 ++++++++++++++++++++------------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 0896458de5..bdcc8c2a2e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1048,7 +1048,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 				    key_file->filename.buf);
 			goto out;
 		}
-		ssh_signing_key_file = strbuf_detach(&key_file->filename, NULL);
+		ssh_signing_key_file = xstrdup(key_file->filename.buf);
 	} else {
 		/* We assume a file */
 		ssh_signing_key_file = interpolate_path(signing_key, 1);
diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
index 065f780636..0f887a3ebe 100755
--- a/t/t7528-signed-commit-ssh.sh
+++ b/t/t7528-signed-commit-ssh.sh
@@ -84,18 +84,26 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen
 	test_config gpg.format ssh &&
 	eval $(ssh-agent) &&
 	test_when_finished "kill ${SSH_AGENT_PID}" &&
-	ssh-add "${GPGSSH_KEY_PRIMARY}" &&
-	echo 1 >file && git add file &&
-	git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
-	echo 2 >file &&
-	test_config user.signingkey "$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
-	git commit -a -m rsa-config -S &&
-	ssh-add "${GPGSSH_KEY_ECDSA}" &&
-	echo 3 >file &&
-	git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
-	echo 4 >file &&
-	test_config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
-	git commit -a -m ecdsa-config -S
+	test_when_finished "test_unconfig user.signingkey" &&
+	mkdir tmpdir &&
+	TMPDIR="$(pwd)/tmpdir" &&
+	(
+		export TMPDIR &&
+		ssh-add "${GPGSSH_KEY_PRIMARY}" &&
+		echo 1 >file && git add file &&
+		git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
+		echo 2 >file &&
+		git config user.signingkey "$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
+		git commit -a -m rsa-config -S &&
+		ssh-add "${GPGSSH_KEY_ECDSA}" &&
+		echo 3 >file &&
+		git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
+		echo 4 >file &&
+		git config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
+		git commit -a -m ecdsa-config -S
+	) &&
+	find tmpdir -type f >tmpfiles &&
+	test_must_be_empty tmpfiles
 '
 
 test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '
-- 
2.49.0





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux