Lets move the time validity checks for TA to cert.c. ta_parse already
checks the pubkey so why not do all validity checks.
While doing that remove the code to extract the subject. All errors print
the filename and the subject itself is just extra information that is less
helpful in the use case of rpki-client.
--
:wq Claudio
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.52
diff -u -p -r1.52 cert.c
--- cert.c 18 Jan 2022 16:52:18 -0000 1.52
+++ cert.c 20 Jan 2022 10:55:52 -0000
@@ -1157,6 +1157,7 @@ struct cert *
ta_parse(const char *fn, const unsigned char *der, size_t len,
const unsigned char *pkey, size_t pkeysz)
{
+ ASN1_TIME *notBefore, *notAfter;
EVP_PKEY *pk = NULL, *opk = NULL;
struct cert *p;
int rc = 0;
@@ -1164,22 +1165,42 @@ ta_parse(const char *fn, const unsigned
if ((p = cert_parse_inner(fn, der, len, 1)) == NULL)
return NULL;
- if (pkey != NULL) {
- pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
- assert(pk != NULL);
-
- if ((opk = X509_get0_pubkey(p->x509)) == NULL)
- cryptowarnx("%s: RFC 6487 (trust anchor): "
- "missing pubkey", fn);
- else if (EVP_PKEY_cmp(pk, opk) != 1)
- cryptowarnx("%s: RFC 6487 (trust anchor): "
- "pubkey does not match TAL pubkey", fn);
- else
- rc = 1;
+ /* first check pubkey against the one from the TAL */
+ pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
+ if (pk == NULL) {
+ cryptowarnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn);
+ goto badcert;
+ }
+ if ((opk = X509_get0_pubkey(p->x509)) == NULL) {
+ cryptowarnx("%s: RFC 6487 (trust anchor): missing pubkey", fn);
+ goto badcert;
+ } else if (EVP_PKEY_cmp(pk, opk) != 1) {
+ cryptowarnx("%s: RFC 6487 (trust anchor): "
+ "pubkey does not match TAL pubkey", fn);
+ goto badcert;
+ }
- EVP_PKEY_free(pk);
+ if ((notBefore = X509_get_notBefore(p->x509)) == NULL) {
+ warnx("%s: certificate has invalid notBefore", fn);
+ goto badcert;
+ }
+ if ((notAfter = X509_get_notAfter(p->x509)) == NULL) {
+ warnx("%s: certificate has invalid notAfter", fn);
+ goto badcert;
}
+ if (X509_cmp_current_time(notBefore) != -1) {
+ warnx("%s: certificate not yet valid", fn);
+ goto badcert;
+ }
+ if (X509_cmp_current_time(notAfter) != 1) {
+ warnx("%s: certificate has expired", fn);
+ goto badcert;
+ }
+
+ rc = 1;
+badcert:
+ EVP_PKEY_free(pk);
if (rc == 0) {
cert_free(p);
p = NULL;
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.46
diff -u -p -r1.46 parser.c
--- parser.c 20 Jan 2022 09:24:08 -0000 1.46
+++ parser.c 20 Jan 2022 10:56:20 -0000
@@ -451,11 +451,7 @@ static struct cert *
proc_parser_root_cert(char *file, const unsigned char *der, size_t len,
unsigned char *pkey, size_t pkeysz, int talid)
{
- char subject[256];
- ASN1_TIME *notBefore, *notAfter;
- X509_NAME *name;
struct cert *cert;
- X509 *x509;
/* Extract certificate data. */
@@ -463,39 +459,10 @@ proc_parser_root_cert(char *file, const
if (cert == NULL)
return NULL;
- x509 = cert->x509;
- if ((name = X509_get_subject_name(x509)) == NULL) {
- warnx("%s Unable to get certificate subject", file);
- goto badcert;
- }
- if (X509_NAME_oneline(name, subject, sizeof(subject)) == NULL) {
- warnx("%s: Unable to parse certificate subject name", file);
- goto badcert;
- }
- if ((notBefore = X509_get_notBefore(x509)) == NULL) {
- warnx("%s: certificate has invalid notBefore, subject='%s'",
- file, subject);
- goto badcert;
- }
- if ((notAfter = X509_get_notAfter(x509)) == NULL) {
- warnx("%s: certificate has invalid notAfter, subject='%s'",
- file, subject);
- goto badcert;
- }
- if (X509_cmp_current_time(notBefore) != -1) {
- warnx("%s: certificate not yet valid, subject='%s'", file,
- subject);
- goto badcert;
- }
- if (X509_cmp_current_time(notAfter) != 1) {
- warnx("%s: certificate has expired, subject='%s'", file,
- subject);
- goto badcert;
- }
if (!valid_ta(file, &auths, cert)) {
- warnx("%s: certificate not a valid ta, subject='%s'",
- file, subject);
- goto badcert;
+ warnx("%s: certificate not a valid ta", file);
+ cert_free(cert);
+ return NULL;
}
cert->talid = talid;
@@ -506,10 +473,6 @@ proc_parser_root_cert(char *file, const
auth_insert(&auths, cert, NULL);
return cert;
-
- badcert:
- cert_free(cert);
- return NULL;
}
/*
