Skip to content

Commit 57bde5d

Browse files
botovqbotovq
committed
Move x509_get_purpose() to a static cert_check_purpose()
This function is only used in cert.c, this way we can rename it to something saner that isn't confusingly close to X509_check_purpose(). Also switch to a more usual argument order. ok claudio
1 parent cf60773 commit 57bde5d

File tree

3 files changed

+151
-151
lines changed

3 files changed

+151
-151
lines changed

‎usr.sbin/rpki-client/cert.c

Lines changed: 149 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: cert.c,v 1.162 2025/06/19 06:47:57 tb Exp $ */
1+
/* $OpenBSD: cert.c,v 1.163 2025/06/19 10:26:34 tb Exp $ */
22
/*
33
* Copyright (c) 2022 Theo Buehler <[email protected]>
44
* Copyright (c) 2021 Job Snijders <[email protected]>
@@ -30,6 +30,7 @@
3030

3131
#include "extern.h"
3232

33+
extern ASN1_OBJECT *bgpsec_oid; /* id-kp-bgpsec-router Key Purpose */
3334
extern ASN1_OBJECT *certpol_oid; /* id-cp-ipAddr-asNumber cert policy */
3435
extern ASN1_OBJECT *carepo_oid; /* 1.3.6.1.5.5.7.48.5 (caRepository) */
3536
extern ASN1_OBJECT *manifest_oid; /* 1.3.6.1.5.5.7.48.10 (rpkiManifest) */
@@ -736,6 +737,151 @@ cert_check_subject_and_issuer(const char *fn, const X509 *x)
736737
return 1;
737738
}
738739

740+
/*
741+
* Check the cert's purpose: the cA bit in basic constraints distinguishes
742+
* between TA/CA and EE/BGPsec router and the key usage bits must match.
743+
* TAs are self-signed, CAs not self-issued, EEs have no extended key usage,
744+
* BGPsec router have id-kp-bgpsec-router OID.
745+
*/
746+
static enum cert_purpose
747+
cert_check_purpose(const char *fn, X509 *x)
748+
{
749+
BASIC_CONSTRAINTS *bc = NULL;
750+
EXTENDED_KEY_USAGE *eku = NULL;
751+
const X509_EXTENSION *ku;
752+
int crit, ext_flags, i, is_ca, ku_idx;
753+
enum cert_purpose purpose = CERT_PURPOSE_INVALID;
754+
755+
if (!x509_cache_extensions(x, fn))
756+
goto out;
757+
758+
ext_flags = X509_get_extension_flags(x);
759+
760+
/* Key usage must be present and critical. KU bits are checked below. */
761+
if ((ku_idx = X509_get_ext_by_NID(x, NID_key_usage, -1)) < 0) {
762+
warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
763+
goto out;
764+
}
765+
if ((ku = X509_get_ext(x, ku_idx)) == NULL) {
766+
warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
767+
goto out;
768+
}
769+
if (!X509_EXTENSION_get_critical(ku)) {
770+
warnx("%s: RFC 6487, section 4.8.4: KeyUsage not critical", fn);
771+
goto out;
772+
}
773+
774+
/* This weird API can return 0, 1, 2, 4, 5 but can't error... */
775+
if ((is_ca = X509_check_ca(x)) > 1) {
776+
if (is_ca == 4)
777+
warnx("%s: RFC 6487: sections 4.8.1 and 4.8.4: "
778+
"no basic constraints, but keyCertSign set", fn);
779+
else
780+
warnx("%s: unexpected legacy certificate", fn);
781+
goto out;
782+
}
783+
784+
if (is_ca) {
785+
bc = X509_get_ext_d2i(x, NID_basic_constraints, &crit, NULL);
786+
if (bc == NULL) {
787+
if (crit != -1)
788+
warnx("%s: RFC 6487 section 4.8.1: "
789+
"error parsing basic constraints", fn);
790+
else
791+
warnx("%s: RFC 6487 section 4.8.1: "
792+
"missing basic constraints", fn);
793+
goto out;
794+
}
795+
if (crit != 1) {
796+
warnx("%s: RFC 6487 section 4.8.1: Basic Constraints "
797+
"must be marked critical", fn);
798+
goto out;
799+
}
800+
if (bc->pathlen != NULL) {
801+
warnx("%s: RFC 6487 section 4.8.1: Path Length "
802+
"Constraint must be absent", fn);
803+
goto out;
804+
}
805+
806+
if (X509_get_key_usage(x) != (KU_KEY_CERT_SIGN | KU_CRL_SIGN)) {
807+
warnx("%s: RFC 6487 section 4.8.4: key usage violation",
808+
fn);
809+
goto out;
810+
}
811+
812+
if (X509_get_extended_key_usage(x) != UINT32_MAX) {
813+
warnx("%s: RFC 6487 section 4.8.5: EKU not allowed",
814+
fn);
815+
goto out;
816+
}
817+
818+
/*
819+
* EXFLAG_SI means that issuer and subject are identical.
820+
* EXFLAG_SS is SI plus the AKI is absent or matches the SKI.
821+
* Thus, exactly the trust anchors should have EXFLAG_SS set
822+
* and we should never see EXFLAG_SI without EXFLAG_SS.
823+
*/
824+
if ((ext_flags & EXFLAG_SS) != 0)
825+
purpose = CERT_PURPOSE_TA;
826+
else if ((ext_flags & EXFLAG_SI) == 0)
827+
purpose = CERT_PURPOSE_CA;
828+
else
829+
warnx("%s: RFC 6487, section 4.8.3: "
830+
"self-issued cert with AKI-SKI mismatch", fn);
831+
goto out;
832+
}
833+
834+
if ((ext_flags & EXFLAG_BCONS) != 0) {
835+
warnx("%s: Basic Constraints ext in non-CA cert", fn);
836+
goto out;
837+
}
838+
839+
if ((ext_flags & (EXFLAG_SI | EXFLAG_SS)) != 0) {
840+
warnx("%s: EE cert must not be self-issued or self-signed", fn);
841+
goto out;
842+
}
843+
844+
if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
845+
warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
846+
fn);
847+
goto out;
848+
}
849+
850+
/*
851+
* EKU is only defined for BGPsec Router certs and must be absent from
852+
* EE certs.
853+
*/
854+
eku = X509_get_ext_d2i(x, NID_ext_key_usage, &crit, NULL);
855+
if (eku == NULL) {
856+
if (crit != -1)
857+
warnx("%s: error parsing EKU", fn);
858+
else
859+
purpose = CERT_PURPOSE_EE; /* EKU absent */
860+
goto out;
861+
}
862+
if (crit != 0) {
863+
warnx("%s: EKU: extension must not be marked critical", fn);
864+
goto out;
865+
}
866+
867+
/*
868+
* Per RFC 8209, section 3.1.3.2 the id-kp-bgpsec-router OID must be
869+
* present and others are allowed, which we don't need to recognize.
870+
* This matches RFC 5280, section 4.2.1.12.
871+
*/
872+
for (i = 0; i < sk_ASN1_OBJECT_num(eku); i++) {
873+
if (OBJ_cmp(bgpsec_oid, sk_ASN1_OBJECT_value(eku, i)) == 0) {
874+
purpose = CERT_PURPOSE_BGPSEC_ROUTER;
875+
break;
876+
}
877+
}
878+
879+
out:
880+
BASIC_CONSTRAINTS_free(bc);
881+
EXTENDED_KEY_USAGE_free(eku);
882+
return purpose;
883+
}
884+
739885
/*
740886
* Lightweight version of cert_parse_pre() for EE certs.
741887
* Parses the two RFC 3779 extensions, and performs some sanity checks.
@@ -766,7 +912,7 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
766912
* Check issuance, basic constraints and (extended) key usage bits are
767913
* appropriate for an EE cert. Covers RFC 6487, 4.8.1, 4.8.4, 4.8.5.
768914
*/
769-
if ((cert->purpose = x509_get_purpose(x, fn)) != CERT_PURPOSE_EE) {
915+
if ((cert->purpose = cert_check_purpose(fn, x)) != CERT_PURPOSE_EE) {
770916
warnx("%s: expected EE cert, got %s", fn,
771917
purpose2str(cert->purpose));
772918
goto out;
@@ -968,7 +1114,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
9681114
goto out;
9691115

9701116
/* Validation on required fields. */
971-
cert->purpose = x509_get_purpose(x, fn);
1117+
cert->purpose = cert_check_purpose(fn, x);
9721118
switch (cert->purpose) {
9731119
case CERT_PURPOSE_TA:
9741120
/* XXX - caller should indicate if it expects TA or CA cert */

‎usr.sbin/rpki-client/extern.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: extern.h,v 1.240 2025/06/04 09:18:28 claudio Exp $ */
1+
/* $OpenBSD: extern.h,v 1.241 2025/06/19 10:26:34 tb Exp $ */
22
/*
33
* Copyright (c) 2019 Kristaps Dzonsons <[email protected]>
44
*
@@ -952,7 +952,6 @@ int x509_get_notafter(X509 *, const char *, time_t *);
952952
int x509_get_crl(X509 *, const char *, char **);
953953
char *x509_get_pubkey(X509 *, const char *);
954954
char *x509_pubkey_get_ski(X509_PUBKEY *, const char *);
955-
enum cert_purpose x509_get_purpose(X509 *, const char *);
956955
int x509_get_time(const ASN1_TIME *, time_t *);
957956
char *x509_convert_seqnum(const char *, const char *,
958957
const ASN1_INTEGER *);

‎usr.sbin/rpki-client/x509.c

Lines changed: 1 addition & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: x509.c,v 1.107 2025/06/19 08:21:56 job Exp $ */
1+
/* $OpenBSD: x509.c,v 1.108 2025/06/19 10:26:34 tb Exp $ */
22
/*
33
* Copyright (c) 2022 Theo Buehler <[email protected]>
44
* Copyright (c) 2021 Claudio Jeker <[email protected]>
@@ -265,151 +265,6 @@ x509_get_ski(X509 *x, const char *fn, char **ski)
265265
return rc;
266266
}
267267

268-
/*
269-
* Check the cert's purpose: the cA bit in basic constraints distinguishes
270-
* between TA/CA and EE/BGPsec router and the key usage bits must match.
271-
* TAs are self-signed, CAs not self-issued, EEs have no extended key usage,
272-
* BGPsec router have id-kp-bgpsec-router OID.
273-
*/
274-
enum cert_purpose
275-
x509_get_purpose(X509 *x, const char *fn)
276-
{
277-
BASIC_CONSTRAINTS *bc = NULL;
278-
EXTENDED_KEY_USAGE *eku = NULL;
279-
const X509_EXTENSION *ku;
280-
int crit, ext_flags, i, is_ca, ku_idx;
281-
enum cert_purpose purpose = CERT_PURPOSE_INVALID;
282-
283-
if (!x509_cache_extensions(x, fn))
284-
goto out;
285-
286-
ext_flags = X509_get_extension_flags(x);
287-
288-
/* Key usage must be present and critical. KU bits are checked below. */
289-
if ((ku_idx = X509_get_ext_by_NID(x, NID_key_usage, -1)) < 0) {
290-
warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
291-
goto out;
292-
}
293-
if ((ku = X509_get_ext(x, ku_idx)) == NULL) {
294-
warnx("%s: RFC 6487, section 4.8.4: missing KeyUsage", fn);
295-
goto out;
296-
}
297-
if (!X509_EXTENSION_get_critical(ku)) {
298-
warnx("%s: RFC 6487, section 4.8.4: KeyUsage not critical", fn);
299-
goto out;
300-
}
301-
302-
/* This weird API can return 0, 1, 2, 4, 5 but can't error... */
303-
if ((is_ca = X509_check_ca(x)) > 1) {
304-
if (is_ca == 4)
305-
warnx("%s: RFC 6487: sections 4.8.1 and 4.8.4: "
306-
"no basic constraints, but keyCertSign set", fn);
307-
else
308-
warnx("%s: unexpected legacy certificate", fn);
309-
goto out;
310-
}
311-
312-
if (is_ca) {
313-
bc = X509_get_ext_d2i(x, NID_basic_constraints, &crit, NULL);
314-
if (bc == NULL) {
315-
if (crit != -1)
316-
warnx("%s: RFC 6487 section 4.8.1: "
317-
"error parsing basic constraints", fn);
318-
else
319-
warnx("%s: RFC 6487 section 4.8.1: "
320-
"missing basic constraints", fn);
321-
goto out;
322-
}
323-
if (crit != 1) {
324-
warnx("%s: RFC 6487 section 4.8.1: Basic Constraints "
325-
"must be marked critical", fn);
326-
goto out;
327-
}
328-
if (bc->pathlen != NULL) {
329-
warnx("%s: RFC 6487 section 4.8.1: Path Length "
330-
"Constraint must be absent", fn);
331-
goto out;
332-
}
333-
334-
if (X509_get_key_usage(x) != (KU_KEY_CERT_SIGN | KU_CRL_SIGN)) {
335-
warnx("%s: RFC 6487 section 4.8.4: key usage violation",
336-
fn);
337-
goto out;
338-
}
339-
340-
if (X509_get_extended_key_usage(x) != UINT32_MAX) {
341-
warnx("%s: RFC 6487 section 4.8.5: EKU not allowed",
342-
fn);
343-
goto out;
344-
}
345-
346-
/*
347-
* EXFLAG_SI means that issuer and subject are identical.
348-
* EXFLAG_SS is SI plus the AKI is absent or matches the SKI.
349-
* Thus, exactly the trust anchors should have EXFLAG_SS set
350-
* and we should never see EXFLAG_SI without EXFLAG_SS.
351-
*/
352-
if ((ext_flags & EXFLAG_SS) != 0)
353-
purpose = CERT_PURPOSE_TA;
354-
else if ((ext_flags & EXFLAG_SI) == 0)
355-
purpose = CERT_PURPOSE_CA;
356-
else
357-
warnx("%s: RFC 6487, section 4.8.3: "
358-
"self-issued cert with AKI-SKI mismatch", fn);
359-
goto out;
360-
}
361-
362-
if ((ext_flags & EXFLAG_BCONS) != 0) {
363-
warnx("%s: Basic Constraints ext in non-CA cert", fn);
364-
goto out;
365-
}
366-
367-
if ((ext_flags & (EXFLAG_SI | EXFLAG_SS)) != 0) {
368-
warnx("%s: EE cert must not be self-issued or self-signed", fn);
369-
goto out;
370-
}
371-
372-
if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
373-
warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
374-
fn);
375-
goto out;
376-
}
377-
378-
/*
379-
* EKU is only defined for BGPsec Router certs and must be absent from
380-
* EE certs.
381-
*/
382-
eku = X509_get_ext_d2i(x, NID_ext_key_usage, &crit, NULL);
383-
if (eku == NULL) {
384-
if (crit != -1)
385-
warnx("%s: error parsing EKU", fn);
386-
else
387-
purpose = CERT_PURPOSE_EE; /* EKU absent */
388-
goto out;
389-
}
390-
if (crit != 0) {
391-
warnx("%s: EKU: extension must not be marked critical", fn);
392-
goto out;
393-
}
394-
395-
/*
396-
* Per RFC 8209, section 3.1.3.2 the id-kp-bgpsec-router OID must be
397-
* present and others are allowed, which we don't need to recognize.
398-
* This matches RFC 5280, section 4.2.1.12.
399-
*/
400-
for (i = 0; i < sk_ASN1_OBJECT_num(eku); i++) {
401-
if (OBJ_cmp(bgpsec_oid, sk_ASN1_OBJECT_value(eku, i)) == 0) {
402-
purpose = CERT_PURPOSE_BGPSEC_ROUTER;
403-
break;
404-
}
405-
}
406-
407-
out:
408-
BASIC_CONSTRAINTS_free(bc);
409-
EXTENDED_KEY_USAGE_free(eku);
410-
return purpose;
411-
}
412-
413268
/*
414269
* Extract Subject Public Key Info (SPKI) from BGPsec X.509 Certificate.
415270
* Returns NULL on failure, on success return the SPKI as base64 encoded pubkey

0 commit comments

Comments
 (0)