Attachment 3009 Details for Bug 2561 – fix double-fclose

[patch] fix double-fclose

bz2561.diff (text/plain), 4.99 KB, created by Damien Miller on 2017-07-07 13:35:49 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2017-07-07 13:35:49 AEST

Size: 4.99 KB

patch

obsolete

>diff --git a/ssh-keygen.1 b/ssh-keygen.1
>index 66f8321..1b66e45 100644
>--- a/ssh-keygen.1
>+++ b/ssh-keygen.1
>@@ -126,6 +126,7 @@
> .Op Fl f Ar input_keyfile
> .Nm ssh-keygen
> .Fl A
>+.Op Fl f Ar prefix_path
> .Nm ssh-keygen
> .Fl k
> .Fl f Ar krl_file
>@@ -224,6 +225,10 @@ For each of the key types (rsa, dsa, ecdsa and ed25519)
> for which host keys
> do not exist, generate the host keys with the default key file path,
> an empty passphrase, default bits for the key type, and default comment.
>+If a
>+.Fl f
>+option has been specified, then its argument is used as a prefix to the
>+default path for the resulting host key files.
> This is used by
> .Pa /etc/rc
> to generate new host keys.
>diff --git a/ssh-keygen.c b/ssh-keygen.c
>index a91f815..6f9f6f3 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -981,20 +981,38 @@ do_gen_all_hostkeys(struct passwd *pw)
> 	int first = 0;
> 	struct stat st;
> 	struct sshkey *private, *public;
>-	char comment[1024];
>+	char comment[1024], *prv_tmp, *pub_tmp, *prv_file, *pub_file;
> 	int i, type, fd, r;
> 	FILE *f;
> 
> 	for (i = 0; key_types[i].key_type; i++) {
>-		if (stat(key_types[i].path, &st) == 0)
>-			continue;
>-		if (errno != ENOENT) {
>+		public = private = NULL;
>+		prv_tmp = pub_tmp = prv_file = pub_file = NULL;
>+
>+		xasprintf(&prv_file, "%s%s",
>+		    identity_file, key_types[i].path);
>+
>+		/* Check whether private key exists and is not zero-length */
>+		if (stat(prv_file, &st) == 0) {
>+			if (st.st_size != 0)
>+				goto next;
>+		} else if (errno != ENOENT) {
> 			error("Could not stat %s: %s", key_types[i].path,
> 			    strerror(errno));
>-			first = 0;
>-			continue;
>+			goto failnext;
> 		}
> 
>+		/*
>+		 * Private key doesn't exist or is invalid; proceed with
>+		 * key generation.
>+		 */
>+		xasprintf(&prv_tmp, "%s%s.XXXXXXXXXX",
>+		    identity_file, key_types[i].path);
>+		xasprintf(&pub_tmp, "%s%s.pub.XXXXXXXXXX",
>+		    identity_file, key_types[i].path);
>+		xasprintf(&pub_file, "%s%s.pub",
>+		    identity_file, key_types[i].path);
>+
> 		if (first == 0) {
> 			first = 1;
> 			printf("%s: generating new host keys: ", __progname);
>@@ -1002,56 +1020,76 @@ do_gen_all_hostkeys(struct passwd *pw)
> 		printf("%s ", key_types[i].key_type_display);
> 		fflush(stdout);
> 		type = sshkey_type_from_name(key_types[i].key_type);
>-		strlcpy(identity_file, key_types[i].path, sizeof(identity_file));
>+		if ((fd = mkstemp(prv_tmp)) == -1) {
>+			error("Could not save your public key in %s: %s",
>+			    prv_tmp, strerror(errno));
>+			goto failnext;
>+		}
>+		close(fd); /* just using mkstemp() to generate/reserve a name */
> 		bits = 0;
> 		type_bits_valid(type, NULL, &bits);
> 		if ((r = sshkey_generate(type, bits, &private)) != 0) {
> 			error("sshkey_generate failed: %s", ssh_err(r));
>-			first = 0;
>-			continue;
>+			goto failnext;
> 		}
> 		if ((r = sshkey_from_private(private, &public)) != 0)
> 			fatal("sshkey_from_private failed: %s", ssh_err(r));
> 		snprintf(comment, sizeof comment, "%s@%s", pw->pw_name,
> 		    hostname);
>-		if ((r = sshkey_save_private(private, identity_file, "",
>+		if ((r = sshkey_save_private(private, prv_tmp, "",
> 		    comment, use_new_format, new_format_cipher, rounds)) != 0) {
> 			error("Saving key \"%s\" failed: %s",
>-			    identity_file, ssh_err(r));
>-			sshkey_free(private);
>-			sshkey_free(public);
>-			first = 0;
>-			continue;
>+			    prv_tmp, ssh_err(r));
>+			goto failnext;
> 		}
>-		sshkey_free(private);
>-		strlcat(identity_file, ".pub", sizeof(identity_file));
>-		fd = open(identity_file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
>-		if (fd == -1) {
>-			error("Could not save your public key in %s",
>-			    identity_file);
>-			sshkey_free(public);
>-			first = 0;
>-			continue;
>+		if ((fd = mkstemp(pub_tmp)) == -1) {
>+			error("Could not save your public key in %s: %s",
>+			    pub_tmp, strerror(errno));
>+			goto failnext;
> 		}
>+		(void)fchmod(fd, 0644);
> 		f = fdopen(fd, "w");
> 		if (f == NULL) {
>-			error("fdopen %s failed", identity_file);
>+			error("fdopen %s failed: %s", pub_tmp, strerror(errno));
> 			close(fd);
>-			sshkey_free(public);
>-			first = 0;
>-			continue;
>+			goto failnext;
> 		}
> 		if ((r = sshkey_write(public, f)) != 0) {
> 			error("write key failed: %s", ssh_err(r));
> 			fclose(f);
>-			sshkey_free(public);
>-			first = 0;
>-			continue;
>+			goto failnext;
> 		}
> 		fprintf(f, " %s\n", comment);
>-		fclose(f);
>-		sshkey_free(public);
>+		if (ferror(f) != 0) {
>+			error("write key failed: %s", strerror(errno));
>+			fclose(f);
>+			goto failnext;
>+		}
>+		if (fclose(f) != 0) {
>+			error("key close failed: %s", strerror(errno));
>+			goto failnext;
>+		}
> 
>+		/* Rename temporary files to their permanent locations. */
>+		if (rename(pub_tmp, pub_file) != 0) {
>+			error("Unable to move %s into position: %s",
>+			    pub_file, strerror(errno));
>+			goto failnext;
>+		}
>+		if (rename(prv_tmp, prv_file) != 0) {
>+			error("Unable to move %s into position: %s",
>+			    key_types[i].path, strerror(errno));
>+ failnext:
>+			first = 0;
>+			goto next;
>+		}
>+ next:
>+		sshkey_free(private);
>+		sshkey_free(public);
>+		free(prv_tmp);
>+		free(pub_tmp);
>+		free(prv_file);
>+		free(pub_file);
> 	}
> 	if (first != 0)
> 		printf("\n");

Flags:

dtucker: ok+

Actions: View | Diff

Attachments on bug 2561: 2804 | 2805 | 3003 | 3004 | 3009