Move the notBefore and notAfter checks from proc_parser_root_cert()
authorclaudio <claudio@openbsd.org>
Thu, 20 Jan 2022 16:36:19 +0000 (16:36 +0000)
committerclaudio <claudio@openbsd.org>
Thu, 20 Jan 2022 16:36:19 +0000 (16:36 +0000)
to ta_parse(). This fits better there. Also drop extracting and
printing the x509 subject of the TAs. The subject is more or less
the filename anyway which is already printed.
OK tb@

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

index 808e78f..b2391fb 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.52 2022/01/18 16:52:18 claudio Exp $ */
+/*     $OpenBSD: cert.c,v 1.53 2022/01/20 16:36:19 claudio Exp $ */
 /*
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 char *der, size_t len,
        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 2c35138..2e33838 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: parser.c,v 1.46 2022/01/20 09:24:08 claudio Exp $ */
+/*     $OpenBSD: parser.c,v 1.47 2022/01/20 16:36:19 claudio Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 unsigned char *der, size_t len,
        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 unsigned char *der, size_t len,
        auth_insert(&auths, cert, NULL);
 
        return cert;
-
- badcert:
-       cert_free(cert);
-       return NULL;
 }
 
 /*