From eefcf659ce2e5a878ac108abff86da265eefa6e2 Mon Sep 17 00:00:00 2001
From: djm <djm@openbsd.org>
Date: Fri, 28 Oct 2022 00:37:24 +0000
Subject: [PATCH] factor out public key serialization

feedback/ok markus@
---
 usr.bin/ssh/ssh-dss.c        |  27 +++++++-
 usr.bin/ssh/ssh-ecdsa-sk.c   |  18 +++++-
 usr.bin/ssh/ssh-ecdsa.c      |  21 ++++++-
 usr.bin/ssh/ssh-ed25519-sk.c |  18 +++++-
 usr.bin/ssh/ssh-ed25519.c    |  18 +++++-
 usr.bin/ssh/ssh-rsa.c        |  21 ++++++-
 usr.bin/ssh/ssh-xmss.c       |  22 ++++++-
 usr.bin/ssh/sshkey.c         | 115 ++++++++---------------------------
 usr.bin/ssh/sshkey.h         |   5 +-
 9 files changed, 166 insertions(+), 99 deletions(-)

diff --git a/usr.bin/ssh/ssh-dss.c b/usr.bin/ssh/ssh-dss.c
index 930f49d327e..fdfc55ce8c7 100644
--- a/usr.bin/ssh/ssh-dss.c
+++ b/usr.bin/ssh/ssh-dss.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-dss.c,v 1.41 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-dss.c,v 1.42 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -94,6 +94,30 @@ ssh_dss_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_dss_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+	const BIGNUM *dsa_p, *dsa_q, *dsa_g, *dsa_pub_key;
+
+	if (key->dsa == NULL)
+		return SSH_ERR_INVALID_ARGUMENT;
+	DSA_get0_pqg(key->dsa, &dsa_p, &dsa_q, &dsa_g);
+	DSA_get0_key(key->dsa, &dsa_pub_key, NULL);
+	if (dsa_p == NULL || dsa_q == NULL ||
+	    dsa_g == NULL || dsa_pub_key == NULL)
+		return SSH_ERR_INTERNAL_ERROR;
+	if ((r = sshbuf_put_cstring(b, typename)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, dsa_p)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, dsa_q)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, dsa_g)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, dsa_pub_key)) != 0)
+		return r;
+
+	return 0;
+}
+
 int
 ssh_dss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -256,6 +280,7 @@ static const struct sshkey_impl_funcs sshkey_dss_funcs = {
 	/* .alloc = */		ssh_dss_alloc,
 	/* .cleanup = */	ssh_dss_cleanup,
 	/* .equal = */		ssh_dss_equal,
+	/* .ssh_serialize_public = */ ssh_dss_serialize_public,
 };
 
 const struct sshkey_impl sshkey_dss_impl = {
diff --git a/usr.bin/ssh/ssh-ecdsa-sk.c b/usr.bin/ssh/ssh-ecdsa-sk.c
index 73028c5b43a..6c663c20cba 100644
--- a/usr.bin/ssh/ssh-ecdsa-sk.c
+++ b/usr.bin/ssh/ssh-ecdsa-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-ecdsa-sk.c,v 1.10 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-ecdsa-sk.c,v 1.11 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -63,6 +63,21 @@ ssh_ecdsa_sk_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_ecdsa_sk_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+
+	if ((r = sshkey_ecdsa_funcs.serialize_public(key, b,
+	    typename, opts)) != 0)
+		return r;
+	if ((r = sshkey_serialize_sk(key, b)) != 0)
+		return r;
+
+	return 0;
+}
+
 /*
  * Check FIDO/W3C webauthn signatures clientData field against the expected
  * format and prepare a hash of it for use in signature verification.
@@ -328,6 +343,7 @@ static const struct sshkey_impl_funcs sshkey_ecdsa_sk_funcs = {
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ecdsa_sk_cleanup,
 	/* .equal = */		ssh_ecdsa_sk_equal,
+	/* .ssh_serialize_public = */ ssh_ecdsa_sk_serialize_public,
 };
 
 const struct sshkey_impl sshkey_ecdsa_sk_impl = {
diff --git a/usr.bin/ssh/ssh-ecdsa.c b/usr.bin/ssh/ssh-ecdsa.c
index 2a6c5e4ac92..af3413efffa 100644
--- a/usr.bin/ssh/ssh-ecdsa.c
+++ b/usr.bin/ssh/ssh-ecdsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-ecdsa.c,v 1.18 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-ecdsa.c,v 1.19 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -79,9 +79,27 @@ ssh_ecdsa_equal(const struct sshkey *a, const struct sshkey *b)
 		return 0;
 	if (EC_POINT_cmp(grp_a, pub_a, pub_b, NULL) != 0)
 		return 0;
+
 	return 1;
 }
 
+static int
+ssh_ecdsa_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+
+	if (key->ecdsa == NULL)
+		return SSH_ERR_INVALID_ARGUMENT;
+	if ((r = sshbuf_put_cstring(b, typename)) != 0 ||
+	    (r = sshbuf_put_cstring(b,
+	    sshkey_curve_nid_to_name(key->ecdsa_nid))) != 0 ||
+	    (r = sshbuf_put_eckey(b, key->ecdsa)) != 0)
+		return r;
+
+	return 0;
+}
+
 /* ARGSUSED */
 int
 ssh_ecdsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
@@ -240,6 +258,7 @@ const struct sshkey_impl_funcs sshkey_ecdsa_funcs = {
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ecdsa_cleanup,
 	/* .equal = */		ssh_ecdsa_equal,
+	/* .ssh_serialize_public = */ ssh_ecdsa_serialize_public,
 };
 
 const struct sshkey_impl sshkey_ecdsa_nistp256_impl = {
diff --git a/usr.bin/ssh/ssh-ed25519-sk.c b/usr.bin/ssh/ssh-ed25519-sk.c
index ba94ec67d43..9183a4394a7 100644
--- a/usr.bin/ssh/ssh-ed25519-sk.c
+++ b/usr.bin/ssh/ssh-ed25519-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-ed25519-sk.c,v 1.8 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-ed25519-sk.c,v 1.9 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl.  All rights reserved.
  *
@@ -53,6 +53,21 @@ ssh_ed25519_sk_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_ed25519_sk_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+
+	if ((r = sshkey_ed25519_funcs.serialize_public(key, b,
+	    typename, opts)) != 0)
+		return r;
+	if ((r = sshkey_serialize_sk(key, b)) != 0)
+		return r;
+
+	return 0;
+}
+
 int
 ssh_ed25519_sk_verify(const struct sshkey *key,
     const u_char *signature, size_t signaturelen,
@@ -185,6 +200,7 @@ static const struct sshkey_impl_funcs sshkey_ed25519_sk_funcs = {
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ed25519_sk_cleanup,
 	/* .equal = */		ssh_ed25519_sk_equal,
+	/* .ssh_serialize_public = */ ssh_ed25519_sk_serialize_public,
 };
 
 const struct sshkey_impl sshkey_ed25519_sk_impl = {
diff --git a/usr.bin/ssh/ssh-ed25519.c b/usr.bin/ssh/ssh-ed25519.c
index f6b90af9d2d..a5a37bd40c6 100644
--- a/usr.bin/ssh/ssh-ed25519.c
+++ b/usr.bin/ssh/ssh-ed25519.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-ed25519.c,v 1.12 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-ed25519.c,v 1.13 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2013 Markus Friedl <markus@openbsd.org>
  *
@@ -48,6 +48,21 @@ ssh_ed25519_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_ed25519_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+
+	if (key->ed25519_pk == NULL)
+		return SSH_ERR_INVALID_ARGUMENT;
+	if ((r = sshbuf_put_cstring(b, typename)) != 0 ||
+	    (r = sshbuf_put_string(b, key->ed25519_pk, ED25519_PK_SZ)) != 0)
+		return r;
+
+	return 0;
+}
+
 int
 ssh_ed25519_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -181,6 +196,7 @@ const struct sshkey_impl_funcs sshkey_ed25519_funcs = {
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ed25519_cleanup,
 	/* .equal = */		ssh_ed25519_equal,
+	/* .ssh_serialize_public = */ ssh_ed25519_serialize_public,
 };
 
 const struct sshkey_impl sshkey_ed25519_impl = {
diff --git a/usr.bin/ssh/ssh-rsa.c b/usr.bin/ssh/ssh-rsa.c
index 5754554aab8..58c071c95bd 100644
--- a/usr.bin/ssh/ssh-rsa.c
+++ b/usr.bin/ssh/ssh-rsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-rsa.c,v 1.70 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: ssh-rsa.c,v 1.71 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
  *
@@ -79,6 +79,24 @@ ssh_rsa_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_rsa_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+	const BIGNUM *rsa_n, *rsa_e;
+
+	if (key->rsa == NULL)
+		return SSH_ERR_INVALID_ARGUMENT;
+	RSA_get0_key(key->rsa, &rsa_n, &rsa_e, NULL);
+	if ((r = sshbuf_put_cstring(b, typename)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, rsa_e)) != 0 ||
+	    (r = sshbuf_put_bignum2(b, rsa_n)) != 0)
+		return r;
+
+	return 0;
+}
+
 static const char *
 rsa_hash_alg_ident(int hash_alg)
 {
@@ -492,6 +510,7 @@ static const struct sshkey_impl_funcs sshkey_rsa_funcs = {
 	/* .alloc = */		ssh_rsa_alloc,
 	/* .cleanup = */	ssh_rsa_cleanup,
 	/* .equal = */		ssh_rsa_equal,
+	/* .ssh_serialize_public = */ ssh_rsa_serialize_public,
 };
 
 const struct sshkey_impl sshkey_rsa_impl = {
diff --git a/usr.bin/ssh/ssh-xmss.c b/usr.bin/ssh/ssh-xmss.c
index 1936af55cd3..aee1edbd951 100644
--- a/usr.bin/ssh/ssh-xmss.c
+++ b/usr.bin/ssh/ssh-xmss.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-xmss.c,v 1.7 2022/10/28 00:36:31 djm Exp $*/
+/* $OpenBSD: ssh-xmss.c,v 1.8 2022/10/28 00:37:24 djm Exp $*/
 /*
  * Copyright (c) 2017 Stefan-Lukas Gazdag.
  * Copyright (c) 2017 Markus Friedl.
@@ -60,6 +60,25 @@ ssh_xmss_equal(const struct sshkey *a, const struct sshkey *b)
 	return 1;
 }
 
+static int
+ssh_xmss_serialize_public(const struct sshkey *key, struct sshbuf *b,
+    const char *typename, enum sshkey_serialize_rep opts)
+{
+	int r;
+
+	if (key->xmss_name == NULL || key->xmss_pk == NULL ||
+	    sshkey_xmss_pklen(key) == 0)
+		return SSH_ERR_INVALID_ARGUMENT;
+	if ((r = sshbuf_put_cstring(b, typename)) != 0 ||
+	    (r = sshbuf_put_cstring(b, key->xmss_name)) != 0 ||
+	    (r = sshbuf_put_string(b, key->xmss_pk,
+	    sshkey_xmss_pklen(key))) != 0 ||
+	    (r = sshkey_xmss_serialize_pk_info(key, b, opts)) != 0)
+		return r;
+
+	return 0;
+}
+
 int
 ssh_xmss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -213,6 +232,7 @@ static const struct sshkey_impl_funcs sshkey_xmss_funcs = {
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_xmss_cleanup,
 	/* .equal = */		ssh_xmss_equal,
+	/* .ssh_serialize_public = */ ssh_xmss_serialize_public,
 };
 
 const struct sshkey_impl sshkey_xmss_impl = {
diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index 9cdd7df749b..e383c62e83a 100644
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.124 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.125 2022/10/28 00:37:24 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -650,114 +650,47 @@ sshkey_equal(const struct sshkey *a, const struct sshkey *b)
 	return sshkey_equal_public(a, b);
 }
 
+
+/* Serialise common FIDO key parts */
+int
+sshkey_serialize_sk(const struct sshkey *key, struct sshbuf *b)
+{
+	int r;
+
+	if ((r = sshbuf_put_cstring(b, key->sk_application)) != 0)
+		return r;
+
+	return 0;
+}
+
 static int
 to_blob_buf(const struct sshkey *key, struct sshbuf *b, int force_plain,
   enum sshkey_serialize_rep opts)
 {
 	int type, ret = SSH_ERR_INTERNAL_ERROR;
 	const char *typename;
-#ifdef WITH_OPENSSL
-	const BIGNUM *rsa_n, *rsa_e, *dsa_p, *dsa_q, *dsa_g, *dsa_pub_key;
-#endif /* WITH_OPENSSL */
+	const struct sshkey_impl *impl;
 
 	if (key == NULL)
 		return SSH_ERR_INVALID_ARGUMENT;
 
-	if (sshkey_is_cert(key)) {
+	type = force_plain ? sshkey_type_plain(key->type) : key->type;
+
+	if (sshkey_type_is_cert(type)) {
 		if (key->cert == NULL)
 			return SSH_ERR_EXPECTED_CERT;
 		if (sshbuf_len(key->cert->certblob) == 0)
 			return SSH_ERR_KEY_LACKS_CERTBLOB;
-	}
-	type = force_plain ? sshkey_type_plain(key->type) : key->type;
-	typename = sshkey_ssh_name_from_type_nid(type, key->ecdsa_nid);
-
-	switch (type) {
-#ifdef WITH_OPENSSL
-	case KEY_DSA_CERT:
-	case KEY_ECDSA_CERT:
-	case KEY_ECDSA_SK_CERT:
-	case KEY_RSA_CERT:
-#endif /* WITH_OPENSSL */
-	case KEY_ED25519_CERT:
-	case KEY_ED25519_SK_CERT:
-#ifdef WITH_XMSS
-	case KEY_XMSS_CERT:
-#endif /* WITH_XMSS */
 		/* Use the existing blob */
-		/* XXX modified flag? */
 		if ((ret = sshbuf_putb(b, key->cert->certblob)) != 0)
 			return ret;
-		break;
-#ifdef WITH_OPENSSL
-	case KEY_DSA:
-		if (key->dsa == NULL)
-			return SSH_ERR_INVALID_ARGUMENT;
-		DSA_get0_pqg(key->dsa, &dsa_p, &dsa_q, &dsa_g);
-		DSA_get0_key(key->dsa, &dsa_pub_key, NULL);
-		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, dsa_p)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, dsa_q)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, dsa_g)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, dsa_pub_key)) != 0)
-			return ret;
-		break;
-	case KEY_ECDSA:
-	case KEY_ECDSA_SK:
-		if (key->ecdsa == NULL)
-			return SSH_ERR_INVALID_ARGUMENT;
-		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_cstring(b,
-		    sshkey_curve_nid_to_name(key->ecdsa_nid))) != 0 ||
-		    (ret = sshbuf_put_eckey(b, key->ecdsa)) != 0)
-			return ret;
-		if (type == KEY_ECDSA_SK) {
-			if ((ret = sshbuf_put_cstring(b,
-			    key->sk_application)) != 0)
-				return ret;
-		}
-		break;
-	case KEY_RSA:
-		if (key->rsa == NULL)
-			return SSH_ERR_INVALID_ARGUMENT;
-		RSA_get0_key(key->rsa, &rsa_n, &rsa_e, NULL);
-		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, rsa_e)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, rsa_n)) != 0)
-			return ret;
-		break;
-#endif /* WITH_OPENSSL */
-	case KEY_ED25519:
-	case KEY_ED25519_SK:
-		if (key->ed25519_pk == NULL)
-			return SSH_ERR_INVALID_ARGUMENT;
-		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_string(b,
-		    key->ed25519_pk, ED25519_PK_SZ)) != 0)
-			return ret;
-		if (type == KEY_ED25519_SK) {
-			if ((ret = sshbuf_put_cstring(b,
-			    key->sk_application)) != 0)
-				return ret;
-		}
-		break;
-#ifdef WITH_XMSS
-	case KEY_XMSS:
-		if (key->xmss_name == NULL || key->xmss_pk == NULL ||
-		    sshkey_xmss_pklen(key) == 0)
-			return SSH_ERR_INVALID_ARGUMENT;
-		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_cstring(b, key->xmss_name)) != 0 ||
-		    (ret = sshbuf_put_string(b,
-		    key->xmss_pk, sshkey_xmss_pklen(key))) != 0 ||
-		    (ret = sshkey_xmss_serialize_pk_info(key, b, opts)) != 0)
-			return ret;
-		break;
-#endif /* WITH_XMSS */
-	default:
-		return SSH_ERR_KEY_TYPE_UNKNOWN;
+		return 0;
 	}
-	return 0;
+	if ((impl = sshkey_impl_from_type(type)) == NULL)
+		return SSH_ERR_KEY_TYPE_UNKNOWN;
+
+	typename = sshkey_ssh_name_from_type_nid(type, key->ecdsa_nid);
+	return impl->funcs->serialize_public(key, b, typename, opts);
 }
 
 int
diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h
index c3fe994e491..87c8005ab18 100644
--- a/usr.bin/ssh/sshkey.h
+++ b/usr.bin/ssh/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.54 2022/10/28 00:36:31 djm Exp $ */
+/* $OpenBSD: sshkey.h,v 1.55 2022/10/28 00:37:24 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -163,6 +163,8 @@ struct sshkey_impl_funcs {
 	int (*alloc)(struct sshkey *);		/* optional */
 	void (*cleanup)(struct sshkey *);	/* optional */
 	int (*equal)(const struct sshkey *, const struct sshkey *);
+	int (*serialize_public)(const struct sshkey *, struct sshbuf *,
+	    const char *, enum sshkey_serialize_rep);
 };
 
 struct sshkey_impl {
@@ -303,6 +305,7 @@ void	 sshkey_sig_details_free(struct sshkey_sig_details *);
 #ifdef SSHKEY_INTERNAL
 int	sshkey_sk_fields_equal(const struct sshkey *a, const struct sshkey *b);
 void	sshkey_sk_cleanup(struct sshkey *k);
+int	sshkey_serialize_sk(const struct sshkey *key, struct sshbuf *b);
 
 int ssh_rsa_sign(const struct sshkey *key,
     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
-- 
2.20.1