When generating all hostkeys (ssh-keygen -A), clobber existing keys

if they exist but are zero length. zero-length keys could previously
be made if ssh-keygen failed part way through generating them, so avoid
that case too. bz#2561 reported by Krzysztof Cieplucha; ok dtucker@

2 files changed, 79 insertions, 36 deletions

diff --git a/usr.bin/ssh/ssh-keygen.1 b/usr.bin/ssh/ssh-keygen.1
index 66f8321c536..2bc61639aa7 100644
--- a/usr.bin/ssh/ssh-keygen.1
+++ b/usr.bin/ssh/ssh-keygen.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ssh-keygen.1,v 1.142 2017/06/28 01:09:22 djm Exp $
+.\"	$OpenBSD: ssh-keygen.1,v 1.143 2017/07/07 03:53:12 djm Exp $
 .\"
 .\" Author: Tatu Ylonen <ylo@cs.hut.fi>
 .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -35,7 +35,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: June 28 2017 $
+.Dd $Mdocdate: July 7 2017 $
 .Dt SSH-KEYGEN 1
 .Os
 .Sh NAME
@@ -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/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c
index a91f8157cf7..2b9782c5c08 100644
--- a/usr.bin/ssh/ssh-keygen.c
+++ b/usr.bin/ssh/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.306 2017/07/01 13:50:45 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.307 2017/07/07 03:53:12 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -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");