From: job <job@openbsd.org>
Date: Fri, 7 Jun 2024 11:48:05 +0000 (+0000)
Subject: Fine-tune the TA tiebreaker logic
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=93ab8db6af8b51343d56596a7cd9ae7b56dff6e5;p=openbsd

Fine-tune the TA tiebreaker logic

Additional tiebreaker: prefer TA certificates with the narrower validity window

OK tb@
---

diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c
index fec811e472d..50f0ee72465 100644
--- a/usr.sbin/rpki-client/parser.c
+++ b/usr.sbin/rpki-client/parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parser.c,v 1.137 2024/06/07 08:33:12 tb Exp $ */
+/*	$OpenBSD: parser.c,v 1.138 2024/06/07 11:48:05 job Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -581,12 +581,15 @@ proc_parser_ta_cmp(const struct cert *cert1, const struct cert *cert2)
 		return 1;
 
 	/*
-	 * Good tie breakers are not obvious. While RFC 6487 and other sources
-	 * advise against backdating, it's explicitly allowed and some TAs do.
-	 * Some TAs have also re-issued with new dates and old serialNumber.
-	 * Ignore the notAfter as hopefully future best practices will encourage
-	 * significant reduction of some egregiously long TA validity periods
-	 * and we want to pick such certs up.
+	 * The standards don't specify tiebreakers. While RFC 6487 and other
+	 * sources advise against backdating, it's explicitly allowed and some
+	 * TAs do. Some TAs have also re-issued with new dates and old
+	 * serialNumber.
+	 * Our tiebreaker logic: a more recent notBefore is taken to mean a
+	 * more recent issuance, and thus preferable. Given equal notBefore
+	 * values, prefer the TA cert with the narrower validity window. This
+	 * hopefully encourages TA operators to reduce egregiously long TA
+	 * validity periods.
 	 */
 
 	if (cert1->notbefore < cert2->notbefore)
@@ -594,9 +597,15 @@ proc_parser_ta_cmp(const struct cert *cert1, const struct cert *cert2)
 	if (cert1->notbefore > cert2->notbefore)
 		return 1;
 
+	if (cert1->notafter > cert2->notafter)
+		return -1;
+	if (cert1->notafter < cert2->notafter)
+		return 1;
+
 	/*
 	 * The serialNumber isn't monotonic and some TAs use semi-random ones.
-	 * If the number changed prefer the freshly-fetched cert.
+	 * If the freshly-fetched cert's serial number is different from the
+	 * cached one, prefer the freshly-fetched cert.
 	 */
 
 	serial1 = X509_get0_serialNumber(cert1->x509);