Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: remove valid_ca_pubkey()
To:
tech@openbsd.org
Date:
Thu, 10 Jul 2025 22:38:17 +0200

Download raw body.

Thread
  • Theo Buehler:

    rpki-client: remove valid_ca_pubkey()

The short version is that the one remaining call to valid_ca_pkey() is
redundant with the SPKI checks from cert_parse_ee_cert(). The cms.c part
of this diff reverts the code change in cms.c r1.29 and this makes
valid_ca_pkey() unused, so we can remove this API.

The slightly longer version is this:

CMS_verify() hangs each signer (EE) cert's public key off the signerInfo
corresponding to it (via CMS_set1_signers_certs() if you must know),
which we then go and validate via valid_ca_pkey(). Uhm...

While this happens to work, it has a wrong smell to it. With recent
changes cert_parse_ee_cert() validates this key more completely later
on, so it's not only not right but also redundant. All the calls to
cert_parse_ee_cert() after cms_parse_validate() will be merged into a
single call in cms_parse_validate(), so this will become tighter.

The valid_ca_pkey API is unused otherwise, so garbage collect it.

Index: cms.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
diff -u -p -r1.51 cms.c
--- cms.c	26 Feb 2025 08:57:36 -0000	1.51
+++ cms.c	10 Jul 2025 19:26:32 -0000
@@ -102,7 +102,6 @@ cms_parse_validate_internal(X509 **xp, c
 	STACK_OF(X509_CRL)		*crls = NULL;
 	STACK_OF(CMS_SignerInfo)	*sinfos;
 	CMS_SignerInfo			*si;
-	EVP_PKEY			*pkey;
 	X509_ALGOR			*pdig, *psig;
 	int				 i, nattrs, nid;
 	int				 has_ct = 0, has_md = 0, has_st = 0;
@@ -242,9 +241,7 @@ cms_parse_validate_internal(X509 **xp, c
 	}
 
 	/* Check digest and signature algorithms (RFC 7935) */
-	CMS_SignerInfo_get0_algs(si, &pkey, NULL, &pdig, &psig);
-	if (!valid_ca_pkey(fn, pkey))
-		goto out;
+	CMS_SignerInfo_get0_algs(si, NULL, NULL, &pdig, &psig);
 
 	X509_ALGOR_get0(&obj, NULL, NULL, pdig);
 	nid = OBJ_obj2nid(obj);
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.246 extern.h
--- extern.h	10 Jul 2025 19:22:48 -0000	1.246
+++ extern.h	10 Jul 2025 19:26:33 -0000
@@ -801,7 +801,6 @@ int		 valid_econtent_version(const char 
 int		 valid_aspa(const char *, struct cert *, struct aspa *);
 int		 valid_geofeed(const char *, struct cert *, struct geofeed *);
 int		 valid_uuid(const char *);
-int		 valid_ca_pkey(const char *, EVP_PKEY *);
 int		 valid_spl(const char *, struct cert *, struct spl *);
 
 /* Working with CMS. */
Index: validate.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
diff -u -p -r1.78 validate.c
--- validate.c	12 Nov 2024 09:23:07 -0000	1.78
+++ validate.c	10 Jul 2025 19:26:33 -0000
@@ -585,86 +585,3 @@ valid_uuid(const char *s)
 		n++;
 	}
 }
-
-static int
-valid_ca_pkey_rsa(const char *fn, EVP_PKEY *pkey)
-{
-	const RSA	*rsa;
-	const BIGNUM	*rsa_e;
-	int		 key_bits;
-
-	if ((key_bits = EVP_PKEY_bits(pkey)) != 2048) {
-		warnx("%s: RFC 7935: expected 2048-bit modulus, got %d bits",
-		    fn, key_bits);
-		return 0;
-	}
-
-	if ((rsa = EVP_PKEY_get0_RSA(pkey)) == NULL) {
-		warnx("%s: failed to extract RSA public key", fn);
-		return 0;
-	}
-
-	if ((rsa_e = RSA_get0_e(rsa)) == NULL) {
-		warnx("%s: failed to get RSA exponent", fn);
-		return 0;
-	}
-
-	if (!BN_is_word(rsa_e, 65537)) {
-		warnx("%s: incorrect exponent (e) in RSA public key", fn);
-		return 0;
-	}
-
-	return 1;
-}
-
-static int
-valid_ca_pkey_ec(const char *fn, EVP_PKEY *pkey)
-{
-	const EC_KEY	*ec;
-	const EC_GROUP	*group;
-	int		 nid;
-	const char	*cname;
-
-	if ((ec = EVP_PKEY_get0_EC_KEY(pkey)) == NULL) {
-		warnx("%s: failed to extract ECDSA public key", fn);
-		return 0;
-	}
-
-	if ((group = EC_KEY_get0_group(ec)) == NULL) {
-		warnx("%s: EC_KEY_get0_group failed", fn);
-		return 0;
-	}
-
-	nid = EC_GROUP_get_curve_name(group);
-	if (nid != NID_X9_62_prime256v1) {
-		if ((cname = EC_curve_nid2nist(nid)) == NULL)
-			cname = nid2str(nid);
-		warnx("%s: Expected P-256, got %s", fn, cname);
-		return 0;
-	}
-
-	if (!EC_KEY_check_key(ec)) {
-		warnx("%s: EC_KEY_check_key failed", fn);
-		return 0;
-	}
-
-	return 1;
-}
-
-int
-valid_ca_pkey(const char *fn, EVP_PKEY *pkey)
-{
-	if (pkey == NULL) {
-		warnx("%s: failure, pkey is NULL", fn);
-		return 0;
-	}
-
-	if (EVP_PKEY_base_id(pkey) == EVP_PKEY_RSA)
-		return valid_ca_pkey_rsa(fn, pkey);
-
-	if (EVP_PKEY_base_id(pkey) == EVP_PKEY_EC)
-		return valid_ca_pkey_ec(fn, pkey);
-
-	warnx("%s: unsupported public key algorithm", fn);
-	return 0;
-}