usign-exec: improve usign -F output handling

While not likely to happen in pratice, nothing guarantees that read()
will retrieve more than 1 byte at a time. The easiest way to make this
code compliant is to wrap the file descriptor using fdopen().

While we're at it, also
- remove useless memset()
- check fingerprint for validity

The check is particularly relevant, as a usign bug [1] causing short
fingerprint outputs only went unnoticed for so long because the trailing
newline was considered one of the 16 characters ucert was expecting.

[1] https://patchwork.ozlabs.org/project/openwrt/patch/8ead1fd6a61117b54b4efd5111fe0d19e4eef9c5.1589642591.git.mschiffer@universe-factory.net/

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>

1 files changed, 15 insertions, 9 deletions

diff --git a/usign-exec.c b/usign-exec.c
index 241d630..482e510 100644
--- a/usign-exec.c
+++ b/usign-exec.c
@@ -93,6 +93,7 @@ int usign_s(const char *msgfile, const char *seckeyfile, const char *sigfile, bo
  */
 static int usign_f(char fingerprint[17], const char *pubkeyfile, const char *seckeyfile, const char *sigfile, bool quiet) {
 	int fds[2];
+	FILE *f;
 	pid_t pid;
 	int status;
 	const char *usign_argv[16] = {0};
@@ -141,17 +142,22 @@ static int usign_f(char fingerprint[17], const char *pubkeyfile, const char *sec
 	waitpid(pid, &status, 0);
 	status = WIFEXITED(status) ? WEXITSTATUS(status) : -1;
 
-	if (fingerprint && !status) {
-		ssize_t r;
-		memset(fingerprint, 0, 17);
-		r = read(fds[0], fingerprint, 17);
-		if (r < 16)
-			status = -1;
+	if (!fingerprint || status) {
+		close(fds[0]);
+		return status;
+	}
 
-		fingerprint[16] = '\0';
+	f = fdopen(fds[0], "r");
+	if (fread(fingerprint, 1, 16, f) != 16)
+		status = -1;
+	fclose(f);
+	if (status)
+		return status;
+
+	fingerprint[16] = '\0';
+	if (strspn(fingerprint, "0123456789abcdefABCDEF") != 16)
+		status = -1;
 
-	}
-	close(fds[0]);
 	return status;
 }