This depends on https://marc.info/?l=openbsd-tech&m=175198368112016&w=2
    
Fully validate the AlgorithmIdentifier in the SPKI for both RSA and EC
keys. We have previously mostly ignored the parameters which in case of
an EC key tell us precisely what it is. Ensure the public key modulus
for RSA is 2048 bit and the exponent 65537. For !BGPsec certs only accept
P-256 when the experimental flag is given. Also take this opportunity to
copy the BRK into the cert.

This improves on checks currently done in x509_get_pubkey() and
valid_ca_pkey(). The former can go already. The latter will be
removed in the next step.

x509_get_pubkey() only warns about compressed point encoding. I made
that an error. First, RFC 8608 explicitly mandates that uncompressed
encoding be used. Second, even if it did not, compressed encoding is
not free and while for P-256 we're in the easiest case p == 3 (mod 4)
for BN_mod_sqrt(), it is still work pushed to all RPs that is way more
expensive than the 32 bytes saved in the transfer.

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 5442bb74665..516c319752a 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1166,6 +1166,152 @@ cert_check_validity_period(const char *fn, struct cert *cert)
 	return 1;
 }
 
+static int
+cert_compliant_rsa_key(const char *fn, struct cert *cert)
+{
+	EVP_PKEY		*pkey;
+	const RSA		*rsa;
+	const BIGNUM		*rsa_n, *rsa_e;
+
+	if ((pkey = X509_get0_pubkey(cert->x509)) == NULL) {
+		warnx("%s: cert without public key", fn);
+		return 0;
+	}
+	if ((rsa = EVP_PKEY_get0_RSA(pkey)) == NULL) {
+		warnx("%s: expected RSA key", fn);
+		return 0;
+	}
+	if ((rsa_n = RSA_get0_n(rsa)) == NULL ||
+	    (rsa_e = RSA_get0_e(rsa)) == NULL) {
+		warnx("%s: missing RSA public key component", fn);
+		return 0;
+	}
+	if (BN_num_bits(rsa_n) != 2048) {
+		warnx("%s: RFC 7935, 3: want 2048-bit RSA modulus, have %d", fn,
+		    BN_num_bits(rsa_n));
+		return 0;
+	}
+	if (!BN_is_word(rsa_e, 65537)) {
+		warnx("%s: RFC 7935, 3: public RSA exponent not %d", fn, 65537);
+		return 0;
+	}
+
+	return 1;
+}
+
+static int
+cert_compliant_ec_key(const char *fn, struct cert *cert)
+{
+	EVP_PKEY		*pkey;
+	const EC_KEY		*ec_key;
+
+	if ((pkey = X509_get0_pubkey(cert->x509)) == NULL) {
+		warnx("%s: cert without public key", fn);
+		return 0;
+	}
+	if ((ec_key = EVP_PKEY_get0_EC_KEY(pkey)) == NULL) {
+		warnx("%s: expected EC key", fn);
+		return 0;
+	}
+	if (EC_KEY_get_conv_form(ec_key) != POINT_CONVERSION_UNCOMPRESSED) {
+		warnx("%s: RFC 8608: 3.1 public key not uncompressed", fn);
+		return 0;
+	}
+	if (!EC_KEY_check_key(ec_key)) {
+		warnx("%s: EC_KEY_check_key failed", fn);
+		return 0;
+	}
+
+	/* Prepare pubkey for the BRK tree - used in the final output dump. */
+	if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) {
+		unsigned char		*der = NULL;
+		int			 der_len;
+
+		if ((der_len = i2d_PUBKEY(pkey, &der)) <= 0) {
+			warnx("%s: i2d_PUBKEY failed", fn);
+			return 0;
+		}
+		if (base64_encode(der, der_len, &cert->pubkey) == -1)
+			errx(1, "base64_encode");
+		free(der);
+	}
+
+	return 1;
+}
+
+static int
+cert_check_spki(const char *fn, struct cert *cert)
+{
+	X509_PUBKEY		*pubkey;
+	X509_ALGOR		*alg = NULL;
+	const ASN1_OBJECT	*aobj = NULL;
+	int			 ptype = 0;
+	const void		*pval = NULL;
+	int			 rc = 0;
+
+	/* Should be called _get0_. It returns a pointer owned by cert->x509. */
+	if ((pubkey = X509_get_X509_PUBKEY(cert->x509)) == NULL) {
+		warnx("%s: RFC 6487, 4.7: certificate without SPKI", fn);
+		goto out;
+	}
+
+	/*
+	 * Excessive initialization above is due to incoherent semantics of
+	 * the following functions, e.g., OpenSSL may or may not set pval.
+	 */
+	if (!X509_PUBKEY_get0_param(NULL, NULL, NULL, &alg, pubkey) ||
+	    alg == NULL) {
+		warnx("%s: RFC 6487, 4.7: no AlgorithmIdentifier in SPKI", fn);
+		goto out;
+	}
+	X509_ALGOR_get0(&aobj, &ptype, &pval, alg);
+
+	switch (cert->purpose) {
+	case CERT_PURPOSE_TA:
+	case CERT_PURPOSE_CA:
+	case CERT_PURPOSE_EE:
+		if (OBJ_obj2nid(aobj) == NID_rsaEncryption) {
+			if (ptype != V_ASN1_NULL || pval != NULL) {
+				warnx("%s: RFC 4055, 1.2, rsaEncryption "
+				    "parameters not NULL", fn);
+				goto out;
+			}
+			if (!cert_compliant_rsa_key(fn, cert))
+				goto out;
+			break;
+		}
+		if (!experimental) {
+			warnx("%s: RFC 7935, 3.1 SPKI not RSAPublicKey", fn);
+			goto out;
+		}
+		/* FALLTHROUGH */
+	case CERT_PURPOSE_BGPSEC_ROUTER:
+		if (OBJ_obj2nid(aobj) == NID_X9_62_id_ecPublicKey) {
+			if (ptype != V_ASN1_OBJECT) {
+				warnx("%s: RFC 5480, 2.1.1, ecPublicKey "
+				    "parameters not namedCurve", fn);
+				goto out;
+			}
+			if (OBJ_obj2nid(pval) != NID_X9_62_prime256v1) {
+				warnx("%s: RFC 8608, 3.1, named curve not "
+				    "P-256", fn);
+				goto out;
+			}
+			if (!cert_compliant_ec_key(fn, cert))
+				goto out;
+			break;
+		}
+		warnx("%s: RFC 8608, 3.1, SPKI not an ecPublicKey", fn);
+		goto out;
+	default:
+		abort();
+	}
+
+	rc = 1;
+ out:
+	return rc;
+}
+
 /*
  * Check the cert's purpose: the cA bit in basic constraints distinguishes
  * between TA/CA and EE/BGPsec router and the key usage bits must match.
@@ -1487,7 +1633,8 @@ cert_parse_internal(const char *fn, X509 *x)
 	if (!cert_check_validity_period(fn, cert))
 		goto out;
 
-	/* XXX - add SPKI here. */
+	if (!cert_check_spki(fn, cert))
+		goto out;
 
 	/*
 	 * Disallowed in RFC 5280, 4.1.2.8. Also, uniqueness of subjects
@@ -1556,7 +1703,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 	const unsigned char	*oder;
 	size_t			 j;
 	X509			*x = NULL;
-	EVP_PKEY		*pkey;
 
 	/* just fail for empty buffers, the warning was printed elsewhere */
 	if (der == NULL)
@@ -1584,13 +1730,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 	case CERT_PURPOSE_TA:
 		/* XXX - caller should indicate if it expects TA or CA cert */
 	case CERT_PURPOSE_CA:
-		if ((pkey = X509_get0_pubkey(x)) == NULL) {
-			warnx("%s: X509_get0_pubkey failed", fn);
-			goto out;
-		}
-		if (!valid_ca_pkey(fn, pkey))
-			goto out;
-
 		if (cert->mft == NULL) {
 			warnx("%s: RFC 6487 section 4.8.8: missing SIA", fn);
 			goto out;
@@ -1601,11 +1740,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		}
 		break;
 	case CERT_PURPOSE_BGPSEC_ROUTER:
-		cert->pubkey = x509_get_pubkey(x, fn);
-		if (cert->pubkey == NULL) {
-			warnx("%s: x509_get_pubkey failed", fn);
-			goto out;
-		}
 		if (cert->num_ips > 0) {
 			warnx("%s: unexpected IP resources in BGPsec cert", fn);
 			goto out;
diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 419b3e3e206..c04cadc03a7 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -959,7 +959,6 @@ int		 x509_get_ski(X509 *, const char *, char **);
 int		 x509_get_notbefore(X509 *, const char *, time_t *);
 int		 x509_get_notafter(X509 *, const char *, time_t *);
 int		 x509_get_crl(X509 *, const char *, char **);
-char		*x509_get_pubkey(X509 *, const char *);
 char		*x509_pubkey_get_ski(X509_PUBKEY *, const char *);
 int		 x509_get_time(const ASN1_TIME *, time_t *);
 char		*x509_convert_seqnum(const char *, const char *,
diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c
index fa09f320509..2f23c33b1d8 100644
--- a/usr.sbin/rpki-client/x509.c
+++ b/usr.sbin/rpki-client/x509.c
@@ -270,80 +270,6 @@ x509_get_ski(X509 *x, const char *fn, char **ski)
 	return rc;
 }
 
-/*
- * Extract Subject Public Key Info (SPKI) from BGPsec X.509 Certificate.
- * Returns NULL on failure, on success return the SPKI as base64 encoded pubkey
- */
-char *
-x509_get_pubkey(X509 *x, const char *fn)
-{
-	EVP_PKEY	*pkey;
-	const EC_KEY	*eckey;
-	const EC_GROUP	*ecg;
-	int		 nid;
-	const char	*cname;
-	uint8_t		*pubkey = NULL;
-	char		*res = NULL;
-	int		 len;
-
-	pkey = X509_get0_pubkey(x);
-	if (pkey == NULL) {
-		warnx("%s: X509_get0_pubkey failed in %s", fn, __func__);
-		goto out;
-	}
-	if (EVP_PKEY_base_id(pkey) != EVP_PKEY_EC) {
-		warnx("%s: Expected EVP_PKEY_EC, got %d", fn,
-		    EVP_PKEY_base_id(pkey));
-		goto out;
-	}
-
-	eckey = EVP_PKEY_get0_EC_KEY(pkey);
-	if (eckey == NULL) {
-		warnx("%s: Incorrect key type", fn);
-		goto out;
-	}
-
-	if ((ecg = EC_KEY_get0_group(eckey)) == NULL) {
-		warnx("%s: EC_KEY_get0_group failed", fn);
-		goto out;
-	}
-
-	if (EC_GROUP_get_asn1_flag(ecg) != OPENSSL_EC_NAMED_CURVE) {
-		warnx("%s: curve encoding issue", fn);
-		goto out;
-	}
-
-	if (EC_GROUP_get_point_conversion_form(ecg) !=
-	    POINT_CONVERSION_UNCOMPRESSED)
-		warnx("%s: unconventional point encoding", fn);
-
-	nid = EC_GROUP_get_curve_name(ecg);
-	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);
-		goto out;
-	}
-
-	if (!EC_KEY_check_key(eckey)) {
-		warnx("%s: EC_KEY_check_key failed in %s", fn, __func__);
-		goto out;
-	}
-
-	len = i2d_PUBKEY(pkey, &pubkey);
-	if (len <= 0) {
-		warnx("%s: i2d_PUBKEY failed in %s", fn, __func__);
-		goto out;
-	}
-
-	if (base64_encode(pubkey, len, &res) == -1)
-		errx(1, "base64_encode failed in %s", __func__);
-
- out:
-	free(pubkey);
-	return res;
-}
-
 /*
  * Compute the SKI of an RSA public key in an X509_PUBKEY using SHA-1.
  * Returns allocated hex-encoded SKI on success, NULL on failure.