From: djm <djm@openbsd.org>
Date: Thu, 8 Jan 2015 10:14:08 +0000 (+0000)
Subject: deprecate key_load_private_pem() and sshkey_load_private_pem()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=03b8b3798bf495099c640d9f1e9556e36f0a419b;p=openbsd

deprecate key_load_private_pem() and sshkey_load_private_pem()
interfaces. Refactor the generic key loading API to not require
pathnames to be specified (they weren't really used).

Fixes a few other things en passant:

Makes ed25519 keys work for hostbased authentication (ssh-keysign
previously used the PEM-only routines).

Fixes key comment regression bz#2306: key pathnames were being lost as
comment fields.

ok markus@
---

diff --git a/usr.bin/ssh/auth2-hostbased.c b/usr.bin/ssh/auth2-hostbased.c
index e378562186a..e75a5cf0cd4 100644
--- a/usr.bin/ssh/auth2-hostbased.c
+++ b/usr.bin/ssh/auth2-hostbased.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-hostbased.c,v 1.20 2014/12/23 22:42:48 djm Exp $ */
+/* $OpenBSD: auth2-hostbased.c,v 1.21 2015/01/08 10:14:08 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -83,6 +83,7 @@ userauth_hostbased(Authctxt *authctxt)
 	buffer_dump(&b);
 	buffer_free(&b);
 #endif
+	/* XXX provide some way to allow admin to specify key types accepted */
 	pktype = key_type_from_name(pkalg);
 	if (pktype == KEY_UNSPEC) {
 		/* this is perfectly legal */
diff --git a/usr.bin/ssh/authfile.c b/usr.bin/ssh/authfile.c
index 0a446a4f64d..4e0a67ae047 100644
--- a/usr.bin/ssh/authfile.c
+++ b/usr.bin/ssh/authfile.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: authfile.c,v 1.108 2014/12/04 02:24:32 djm Exp $ */
+/* $OpenBSD: authfile.c,v 1.109 2015/01/08 10:14:08 djm Exp $ */
 /*
  * Copyright (c) 2000, 2013 Markus Friedl.  All rights reserved.
  *
@@ -93,7 +93,7 @@ sshkey_save_private(struct sshkey *key, const char *filename,
 
 /* Load a key from a fd into a buffer */
 int
-sshkey_load_file(int fd, const char *filename, struct sshbuf *blob)
+sshkey_load_file(int fd, struct sshbuf *blob)
 {
 	u_char buf[1024];
 	size_t len;
@@ -140,8 +140,7 @@ sshkey_load_file(int fd, const char *filename, struct sshbuf *blob)
  * otherwise.
  */
 static int
-sshkey_load_public_rsa1(int fd, const char *filename,
-    struct sshkey **keyp, char **commentp)
+sshkey_load_public_rsa1(int fd, struct sshkey **keyp, char **commentp)
 {
 	struct sshbuf *b = NULL;
 	int r;
@@ -152,7 +151,7 @@ sshkey_load_public_rsa1(int fd, const char *filename,
 
 	if ((b = sshbuf_new()) == NULL)
 		return SSH_ERR_ALLOC_FAIL;
-	if ((r = sshkey_load_file(fd, filename, b)) != 0)
+	if ((r = sshkey_load_file(fd, b)) != 0)
 		goto out;
 	if ((r = sshkey_parse_public_rsa1_fileblob(b, keyp, commentp)) != 0)
 		goto out;
@@ -163,33 +162,6 @@ sshkey_load_public_rsa1(int fd, const char *filename,
 }
 #endif /* WITH_SSH1 */
 
-#ifdef WITH_OPENSSL
-/* XXX Deprecate? */
-int
-sshkey_load_private_pem(int fd, int type, const char *passphrase,
-    struct sshkey **keyp, char **commentp)
-{
-	struct sshbuf *buffer = NULL;
-	int r;
-
-	*keyp = NULL;
-	if (commentp != NULL)
-		*commentp = NULL;
-
-	if ((buffer = sshbuf_new()) == NULL)
-		return SSH_ERR_ALLOC_FAIL;
-	if ((r = sshkey_load_file(fd, NULL, buffer)) != 0)
-		goto out;
-	if ((r = sshkey_parse_private_pem_fileblob(buffer, type, passphrase,
-	    keyp, commentp)) != 0)
-		goto out;
-	r = 0;
- out:
-	sshbuf_free(buffer);
-	return r;
-}
-#endif /* WITH_OPENSSL */
-
 /* XXX remove error() calls from here? */
 int
 sshkey_perm_ok(int fd, const char *filename)
@@ -222,7 +194,6 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase,
     struct sshkey **keyp, char **commentp, int *perm_ok)
 {
 	int fd, r;
-	struct sshbuf *buffer = NULL;
 
 	*keyp = NULL;
 	if (commentp != NULL)
@@ -242,18 +213,31 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase,
 	if (perm_ok != NULL)
 		*perm_ok = 1;
 
+	r = sshkey_load_private_type_fd(fd, type, passphrase, keyp, commentp);
+ out:
+	close(fd);
+	return r;
+}
+
+int
+sshkey_load_private_type_fd(int fd, int type, const char *passphrase,
+    struct sshkey **keyp, char **commentp)
+{
+	struct sshbuf *buffer = NULL;
+	int r;
+
 	if ((buffer = sshbuf_new()) == NULL) {
 		r = SSH_ERR_ALLOC_FAIL;
 		goto out;
 	}
-	if ((r = sshkey_load_file(fd, filename, buffer)) != 0)
-		goto out;
-	if ((r = sshkey_parse_private_fileblob_type(buffer, type, passphrase,
-	    keyp, commentp)) != 0)
+	if ((r = sshkey_load_file(fd, buffer)) != 0 ||
+	    (r = sshkey_parse_private_fileblob_type(buffer, type,
+	    passphrase, keyp, commentp)) != 0)
 		goto out;
+
+	/* success */
 	r = 0;
  out:
-	close(fd);
 	if (buffer != NULL)
 		sshbuf_free(buffer);
 	return r;
@@ -282,7 +266,7 @@ sshkey_load_private(const char *filename, const char *passphrase,
 		r = SSH_ERR_ALLOC_FAIL;
 		goto out;
 	}
-	if ((r = sshkey_load_file(fd, filename, buffer)) != 0 ||
+	if ((r = sshkey_load_file(fd, buffer)) != 0 ||
 	    (r = sshkey_parse_private_fileblob(buffer, passphrase, filename,
 	    keyp, commentp)) != 0)
 		goto out;
@@ -358,7 +342,7 @@ sshkey_load_public(const char *filename, struct sshkey **keyp, char **commentp)
 		goto skip;
 #ifdef WITH_SSH1
 	/* try rsa1 private key */
-	r = sshkey_load_public_rsa1(fd, filename, keyp, commentp);
+	r = sshkey_load_public_rsa1(fd, keyp, commentp);
 	close(fd);
 	switch (r) {
 	case SSH_ERR_INTERNAL_ERROR:
diff --git a/usr.bin/ssh/authfile.h b/usr.bin/ssh/authfile.h
index 645404e61f9..624d269f1bd 100644
--- a/usr.bin/ssh/authfile.h
+++ b/usr.bin/ssh/authfile.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: authfile.h,v 1.20 2014/12/04 02:24:32 djm Exp $ */
+/* $OpenBSD: authfile.h,v 1.21 2015/01/08 10:14:08 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2013 Markus Friedl.  All rights reserved.
@@ -30,9 +30,12 @@
 struct sshbuf;
 struct sshkey;
 
+/* XXX document these */
+/* XXX some of these could probably be merged/retired */
+
 int sshkey_save_private(struct sshkey *, const char *,
     const char *, const char *, int, const char *, int);
-int sshkey_load_file(int, const char *, struct sshbuf *);
+int sshkey_load_file(int, struct sshbuf *);
 int sshkey_load_cert(const char *, struct sshkey **);
 int sshkey_load_public(const char *, struct sshkey **, char **);
 int sshkey_load_private(const char *, const char *, struct sshkey **, char **);
@@ -40,7 +43,8 @@ int sshkey_load_private_cert(int, const char *, const char *,
     struct sshkey **, int *);
 int sshkey_load_private_type(int, const char *, const char *,
     struct sshkey **, char **, int *);
-int sshkey_load_private_pem(int, int, const char *, struct sshkey **, char **);
+int sshkey_load_private_type_fd(int fd, int type, const char *passphrase,
+    struct sshkey **keyp, char **commentp);
 int sshkey_perm_ok(int, const char *);
 int sshkey_in_file(struct sshkey *, const char *, int, int);
 int sshkey_check_revoked(struct sshkey *key, const char *revoked_keys_file);
diff --git a/usr.bin/ssh/key.c b/usr.bin/ssh/key.c
index 2cc6949b62e..a2b9a97814c 100644
--- a/usr.bin/ssh/key.c
+++ b/usr.bin/ssh/key.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: key.c,v 1.124 2014/12/21 22:27:56 djm Exp $ */
+/* $OpenBSD: key.c,v 1.125 2015/01/08 10:14:08 djm Exp $ */
 /*
  * placed in the public domain
  */
@@ -326,7 +326,7 @@ key_load_file(int fd, const char *filename, struct sshbuf *blob)
 {
 	int r;
 
-	if ((r = sshkey_load_file(fd, filename, blob)) != 0) {
+	if ((r = sshkey_load_file(fd, blob)) != 0) {
 		fatal_on_fatal_errors(r, __func__, SSH_ERR_LIBCRYPTO_ERROR);
 		error("%s: %s", __func__, ssh_err(r));
 		return 0;
@@ -433,27 +433,6 @@ key_load_private_type(int type, const char *filename, const char *passphrase,
 	return ret;
 }
 
-#ifdef WITH_OPENSSL
-Key *
-key_load_private_pem(int fd, int type, const char *passphrase,
-    char **commentp)
-{
-	int r;
-	Key *ret = NULL;
-
-	if ((r = sshkey_load_private_pem(fd, type, passphrase,
-	     &ret, commentp)) != 0) {
-		fatal_on_fatal_errors(r, __func__, SSH_ERR_LIBCRYPTO_ERROR);
-		if (r == SSH_ERR_KEY_WRONG_PASSPHRASE)
-			debug("%s: %s", __func__, ssh_err(r));
-		else
-			error("%s: %s", __func__, ssh_err(r));
-		return NULL;
-	}
-	return ret;
-}
-#endif /* WITH_OPENSSL */
-
 int
 key_perm_ok(int fd, const char *filename)
 {
diff --git a/usr.bin/ssh/key.h b/usr.bin/ssh/key.h
index 2cf9bc22608..9d7e84e78e4 100644
--- a/usr.bin/ssh/key.h
+++ b/usr.bin/ssh/key.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: key.h,v 1.44 2014/12/21 22:27:56 djm Exp $ */
+/* $OpenBSD: key.h,v 1.45 2015/01/08 10:14:08 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -104,7 +104,6 @@ Key	*key_load_public(const char *, char **);
 Key	*key_load_private(const char *, const char *, char **);
 Key	*key_load_private_cert(int, const char *, const char *, int *);
 Key	*key_load_private_type(int, const char *, const char *, char **, int *);
-Key	*key_load_private_pem(int, int, const char *, char **);
 int	 key_perm_ok(int, const char *);
 
 #endif
diff --git a/usr.bin/ssh/krl.c b/usr.bin/ssh/krl.c
index dae6aed546b..977535f5012 100644
--- a/usr.bin/ssh/krl.c
+++ b/usr.bin/ssh/krl.c
@@ -14,7 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $OpenBSD: krl.c,v 1.21 2014/12/21 22:27:56 djm Exp $ */
+/* $OpenBSD: krl.c,v 1.22 2015/01/08 10:14:08 djm Exp $ */
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -1246,7 +1246,7 @@ ssh_krl_file_contains_key(const char *path, const struct sshkey *key)
 		oerrno = errno;
 		goto out;
 	}
-	if ((r = sshkey_load_file(fd, path, krlbuf)) != 0) {
+	if ((r = sshkey_load_file(fd, krlbuf)) != 0) {
 		oerrno = errno;
 		goto out;
 	}
diff --git a/usr.bin/ssh/ssh-keysign.c b/usr.bin/ssh/ssh-keysign.c
index 7b119c993ed..5d1a4167a1f 100644
--- a/usr.bin/ssh/ssh-keysign.c
+++ b/usr.bin/ssh/ssh-keysign.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keysign.c,v 1.44 2014/12/21 22:27:56 djm Exp $ */
+/* $OpenBSD: ssh-keysign.c,v 1.45 2015/01/08 10:14:08 djm Exp $ */
 /*
  * Copyright (c) 2002 Markus Friedl.  All rights reserved.
  *
@@ -48,6 +48,8 @@
 #include "pathnames.h"
 #include "readconf.h"
 #include "uidswap.h"
+#include "sshkey.h"
+#include "ssherr.h"
 
 /* XXX readconf.c needs these */
 uid_t original_real_uid;
@@ -63,6 +65,8 @@ valid_request(struct passwd *pw, char *host, Key **ret, u_char *data,
 	char *pkalg, *p;
 	int pktype, fail;
 
+	if (ret != NULL)
+		*ret = NULL;
 	fail = 0;
 
 	buffer_init(&b);
@@ -147,7 +151,7 @@ main(int argc, char **argv)
 #define NUM_KEYTYPES 4
 	Key *keys[NUM_KEYTYPES], *key = NULL;
 	struct passwd *pw;
-	int key_fd[NUM_KEYTYPES], i, found, version = 2, fd;
+	int r, key_fd[NUM_KEYTYPES], i, found, version = 2, fd;
 	u_char *signature, *data;
 	char *host, *fp;
 	u_int slen, dlen;
@@ -198,14 +202,15 @@ main(int argc, char **argv)
 		keys[i] = NULL;
 		if (key_fd[i] == -1)
 			continue;
-#ifdef WITH_OPENSSL
-/* XXX wrong api */
-		keys[i] = key_load_private_pem(key_fd[i], KEY_UNSPEC,
-		    NULL, NULL);
-#endif
+		r = sshkey_load_private_type_fd(key_fd[i], KEY_UNSPEC,
+		    NULL, &key, NULL);
 		close(key_fd[i]);
-		if (keys[i] != NULL)
+		if (r != 0)
+			debug("parse key %d: %s", i, ssh_err(r));
+		else if (key != NULL) {
+			keys[i] = key;
 			found = 1;
+		}
 	}
 	if (!found)
 		fatal("no hostkey found");
diff --git a/usr.bin/ssh/sshconnect2.c b/usr.bin/ssh/sshconnect2.c
index 476cf4447e4..058381ee4af 100644
--- a/usr.bin/ssh/sshconnect2.c
+++ b/usr.bin/ssh/sshconnect2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect2.c,v 1.212 2014/12/21 22:27:56 djm Exp $ */
+/* $OpenBSD: sshconnect2.c,v 1.213 2015/01/08 10:14:08 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Damien Miller.  All rights reserved.
@@ -1483,6 +1483,8 @@ userauth_hostbased(Authctxt *authctxt)
 	u_int blen, slen;
 	int ok, i, found = 0;
 
+	/* XXX provide some way to allow user to specify key types attempted */
+
 	/* check for a useful key */
 	for (i = 0; i < sensitive->nkeys; i++) {
 		private = sensitive->keys[i];
diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index 84e39a1636a..55b9352ed3a 100644
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.7 2014/12/21 22:27:55 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.8 2015/01/08 10:14:08 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -3663,20 +3663,16 @@ sshkey_parse_private_rsa1(struct sshbuf *blob, const char *passphrase,
 #endif /* WITH_SSH1 */
 
 #ifdef WITH_OPENSSL
-/* XXX make private once ssh-keysign.c fixed */
-int
+static int
 sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
-    const char *passphrase, struct sshkey **keyp, char **commentp)
+    const char *passphrase, struct sshkey **keyp)
 {
 	EVP_PKEY *pk = NULL;
 	struct sshkey *prv = NULL;
-	char *name = "<no key>";
 	BIO *bio = NULL;
 	int r;
 
 	*keyp = NULL;
-	if (commentp != NULL)
-		*commentp = NULL;
 
 	if ((bio = BIO_new(BIO_s_mem())) == NULL || sshbuf_len(blob) > INT_MAX)
 		return SSH_ERR_ALLOC_FAIL;
@@ -3699,7 +3695,6 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
 		}
 		prv->rsa = EVP_PKEY_get1_RSA(pk);
 		prv->type = KEY_RSA;
-		name = "rsa w/o comment";
 #ifdef DEBUG_PK
 		RSA_print_fp(stderr, prv->rsa, 8);
 #endif
@@ -3715,7 +3710,6 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
 		}
 		prv->dsa = EVP_PKEY_get1_DSA(pk);
 		prv->type = KEY_DSA;
-		name = "dsa w/o comment";
 #ifdef DEBUG_PK
 		DSA_print_fp(stderr, prv->dsa, 8);
 #endif
@@ -3736,7 +3730,6 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
 			r = SSH_ERR_INVALID_FORMAT;
 			goto out;
 		}
-		name = "ecdsa w/o comment";
 #ifdef DEBUG_PK
 		if (prv != NULL && prv->ecdsa != NULL)
 			sshkey_dump_ec_key(prv->ecdsa);
@@ -3745,11 +3738,6 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
 		r = SSH_ERR_INVALID_FORMAT;
 		goto out;
 	}
-	if (commentp != NULL &&
-	    (*commentp = strdup(name)) == NULL) {
-		r = SSH_ERR_ALLOC_FAIL;
-		goto out;
-	}
 	r = 0;
 	*keyp = prv;
 	prv = NULL;
@@ -3781,8 +3769,8 @@ sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type,
 	case KEY_DSA:
 	case KEY_ECDSA:
 	case KEY_RSA:
-		return sshkey_parse_private_pem_fileblob(blob, type, passphrase,
-		    keyp, commentp);
+		return sshkey_parse_private_pem_fileblob(blob, type,
+		    passphrase, keyp);
 #endif /* WITH_OPENSSL */
 	case KEY_ED25519:
 		return sshkey_parse_private2(blob, type, passphrase,
@@ -3792,8 +3780,8 @@ sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type,
 		    commentp)) == 0)
 			return 0;
 #ifdef WITH_OPENSSL
-		return sshkey_parse_private_pem_fileblob(blob, type, passphrase,
-		    keyp, commentp);
+		return sshkey_parse_private_pem_fileblob(blob, type,
+		    passphrase, keyp);
 #else
 		return SSH_ERR_INVALID_FORMAT;
 #endif /* WITH_OPENSSL */
diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h
index 7f595c18894..2db64d76787 100644
--- a/usr.bin/ssh/sshkey.h
+++ b/usr.bin/ssh/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.2 2014/12/21 22:27:55 djm Exp $ */
+/* $OpenBSD: sshkey.h,v 1.3 2015/01/08 10:14:08 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -178,8 +178,6 @@ int	sshkey_private_to_fileblob(struct sshkey *key, struct sshbuf *blob,
     int force_new_format, const char *new_format_cipher, int new_format_rounds);
 int	sshkey_parse_public_rsa1_fileblob(struct sshbuf *blob,
     struct sshkey **keyp, char **commentp);
-int	sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
-    const char *passphrase, struct sshkey **keyp, char **commentp);
 int	sshkey_parse_private_fileblob(struct sshbuf *buffer,
     const char *passphrase, const char *filename, struct sshkey **keyp,
     char **commentp);