[PATCH] ssh-pkcs11: skip EC keys on curves unusable for SSH

[Avinash Duduskar]

pkcs11_fetch_ecdsa_pubkey() and pkcs11_fetch_x509_pubkey() accept any
EC key whose curve OpenSSL recognises and build a KEY_ECDSA sshkey from
it. SSH only defines ECDSA over nistp256, nistp384 and nistp521
(RFC 5656), so a key on any other curve, for example secp256k1 or a
Brainpool curve (both common on PKCS#11 tokens), becomes a key with no
usable type and serialises as "ssh-unknown".

The cause is that sshkey_ecdsa_key_to_nid() returns the raw OpenSSL
curve nid for any named curve, so the existing "nid < 0" guard never
fires for these curves. The file-based key path already rejects them
with sshkey_curve_nid_to_name(nid) == NULL; the PKCS#11 path is missing
the same check.

The effect is not cosmetic. Adding such a token with "ssh-add -s" fails
outright ("communication with agent failed"), so no keys from that
token, including usable nistp256/384/521 keys on the same token, reach
the agent. "ssh-keygen -D" lists the key as "ssh-unknown". The ssh(1)
client is unaffected at authentication time because it filters
identities by PubkeyAcceptedAlgorithms.

Reject EC keys and certificates whose curve has no SSH name, in both
pkcs11_fetch_ecdsa_pubkey() and pkcs11_fetch_x509_pubkey(), matching the
file-based key path, and skip the object so the rest of the token is
still processed. Keys on supported curves are unaffected.

Verified before and after on a Nitrokey HSM 2 (SmartCard-HSM via
OpenSC) and SoftHSM2, with secp256k1 and brainpoolP256r1 objects,
across OpenSSL and LibreSSL on glibc, musl and OpenBSD. The X.509
certificate path was exercised on SoftHSM2 under both OpenSSL and
LibreSSL. The unpatched build leaked the object as "ssh-unknown" and
"ssh-add -s" failed, locking the valid nistp256 key out of the agent;
the patched build skips the object (logged with its curve, for example
"skipping unsupported EC curve in PKCS#11 certificate: secp256k1") and
the nistp256 key loads. The curve handling was also confirmed against
FreeBSD 14.2 base OpenSSL 3.0.15.

Signed-off-by: Avinash Duduskar <avinash.duduskar at gmail.com>
---
 ssh-pkcs11.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

I did not find a prior report of this in the openssh-unix-dev archives or
on bugzilla.mindrot.org.

The X.509 certificate path was exercised on SoftHSM2 across Debian
glibc, Alpine musl, Alpine LibreSSL, OpenBSD and FreeBSD; the hardware
token covered the pubkey path only, since the cert path is
token-agnostic from the openssh side (the card returns the cert bytes
and the curve check runs in openssh).

On FreeBSD the packaged libsofthsm2.so is stripped, so portable's
lib_contains_symbol() nlist(3) check rejects it ("not a PKCS11 library")
before the curve code runs; rebuilding softhsm2 unstripped lets the
before/after run and it matches the other platforms.

The bug is present in OpenBSD -current and in FreeBSD main and releng/15.0
too, so the fix is needed on those trees as well: both PKCS#11 fetch sites
still carry the bare "if (nid < 0)" guard with no curve-name check, and
sshkey_ecdsa_key_to_nid still returns the raw OpenSSL curve nid. The test
machines ran OpenBSD 7.8 and FreeBSD 14.2. (The surrounding code is not
identical across trees, since the portable distribution carries
OPENSSL_HAS_ECC and OPENSSL_HAS_NISTP521 wrappers the OpenBSD tree lacks
and FreeBSD 15.0 vendors an older import, so the patch applies verbatim to
the portable and FreeBSD-main sources but the guard logic the fix keys on
is the same in all of them.)

Self-contained SoftHSM2 reproducer (no hardware needed):

  export SOFTHSM2_CONF=$PWD/sh2.conf
  mkdir -p tokens
  printf 'directories.tokendir = %s/tokens\n' "$PWD" >sh2.conf
  MOD=$(find /usr/lib /usr/lib64 /usr/local/lib -name libsofthsm2.so 2>/dev/null | head -1)
  softhsm2-util --init-token --free --label t --pin 1234 --so-pin 12345678
  # a usable nistp256 key
  pkcs11-tool --module "$MOD" --token-label t --login --pin 1234 \
      --keypairgen --key-type EC:prime256v1 --label good --id 01
  # an unusable secp256k1 key (pubkey path)
  pkcs11-tool --module "$MOD" --token-label t --login --pin 1234 \
      --keypairgen --key-type EC:secp256k1 --label bad-key --id 02
  # an unusable secp256k1 X.509 cert (pkcs11_fetch_x509_pubkey path)
  openssl ecparam -name secp256k1 -genkey -out k.pem
  openssl req -x509 -new -key k.pem -subj /CN=bad-cert -days 1 \
      -outform DER -out bad.der
  pkcs11-tool --module "$MOD" --token-label t --login --pin 1234 \
      --write-object bad.der --type cert --label bad-cert --id 04
  ssh-keygen -D "$MOD"          # unpatched: lists ssh-unknown entries
  ssh-add -s "$MOD"             # unpatched: communication with agent failed


diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c
index 7a7d3b8ea..d965dcb74 100644
--- a/ssh-pkcs11.c
+++ b/ssh-pkcs11.c
@@ -36,6 +36,7 @@
 #include <openssl/ecdsa.h>
 #include <openssl/x509.h>
 #include <openssl/err.h>
+#include <openssl/objects.h>
 #endif
 
 #define CRYPTOKI_COMPAT
@@ -944,8 +945,10 @@ pkcs11_fetch_ecdsa_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
 	}
 
 	nid = sshkey_ecdsa_key_to_nid(ec);
-	if (nid < 0) {
-		error("couldn't get curve nid");
+	if (nid < 0 || sshkey_curve_nid_to_name(nid) == NULL) {
+		const char *cn = nid > 0 ? OBJ_nid2sn(nid) : NULL;
+		error("skipping unsupported EC curve in PKCS#11 key: %s",
+		    cn != NULL ? cn : "unknown");
 		goto fail;
 	}
 
@@ -1330,8 +1333,10 @@ pkcs11_fetch_x509_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
 			goto out;
 		}
 		nid = sshkey_ecdsa_key_to_nid(ec);
-		if (nid < 0) {
-			error("couldn't get curve nid");
+		if (nid < 0 || sshkey_curve_nid_to_name(nid) == NULL) {
+			const char *cn = nid > 0 ? OBJ_nid2sn(nid) : NULL;
+			error("skipping unsupported EC curve in PKCS#11 "
+			    "certificate: %s", cn != NULL ? cn : "unknown");
 			goto out;
 		}
 

base-commit: cf6c0b3b94cdc223f1b8be1ef2d93e993af5d976
-- 
2.54.0