From: Damien Miller <djm@mindrot.org>
Subject: Re: PKCS#11 Ed25519 support
To: Theo Buehler <tb@theobuehler.org>
Cc: openssh@openssh.com, tech@openbsd.org
Date: Fri, 25 Jul 2025 23:27:29 +1000

On Fri, 25 Jul 2025, Theo Buehler wrote:

> On Fri, Jul 25, 2025 at 04:28:20PM +1000, Damien Miller wrote:
> > Hi,
> > 
> > Now that the PKCS#11 refactoring has landed, it's now quite
> > straightforward to teach OpenSSH how to use Ed25519 keys stored in
> > PKCS#11 tokens.
> > 
> > The only wrinkle is that different tokens seems to use different
> > ASN.1 encodings for the public key and its type identifier.
> > This implementation handles the two possibilities I've observed so
> > far: Yubikeys and SoftHSM2.
> > 
> > Note: this requires the pkcs11.h update diff I sent out earlier.
> > 
> > ok?
> 
> Generally looks good. A few comments inline.

Thanks - I think this whacks all of these:

diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c
index 26ee228..1b57a1a 100644
--- a/ssh-pkcs11.c
+++ b/ssh-pkcs11.c
@@ -1092,11 +1092,12 @@ pkcs11_fetch_ed25519_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
 	size_t			len;
 	char			*hex = NULL;
 	int			 success = -1, i;
-	const u_char		 oid1[14] = {
+	/* https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/os/pkcs11-curr-v3.0-os.html#_Toc30061180 */
+	const u_char		 id1[14] = {
 		0x13, 0x0c, 0x65, 0x64, 0x77, 0x61, 0x72, 0x64,
 		0x73, 0x32, 0x35, 0x35, 0x31, 0x39,
 	}; /* PrintableString { "edwards25519" } */
-	const u_char		 oid2[5] = {
+	const u_char		 id1[5] = {
 		0x06, 0x03, 0x2b, 0x65, 0x70,
 	}; /* OBJECT_IDENTIFIER { 1.3.101.112 } */
 
@@ -1139,18 +1140,18 @@ pkcs11_fetch_ed25519_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
 		goto fail;
 	}
 
-	/* Expect one of the supported oids in CKA_EC_PARAMS */
+	/* Expect one of the supported identifiers in CKA_EC_PARAMS */
 	d = (u_char *)key_attr[2].pValue;
 	len = key_attr[2].ulValueLen;
-	if ((len != sizeof(oid1) || memcmp(d, oid1, sizeof(oid1)) != 0) &&
-	    (len != sizeof(oid2) || memcmp(d, oid2, sizeof(oid2)) != 0)) {
+	if ((len != sizeof(id1) || memcmp(d, id1, sizeof(id1)) != 0) &&
+	    (len != sizeof(id2) || memcmp(d, id2, sizeof(id2)) != 0)) {
 		hex = tohex(d, len);
 		logit_f("unsupported CKA_EC_PARAMS: %s (len %zu)", hex, len);
 		goto fail;
 	}
 
 	/*
-	 * Expect an either a raw 32 byte pubkey or an OCTET_STRING with
+	 * Expect either a raw 32 byte pubkey or an OCTET STRING with
 	 * a 32 byte pubkey in CKA_VALUE
 	 */
 	d = (u_char *)key_attr[1].pValue;
@@ -2233,7 +2234,6 @@ pkcs11_destroy_keypair(char *provider_id, char *pin, unsigned long slotidx,
 			*err = rv;
 			goto out;
 		}
-		sshkey_free(k);
 	}
 
 out: