Download raw body.
rpki-client: add self-issuance check for EE certs
On Thu, Jun 19, 2025 at 07:54:55AM +0200, Claudio Jeker wrote:
> On Thu, Jun 19, 2025 at 07:49:24AM +0200, Theo Buehler wrote:
> > Next simple step of reworking the extension handling and in particular
> > making checks for EE certs stricter.
> >
> > Tangentially, we never agreed on a better name for x509_get_purpose().
> > Since it does a decent amount of checking, x509_check_purpose() would
> > perhaps be better. This clashes with the related X509_check_purpose()
> > from libcrypto, which I'm sure will confuse me down the road. So I think
> > I want to move that function to cert.c, make it static and call it
> > cert_check_purpose().
>
> OK claudio@, also for the plan to move the function.
That would be this diff for moving, renaming and making it static. I
switched the argument order since that matches what most other functions
do. Nothing else changed.
I think putting the cert first for various x509_* functions is an
artefact from originally having some X509 **x509 arguments first.
That's cleanup for another day.
The extern stuff also isn't ideal but that's for another turd polishing
session on another day, too.
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.162 cert.c
--- cert.c 19 Jun 2025 06:47:57 -0000 1.162
+++ cert.c 19 Jun 2025 08:08:37 -0000
@@ -30,6 +30,7 @@
#include "extern.h"
+extern ASN1_OBJECT *bgpsec_oid; /* id-kp-bgpsec-router Key Purpose */
extern ASN1_OBJECT *certpol_oid; /* id-cp-ipAddr-asNumber cert policy */
extern ASN1_OBJECT *carepo_oid; /* 1.3.6.1.5.5.7.48.5 (caRepository) */
extern ASN1_OBJECT *manifest_oid; /* 1.3.6.1.5.5.7.48.10 (rpkiManifest) */
@@ -737,6 +738,151 @@ cert_check_subject_and_issuer(const char
}
/*
+ * 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.
+ * TAs are self-signed, CAs not self-issued, EEs have no extended key usage,
+ * BGPsec router have id-kp-bgpsec-router OID.
+ */
+static enum cert_purpose
+cert_check_purpose(const char *fn, X509 *x)
+{
+ BASIC_CONSTRAINTS *bc = NULL;
+ EXTENDED_KEY_USAGE *eku = NULL;
+ const X509_EXTENSION *ku;
+ int crit, ext_flags, i, is_ca, ku_idx;
+ enum cert_purpose purpose = CERT_PURPOSE_INVALID;
+
+ if (!x509_cache_extensions(x, fn))
+ goto out;
+
+ ext_flags = X509_get_extension_flags(x);
+
+ /* Key usage must be present and critical. KU bits are checked below. */
+ if ((ku_idx = X509_get_ext_by_NID(x, NID_key_usage, -1)) < 0) {
+ warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
+ goto out;
+ }
+ if ((ku = X509_get_ext(x, ku_idx)) == NULL) {
+ warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
+ goto out;
+ }
+ if (!X509_EXTENSION_get_critical(ku)) {
+ warnx("%s: RFC 6487, section 4.8.4: KeyUsage not critical", fn);
+ goto out;
+ }
+
+ /* This weird API can return 0, 1, 2, 4, 5 but can't error... */
+ if ((is_ca = X509_check_ca(x)) > 1) {
+ if (is_ca == 4)
+ warnx("%s: RFC 6487: sections 4.8.1 and 4.8.4: "
+ "no basic constraints, but keyCertSign set", fn);
+ else
+ warnx("%s: unexpected legacy certificate", fn);
+ goto out;
+ }
+
+ if (is_ca) {
+ bc = X509_get_ext_d2i(x, NID_basic_constraints, &crit, NULL);
+ if (bc == NULL) {
+ if (crit != -1)
+ warnx("%s: RFC 6487 section 4.8.1: "
+ "error parsing basic constraints", fn);
+ else
+ warnx("%s: RFC 6487 section 4.8.1: "
+ "missing basic constraints", fn);
+ goto out;
+ }
+ if (crit != 1) {
+ warnx("%s: RFC 6487 section 4.8.1: Basic Constraints "
+ "must be marked critical", fn);
+ goto out;
+ }
+ if (bc->pathlen != NULL) {
+ warnx("%s: RFC 6487 section 4.8.1: Path Length "
+ "Constraint must be absent", fn);
+ goto out;
+ }
+
+ if (X509_get_key_usage(x) != (KU_KEY_CERT_SIGN | KU_CRL_SIGN)) {
+ warnx("%s: RFC 6487 section 4.8.4: key usage violation",
+ fn);
+ goto out;
+ }
+
+ if (X509_get_extended_key_usage(x) != UINT32_MAX) {
+ warnx("%s: RFC 6487 section 4.8.5: EKU not allowed",
+ fn);
+ goto out;
+ }
+
+ /*
+ * EXFLAG_SI means that issuer and subject are identical.
+ * EXFLAG_SS is SI plus the AKI is absent or matches the SKI.
+ * Thus, exactly the trust anchors should have EXFLAG_SS set
+ * and we should never see EXFLAG_SI without EXFLAG_SS.
+ */
+ if ((ext_flags & EXFLAG_SS) != 0)
+ purpose = CERT_PURPOSE_TA;
+ else if ((ext_flags & EXFLAG_SI) == 0)
+ purpose = CERT_PURPOSE_CA;
+ else
+ warnx("%s: RFC 6487, section 4.8.3: "
+ "self-issued cert with AKI-SKI mismatch", fn);
+ goto out;
+ }
+
+ if ((ext_flags & EXFLAG_BCONS) != 0) {
+ warnx("%s: Basic Constraints ext in non-CA cert", fn);
+ goto out;
+ }
+
+ if ((ext_flags & (EXFLAG_SI | EXFLAG_SS)) != 0) {
+ warnx("%s: EE cert must not be self-issued or self-signed", fn);
+ goto out;
+ }
+
+ if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
+ warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
+ fn);
+ goto out;
+ }
+
+ /*
+ * EKU is only defined for BGPsec Router certs and must be absent from
+ * EE certs.
+ */
+ eku = X509_get_ext_d2i(x, NID_ext_key_usage, &crit, NULL);
+ if (eku == NULL) {
+ if (crit != -1)
+ warnx("%s: error parsing EKU", fn);
+ else
+ purpose = CERT_PURPOSE_EE; /* EKU absent */
+ goto out;
+ }
+ if (crit != 0) {
+ warnx("%s: EKU: extension must not be marked critical", fn);
+ goto out;
+ }
+
+ /*
+ * Per RFC 8209, section 3.1.3.2 the id-kp-bgpsec-router OID must be
+ * present and others are allowed, which we don't need to recognize.
+ * This matches RFC 5280, section 4.2.1.12.
+ */
+ for (i = 0; i < sk_ASN1_OBJECT_num(eku); i++) {
+ if (OBJ_cmp(bgpsec_oid, sk_ASN1_OBJECT_value(eku, i)) == 0) {
+ purpose = CERT_PURPOSE_BGPSEC_ROUTER;
+ break;
+ }
+ }
+
+ out:
+ BASIC_CONSTRAINTS_free(bc);
+ EXTENDED_KEY_USAGE_free(eku);
+ return purpose;
+}
+
+/*
* Lightweight version of cert_parse_pre() for EE certs.
* Parses the two RFC 3779 extensions, and performs some sanity checks.
* Returns cert on success and NULL on failure.
@@ -766,7 +912,7 @@ cert_parse_ee_cert(const char *fn, int t
* Check issuance, basic constraints and (extended) key usage bits are
* appropriate for an EE cert. Covers RFC 6487, 4.8.1, 4.8.4, 4.8.5.
*/
- if ((cert->purpose = x509_get_purpose(x, fn)) != CERT_PURPOSE_EE) {
+ if ((cert->purpose = cert_check_purpose(fn, x)) != CERT_PURPOSE_EE) {
warnx("%s: expected EE cert, got %s", fn,
purpose2str(cert->purpose));
goto out;
@@ -968,7 +1114,7 @@ cert_parse_pre(const char *fn, const uns
goto out;
/* Validation on required fields. */
- cert->purpose = x509_get_purpose(x, fn);
+ cert->purpose = cert_check_purpose(fn, x);
switch (cert->purpose) {
case CERT_PURPOSE_TA:
/* XXX - caller should indicate if it expects TA or CA cert */
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.240 extern.h
--- extern.h 4 Jun 2025 09:18:28 -0000 1.240
+++ extern.h 19 Jun 2025 08:08:37 -0000
@@ -952,7 +952,6 @@ int x509_get_notafter(X509 *, const ch
int x509_get_crl(X509 *, const char *, char **);
char *x509_get_pubkey(X509 *, const char *);
char *x509_pubkey_get_ski(X509_PUBKEY *, const char *);
-enum cert_purpose x509_get_purpose(X509 *, const char *);
int x509_get_time(const ASN1_TIME *, time_t *);
char *x509_convert_seqnum(const char *, const char *,
const ASN1_INTEGER *);
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
diff -u -p -r1.106 x509.c
--- x509.c 19 Jun 2025 06:46:56 -0000 1.106
+++ x509.c 19 Jun 2025 08:08:37 -0000
@@ -266,151 +266,6 @@ x509_get_ski(X509 *x, const char *fn, ch
}
/*
- * 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.
- * TAs are self-signed, CAs not self-issued, EEs have no extended key usage,
- * BGPsec router have id-kp-bgpsec-router OID.
- */
-enum cert_purpose
-x509_get_purpose(X509 *x, const char *fn)
-{
- BASIC_CONSTRAINTS *bc = NULL;
- EXTENDED_KEY_USAGE *eku = NULL;
- const X509_EXTENSION *ku;
- int crit, ext_flags, i, is_ca, ku_idx;
- enum cert_purpose purpose = CERT_PURPOSE_INVALID;
-
- if (!x509_cache_extensions(x, fn))
- goto out;
-
- ext_flags = X509_get_extension_flags(x);
-
- /* Key usage must be present and critical. KU bits are checked below. */
- if ((ku_idx = X509_get_ext_by_NID(x, NID_key_usage, -1)) < 0) {
- warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
- goto out;
- }
- if ((ku = X509_get_ext(x, ku_idx)) == NULL) {
- warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
- goto out;
- }
- if (!X509_EXTENSION_get_critical(ku)) {
- warnx("%s: RFC 6487, section 4.8.4: KeyUsage not critical", fn);
- goto out;
- }
-
- /* This weird API can return 0, 1, 2, 4, 5 but can't error... */
- if ((is_ca = X509_check_ca(x)) > 1) {
- if (is_ca == 4)
- warnx("%s: RFC 6487: sections 4.8.1 and 4.8.4: "
- "no basic constraints, but keyCertSign set", fn);
- else
- warnx("%s: unexpected legacy certificate", fn);
- goto out;
- }
-
- if (is_ca) {
- bc = X509_get_ext_d2i(x, NID_basic_constraints, &crit, NULL);
- if (bc == NULL) {
- if (crit != -1)
- warnx("%s: RFC 6487 section 4.8.1: "
- "error parsing basic constraints", fn);
- else
- warnx("%s: RFC 6487 section 4.8.1: "
- "missing basic constraints", fn);
- goto out;
- }
- if (crit != 1) {
- warnx("%s: RFC 6487 section 4.8.1: Basic Constraints "
- "must be marked critical", fn);
- goto out;
- }
- if (bc->pathlen != NULL) {
- warnx("%s: RFC 6487 section 4.8.1: Path Length "
- "Constraint must be absent", fn);
- goto out;
- }
-
- if (X509_get_key_usage(x) != (KU_KEY_CERT_SIGN | KU_CRL_SIGN)) {
- warnx("%s: RFC 6487 section 4.8.4: key usage violation",
- fn);
- goto out;
- }
-
- if (X509_get_extended_key_usage(x) != UINT32_MAX) {
- warnx("%s: RFC 6487 section 4.8.5: EKU not allowed",
- fn);
- goto out;
- }
-
- /*
- * EXFLAG_SI means that issuer and subject are identical.
- * EXFLAG_SS is SI plus the AKI is absent or matches the SKI.
- * Thus, exactly the trust anchors should have EXFLAG_SS set
- * and we should never see EXFLAG_SI without EXFLAG_SS.
- */
- if ((ext_flags & EXFLAG_SS) != 0)
- purpose = CERT_PURPOSE_TA;
- else if ((ext_flags & EXFLAG_SI) == 0)
- purpose = CERT_PURPOSE_CA;
- else
- warnx("%s: RFC 6487, section 4.8.3: "
- "self-issued cert with AKI-SKI mismatch", fn);
- goto out;
- }
-
- if ((ext_flags & EXFLAG_BCONS) != 0) {
- warnx("%s: Basic Constraints ext in non-CA cert", fn);
- goto out;
- }
-
- if ((ext_flags & (EXFLAG_SI | EXFLAG_SS)) != 0) {
- warnx("%s: EE cert must not be self-issued or self-signed", fn);
- goto out;
- }
-
- if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
- warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
- fn);
- goto out;
- }
-
- /*
- * EKU is only defined for BGPsec Router certs and must be absent from
- * EE certs.
- */
- eku = X509_get_ext_d2i(x, NID_ext_key_usage, &crit, NULL);
- if (eku == NULL) {
- if (crit != -1)
- warnx("%s: error parsing EKU", fn);
- else
- purpose = CERT_PURPOSE_EE; /* EKU absent */
- goto out;
- }
- if (crit != 0) {
- warnx("%s: EKU: extension must not be marked critical", fn);
- goto out;
- }
-
- /*
- * Per RFC 8209, section 3.1.3.2 the id-kp-bgpsec-router OID must be
- * present and others are allowed, which we don't need to recognize.
- * This matches RFC 5280, section 4.2.1.12.
- */
- for (i = 0; i < sk_ASN1_OBJECT_num(eku); i++) {
- if (OBJ_cmp(bgpsec_oid, sk_ASN1_OBJECT_value(eku, i)) == 0) {
- purpose = CERT_PURPOSE_BGPSEC_ROUTER;
- break;
- }
- }
-
- out:
- BASIC_CONSTRAINTS_free(bc);
- EXTENDED_KEY_USAGE_free(eku);
- return purpose;
-}
-
-/*
* Extract Subject Public Key Info (SPKI) from BGPsec X.509 Certificate.
* Returns NULL on failure, on success return the SPKI as base64 encoded pubkey
*/
rpki-client: add self-issuance check for EE certs