From: djm <djm@openbsd.org>
Date: Fri, 28 Oct 2022 00:36:31 +0000 (+0000)
Subject: factor out sshkey_equal_public()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=712f5ecfb4f262dbdab7134fa06a9ae936b29f32;p=openbsd

factor out sshkey_equal_public()

feedback/ok markus@
---

diff --git a/usr.bin/ssh/ssh-dss.c b/usr.bin/ssh/ssh-dss.c
index 104222ead86..930f49d327e 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.40 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-dss.c,v 1.41 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -66,6 +66,34 @@ ssh_dss_cleanup(struct sshkey *k)
 	k->dsa = NULL;
 }
 
+static int
+ssh_dss_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	const BIGNUM *dsa_p_a, *dsa_q_a, *dsa_g_a, *dsa_pub_key_a;
+	const BIGNUM *dsa_p_b, *dsa_q_b, *dsa_g_b, *dsa_pub_key_b;
+
+	if (a->dsa == NULL || b->dsa == NULL)
+		return 0;
+	DSA_get0_pqg(a->dsa, &dsa_p_a, &dsa_q_a, &dsa_g_a);
+	DSA_get0_pqg(b->dsa, &dsa_p_b, &dsa_q_b, &dsa_g_b);
+	DSA_get0_key(a->dsa, &dsa_pub_key_a, NULL);
+	DSA_get0_key(b->dsa, &dsa_pub_key_b, NULL);
+	if (dsa_p_a == NULL || dsa_p_b == NULL ||
+	    dsa_q_a == NULL || dsa_q_b == NULL ||
+	    dsa_g_a == NULL || dsa_g_b == NULL ||
+	    dsa_pub_key_a == NULL || dsa_pub_key_b == NULL)
+		return 0;
+	if (BN_cmp(dsa_p_a, dsa_p_b) != 0)
+		return 0;
+	if (BN_cmp(dsa_q_a, dsa_q_b) != 0)
+		return 0;
+	if (BN_cmp(dsa_g_a, dsa_g_b) != 0)
+		return 0;
+	if (BN_cmp(dsa_pub_key_a, dsa_pub_key_b) != 0)
+		return 0;
+	return 1;
+}
+
 int
 ssh_dss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -227,6 +255,7 @@ static const struct sshkey_impl_funcs sshkey_dss_funcs = {
 	/* .size = */		ssh_dss_size,
 	/* .alloc = */		ssh_dss_alloc,
 	/* .cleanup = */	ssh_dss_cleanup,
+	/* .equal = */		ssh_dss_equal,
 };
 
 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 c48e089ea0e..73028c5b43a 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.9 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-ecdsa-sk.c,v 1.10 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -43,14 +43,24 @@
 #define SSHKEY_INTERNAL
 #include "sshkey.h"
 
+/* Reuse some ECDSA internals */
+extern struct sshkey_impl_funcs sshkey_ecdsa_funcs;
+
 static void
 ssh_ecdsa_sk_cleanup(struct sshkey *k)
 {
-	free(k->sk_application);
-	sshbuf_free(k->sk_key_handle);
-	sshbuf_free(k->sk_reserved);
-	EC_KEY_free(k->ecdsa);
-	k->ecdsa = NULL;
+	sshkey_sk_cleanup(k);
+	sshkey_ecdsa_funcs.cleanup(k);
+}
+
+static int
+ssh_ecdsa_sk_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	if (!sshkey_sk_fields_equal(a, b))
+		return 0;
+	if (!sshkey_ecdsa_funcs.equal(a, b))
+		return 0;
+	return 1;
 }
 
 /*
@@ -317,6 +327,7 @@ static const struct sshkey_impl_funcs sshkey_ecdsa_sk_funcs = {
 	/* .size = */		NULL,
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ecdsa_sk_cleanup,
+	/* .equal = */		ssh_ecdsa_sk_equal,
 };
 
 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 661f9c2c110..2a6c5e4ac92 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.17 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-ecdsa.c,v 1.18 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -61,6 +61,27 @@ ssh_ecdsa_cleanup(struct sshkey *k)
 	k->ecdsa = NULL;
 }
 
+static int
+ssh_ecdsa_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	const EC_GROUP *grp_a, *grp_b;
+	const EC_POINT *pub_a, *pub_b;
+
+	if (a->ecdsa == NULL || b->ecdsa == NULL)
+		return 0;
+	if ((grp_a = EC_KEY_get0_group(a->ecdsa)) == NULL ||
+	    (grp_b = EC_KEY_get0_group(b->ecdsa)) == NULL)
+		return 0;
+	if ((pub_a = EC_KEY_get0_public_key(a->ecdsa)) == NULL ||
+	    (pub_b = EC_KEY_get0_public_key(b->ecdsa)) == NULL)
+		return 0;
+	if (EC_GROUP_cmp(grp_a, grp_b, NULL) != 0)
+		return 0;
+	if (EC_POINT_cmp(grp_a, pub_a, pub_b, NULL) != 0)
+		return 0;
+	return 1;
+}
+
 /* ARGSUSED */
 int
 ssh_ecdsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
@@ -213,10 +234,12 @@ ssh_ecdsa_verify(const struct sshkey *key,
 	return ret;
 }
 
-static const struct sshkey_impl_funcs sshkey_ecdsa_funcs = {
+/* NB. not static; used by ECDSA-SK */
+const struct sshkey_impl_funcs sshkey_ecdsa_funcs = {
 	/* .size = */		ssh_ecdsa_size,
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ecdsa_cleanup,
+	/* .equal = */		ssh_ecdsa_equal,
 };
 
 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 ce0631a7185..ba94ec67d43 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.7 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-ed25519-sk.c,v 1.8 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl.  All rights reserved.
  *
@@ -33,16 +33,24 @@
 #include "ssh.h"
 #include "digest.h"
 
+/* Reuse some ED25519 internals */
+extern struct sshkey_impl_funcs sshkey_ed25519_funcs;
+
 static void
 ssh_ed25519_sk_cleanup(struct sshkey *k)
 {
-	free(k->sk_application);
-	sshbuf_free(k->sk_key_handle);
-	sshbuf_free(k->sk_reserved);
-	freezero(k->ed25519_pk, ED25519_PK_SZ);
-	freezero(k->ed25519_sk, ED25519_SK_SZ);
-	k->ed25519_pk = NULL;
-	k->ed25519_sk = NULL;
+	sshkey_sk_cleanup(k);
+	sshkey_ed25519_funcs.cleanup(k);
+}
+
+static int
+ssh_ed25519_sk_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	if (!sshkey_sk_fields_equal(a, b))
+		return 0;
+	if (!sshkey_ed25519_funcs.equal(a, b))
+		return 0;
+	return 1;
 }
 
 int
@@ -176,6 +184,7 @@ static const struct sshkey_impl_funcs sshkey_ed25519_sk_funcs = {
 	/* .size = */		NULL,
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ed25519_sk_cleanup,
+	/* .equal = */		ssh_ed25519_sk_equal,
 };
 
 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 fdc08d18182..f6b90af9d2d 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.11 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-ed25519.c,v 1.12 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2013 Markus Friedl <markus@openbsd.org>
  *
@@ -38,6 +38,16 @@ ssh_ed25519_cleanup(struct sshkey *k)
 	k->ed25519_sk = NULL;
 }
 
+static int
+ssh_ed25519_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	if (a->ed25519_pk == NULL || b->ed25519_pk == NULL)
+		return 0;
+	if (memcmp(a->ed25519_pk, b->ed25519_pk, ED25519_PK_SZ) != 0)
+		return 0;
+	return 1;
+}
+
 int
 ssh_ed25519_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -165,10 +175,12 @@ ssh_ed25519_verify(const struct sshkey *key,
 	return r;
 }
 
-static const struct sshkey_impl_funcs sshkey_ed25519_funcs = {
+/* NB. not static; used by ED25519-SK */
+const struct sshkey_impl_funcs sshkey_ed25519_funcs = {
 	/* .size = */		NULL,
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_ed25519_cleanup,
+	/* .equal = */		ssh_ed25519_equal,
 };
 
 const struct sshkey_impl sshkey_ed25519_impl = {
diff --git a/usr.bin/ssh/ssh-rsa.c b/usr.bin/ssh/ssh-rsa.c
index 6aa3f75f543..5754554aab8 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.69 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: ssh-rsa.c,v 1.70 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
  *
@@ -58,6 +58,27 @@ ssh_rsa_cleanup(struct sshkey *k)
 	k->rsa = NULL;
 }
 
+static int
+ssh_rsa_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	const BIGNUM *rsa_e_a, *rsa_n_a;
+	const BIGNUM *rsa_e_b, *rsa_n_b;
+
+	if (a->rsa == NULL || b->rsa == NULL)
+		return 0;
+	RSA_get0_key(a->rsa, &rsa_n_a, &rsa_e_a, NULL);
+	RSA_get0_key(b->rsa, &rsa_n_b, &rsa_e_b, NULL);
+	if (rsa_e_a == NULL || rsa_e_b == NULL)
+		return 0;
+	if (rsa_n_a == NULL || rsa_n_b == NULL)
+		return 0;
+	if (BN_cmp(rsa_e_a, rsa_e_b) != 0)
+		return 0;
+	if (BN_cmp(rsa_n_a, rsa_n_b) != 0)
+		return 0;
+	return 1;
+}
+
 static const char *
 rsa_hash_alg_ident(int hash_alg)
 {
@@ -470,6 +491,7 @@ static const struct sshkey_impl_funcs sshkey_rsa_funcs = {
 	/* .size = */		ssh_rsa_size,
 	/* .alloc = */		ssh_rsa_alloc,
 	/* .cleanup = */	ssh_rsa_cleanup,
+	/* .equal = */		ssh_rsa_equal,
 };
 
 const struct sshkey_impl sshkey_rsa_impl = {
diff --git a/usr.bin/ssh/ssh-xmss.c b/usr.bin/ssh/ssh-xmss.c
index 5309ba801b2..1936af55cd3 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.6 2022/10/28 00:35:40 djm Exp $*/
+/* $OpenBSD: ssh-xmss.c,v 1.7 2022/10/28 00:36:31 djm Exp $*/
 /*
  * Copyright (c) 2017 Stefan-Lukas Gazdag.
  * Copyright (c) 2017 Markus Friedl.
@@ -48,6 +48,18 @@ ssh_xmss_cleanup(struct sshkey *k)
 	k->xmss_filename = NULL;
 }
 
+static int
+ssh_xmss_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	if (a->xmss_pk == NULL || b->xmss_pk == NULL)
+		return 0;
+	if (sshkey_xmss_pklen(a) != sshkey_xmss_pklen(b))
+		return 0;
+	if (memcmp(a->xmss_pk, b->xmss_pk, sshkey_xmss_pklen(a)) != 0)
+		return 0;
+	return 1;
+}
+
 int
 ssh_xmss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, u_int compat)
@@ -200,6 +212,7 @@ static const struct sshkey_impl_funcs sshkey_xmss_funcs = {
 	/* .size = */		NULL,
 	/* .alloc = */		NULL,
 	/* .cleanup = */	ssh_xmss_cleanup,
+	/* .equal = */		ssh_xmss_equal,
 };
 
 const struct sshkey_impl sshkey_xmss_impl = {
diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index f959debad7a..9cdd7df749b 100644
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.123 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.124 2022/10/28 00:36:31 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -567,6 +567,17 @@ sshkey_new(int type)
 	return k;
 }
 
+/* Frees common FIDO fields */
+void
+sshkey_sk_cleanup(struct sshkey *k)
+{
+	free(k->sk_application);
+	sshbuf_free(k->sk_key_handle);
+	sshbuf_free(k->sk_reserved);
+	k->sk_application = NULL;
+	k->sk_key_handle = k->sk_reserved = NULL;
+}
+
 void
 sshkey_free(struct sshkey *k)
 {
@@ -599,6 +610,17 @@ cert_compare(struct sshkey_cert *a, struct sshkey_cert *b)
 	return 1;
 }
 
+/* Compares FIDO-specific pubkey fields only */
+int
+sshkey_sk_fields_equal(const struct sshkey *a, const struct sshkey *b)
+{
+	if (a->sk_application == NULL || b->sk_application == NULL)
+		return 0;
+	if (strcmp(a->sk_application, b->sk_application) != 0)
+		return 0;
+	return 1;
+}
+
 /*
  * Compare public portions of key only, allowing comparisons between
  * certificates and plain keys too.
@@ -606,82 +628,14 @@ cert_compare(struct sshkey_cert *a, struct sshkey_cert *b)
 int
 sshkey_equal_public(const struct sshkey *a, const struct sshkey *b)
 {
-#ifdef WITH_OPENSSL
-	const BIGNUM *rsa_e_a, *rsa_n_a;
-	const BIGNUM *rsa_e_b, *rsa_n_b;
-	const BIGNUM *dsa_p_a, *dsa_q_a, *dsa_g_a, *dsa_pub_key_a;
-	const BIGNUM *dsa_p_b, *dsa_q_b, *dsa_g_b, *dsa_pub_key_b;
-#endif /* WITH_OPENSSL */
+	const struct sshkey_impl *impl;
 
 	if (a == NULL || b == NULL ||
 	    sshkey_type_plain(a->type) != sshkey_type_plain(b->type))
 		return 0;
-
-	switch (a->type) {
-#ifdef WITH_OPENSSL
-	case KEY_RSA_CERT:
-	case KEY_RSA:
-		if (a->rsa == NULL || b->rsa == NULL)
-			return 0;
-		RSA_get0_key(a->rsa, &rsa_n_a, &rsa_e_a, NULL);
-		RSA_get0_key(b->rsa, &rsa_n_b, &rsa_e_b, NULL);
-		return BN_cmp(rsa_e_a, rsa_e_b) == 0 &&
-		    BN_cmp(rsa_n_a, rsa_n_b) == 0;
-	case KEY_DSA_CERT:
-	case KEY_DSA:
-		if (a->dsa == NULL || b->dsa == NULL)
-			return 0;
-		DSA_get0_pqg(a->dsa, &dsa_p_a, &dsa_q_a, &dsa_g_a);
-		DSA_get0_pqg(b->dsa, &dsa_p_b, &dsa_q_b, &dsa_g_b);
-		DSA_get0_key(a->dsa, &dsa_pub_key_a, NULL);
-		DSA_get0_key(b->dsa, &dsa_pub_key_b, NULL);
-		return BN_cmp(dsa_p_a, dsa_p_b) == 0 &&
-		    BN_cmp(dsa_q_a, dsa_q_b) == 0 &&
-		    BN_cmp(dsa_g_a, dsa_g_b) == 0 &&
-		    BN_cmp(dsa_pub_key_a, dsa_pub_key_b) == 0;
-	case KEY_ECDSA_SK:
-	case KEY_ECDSA_SK_CERT:
-		if (a->sk_application == NULL || b->sk_application == NULL)
-			return 0;
-		if (strcmp(a->sk_application, b->sk_application) != 0)
-			return 0;
-		/* FALLTHROUGH */
-	case KEY_ECDSA_CERT:
-	case KEY_ECDSA:
-		if (a->ecdsa == NULL || b->ecdsa == NULL ||
-		    EC_KEY_get0_public_key(a->ecdsa) == NULL ||
-		    EC_KEY_get0_public_key(b->ecdsa) == NULL)
-			return 0;
-		if (EC_GROUP_cmp(EC_KEY_get0_group(a->ecdsa),
-		    EC_KEY_get0_group(b->ecdsa), NULL) != 0 ||
-		    EC_POINT_cmp(EC_KEY_get0_group(a->ecdsa),
-		    EC_KEY_get0_public_key(a->ecdsa),
-		    EC_KEY_get0_public_key(b->ecdsa), NULL) != 0)
-			return 0;
-		return 1;
-#endif /* WITH_OPENSSL */
-	case KEY_ED25519_SK:
-	case KEY_ED25519_SK_CERT:
-		if (a->sk_application == NULL || b->sk_application == NULL)
-			return 0;
-		if (strcmp(a->sk_application, b->sk_application) != 0)
-			return 0;
-		/* FALLTHROUGH */
-	case KEY_ED25519:
-	case KEY_ED25519_CERT:
-		return a->ed25519_pk != NULL && b->ed25519_pk != NULL &&
-		    memcmp(a->ed25519_pk, b->ed25519_pk, ED25519_PK_SZ) == 0;
-#ifdef WITH_XMSS
-	case KEY_XMSS:
-	case KEY_XMSS_CERT:
-		return a->xmss_pk != NULL && b->xmss_pk != NULL &&
-		    sshkey_xmss_pklen(a) == sshkey_xmss_pklen(b) &&
-		    memcmp(a->xmss_pk, b->xmss_pk, sshkey_xmss_pklen(a)) == 0;
-#endif /* WITH_XMSS */
-	default:
+	if ((impl = sshkey_impl_from_type(a->type)) == NULL)
 		return 0;
-	}
-	/* NOTREACHED */
+	return impl->funcs->equal(a, b);
 }
 
 int
diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h
index 748cd41a254..c3fe994e491 100644
--- a/usr.bin/ssh/sshkey.h
+++ b/usr.bin/ssh/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.53 2022/10/28 00:35:40 djm Exp $ */
+/* $OpenBSD: sshkey.h,v 1.54 2022/10/28 00:36:31 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -162,6 +162,7 @@ struct sshkey_impl_funcs {
 	u_int (*size)(const struct sshkey *);	/* optional */
 	int (*alloc)(struct sshkey *);		/* optional */
 	void (*cleanup)(struct sshkey *);	/* optional */
+	int (*equal)(const struct sshkey *, const struct sshkey *);
 };
 
 struct sshkey_impl {
@@ -300,6 +301,9 @@ int	 sshkey_private_serialize_maxsign(struct sshkey *key,
 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 ssh_rsa_sign(const struct sshkey *key,
     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
     const char *ident);