From: claudio <claudio@openbsd.org>
Date: Thu, 20 Jan 2022 16:36:19 +0000 (+0000)
Subject: Move the notBefore and notAfter checks from proc_parser_root_cert()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4fc676bcf7dfc80749a728247649c500203ca847;p=openbsd

Move the notBefore and notAfter checks from proc_parser_root_cert()
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@
---

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 808e78f7faa..b2391fb3d25 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -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;
diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c
index 2c351383a2f..2e33838368e 100644
--- a/usr.sbin/rpki-client/parser.c
+++ b/usr.sbin/rpki-client/parser.c
@@ -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;
 }
 
 /*