Attachment 2805 Details for Bug 2561 – overwrite zero-length key files, move keys into place atomically

[patch] overwrite zero-length key files, move keys into place atomically

bz2561.diff (text/plain), 6.51 KB, created by Damien Miller on 2016-04-08 15:37:05 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2016-04-08 15:37:05 AEST

Size: 6.51 KB

patch

obsolete

>diff --git a/authfile.c b/authfile.c
>index d670424..04f0c3b 100644
>--- a/authfile.c
>+++ b/authfile.c
>@@ -200,7 +200,8 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase,
> {
> 	int fd, r;
> 
>-	*keyp = NULL;
>+	if (keyp != NULL)
>+		*keyp = NULL;
> 	if (commentp != NULL)
> 		*commentp = NULL;
> 
>diff --git a/ssh-keygen.1 b/ssh-keygen.1
>index 37a4fc2..da6638a 100644
>--- a/ssh-keygen.1
>+++ b/ssh-keygen.1
>@@ -124,6 +124,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
>@@ -227,6 +228,10 @@ For each of the key types (rsa1, 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 4785201..4087f63 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -992,18 +992,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;
> 
>+	/*
>+	 * Ensure that prefix path identity_file ends in '/' if it has been
>+	 * specified for easier joining to its suffixes later.
>+	 */
>+	if (*identity_file && identity_file[strlen(identity_file) - 1] != '/') {
>+		if (strlcat(identity_file, "/", sizeof(identity_file)) >=
>+		    sizeof(identity_file))
>+			fatal("prefix path is too long");
>+	}
>+
> 	for (i = 0; key_types[i].key_type; i++) {
>-		if (stat(key_types[i].path, &st) == 0)
>-			continue;
>-		if (errno != ENOENT) {
>+		public = private = NULL;
>+		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(&prv_file, "%s%s",
>+		    identity_file, key_types[i].path);
>+		xasprintf(&pub_file, "%s%s.pub",
>+		    identity_file, key_types[i].path);
>+
>+		if (stat(prv_file, &st) == 0) {
>+			if (st.st_size != 0)
>+				continue;
>+		} else if (errno != ENOENT) {
> 			error("Could not stat %s: %s", key_types[i].path,
> 			    strerror(errno));
>-			first = 0;
>-			continue;
>+			goto failnext;
> 		}
> 
> 		if (first == 0) {
>@@ -1013,56 +1033,68 @@ 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("key_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", pub_tmp);
> 			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);
> 
>+		/* 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");
>diff --git a/sshkey.c b/sshkey.c
>index 87b093e..f0f373d 100644
>--- a/sshkey.c
>+++ b/sshkey.c
>@@ -3657,7 +3657,8 @@ sshkey_parse_private_rsa1(struct sshbuf *blob, const char *passphrase,
> 	const struct sshcipher *cipher;
> 	struct sshkey *prv = NULL;
> 
>-	*keyp = NULL;
>+	if (keyp != NULL)
>+		*keyp = NULL;
> 	if (commentp != NULL)
> 		*commentp = NULL;
> 
>@@ -3743,8 +3744,10 @@ sshkey_parse_private_rsa1(struct sshbuf *blob, const char *passphrase,
> 		goto out;
> 	}
> 	r = 0;
>-	*keyp = prv;
>-	prv = NULL;
>+	if (keyp != NULL) {
>+		*keyp = prv;
>+		prv = NULL;
>+	}
> 	if (commentp != NULL) {
> 		*commentp = comment;
> 		comment = NULL;
>@@ -3769,7 +3772,8 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
> 	BIO *bio = NULL;
> 	int r;
> 
>-	*keyp = NULL;
>+	if (keyp != NULL)
>+		*keyp = NULL;
> 
> 	if ((bio = BIO_new(BIO_s_mem())) == NULL || sshbuf_len(blob) > INT_MAX)
> 		return SSH_ERR_ALLOC_FAIL;
>@@ -3838,8 +3842,10 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
> 		goto out;
> 	}
> 	r = 0;
>-	*keyp = prv;
>-	prv = NULL;
>+	if (keyp != NULL) {
>+		*keyp = prv;
>+		prv = NULL;
>+	}
>  out:
> 	BIO_free(bio);
> 	if (pk != NULL)
>@@ -3853,7 +3859,8 @@ int
> sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type,
>     const char *passphrase, struct sshkey **keyp, char **commentp)
> {
>-	*keyp = NULL;
>+	if (keyp != NULL)
>+		*keyp = NULL;
> 	if (commentp != NULL)
> 		*commentp = NULL;
>

Actions: View | Diff

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