fix(x509.c): Fixed regression of openssl x509 -checkend return values

Fixes #28928

Also adds functionality to -checkend to account for -multi behaviour.
Man page and unit tests updated accordingly.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/29155)

(cherry picked from commit 679a10110e4e60dbfe8acc87f5c697cebd501876)

diff --git a/apps/x509.c b/apps/x509.c
index c9d26f8b203f90acac3485f78bf5ef38b586e2b5..d8c8dc9ae6c73889d8fed5a6481935771a22da8a 100644 (file)
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -1098,13 +1098,22 @@ int x509_main(int argc, char **argv)
 
     if (checkend) {
         time_t tcheck = time(NULL) + checkoffset;
+        int expired = X509_cmp_time(X509_get0_notAfter(x), &tcheck) < 0;
 
-        ret = X509_cmp_time(X509_get0_notAfter(x), &tcheck) < 0;
-        if (ret)
+        if (expired)
             BIO_printf(out, "Certificate will expire\n");
         else
             BIO_printf(out, "Certificate will not expire\n");
-        goto end_cert_loop;
+
+        if (multi && k > 0)
+            ret |= expired;
+        else
+            ret = expired;
+
+        if (multi && k < sk_X509_num(certs) - 1)
+            goto end_cert_loop;
+        else
+            goto end;
     }
 
     if (!check_cert_attributes(out, x, checkhost, checkemail, checkip, 1))

diff --git a/doc/man1/openssl-x509.pod.in b/doc/man1/openssl-x509.pod.in
index fbe42b203468954aeef2da4447353ae14ef647ee..835a55eddf240fd20f8346cc86f5c0f4482862a5 100644 (file)
--- a/doc/man1/openssl-x509.pod.in
+++ b/doc/man1/openssl-x509.pod.in
@@ -352,8 +352,12 @@ contained in the input.
 
 =item B<-checkend> I<arg>
 
-Checks if the certificate expires within the next I<arg> seconds and exits
-nonzero if yes it will expire or zero if not.
+Without B<-multi> checks if the certificate expires within the next
+I<arg> seconds and exits nonzero if it will expire or zero if not.
+
+With B<-multi> checks if any certificate in the input will expire
+within the next I<arg> seconds and exits nonzero if any will expire
+or zero if none will.
 
 =item B<-checkhost> I<host>
 
@@ -792,6 +796,12 @@ Set a certificate to be trusted for SSL client use and change set its alias to
  openssl x509 -in cert.pem -addtrust clientAuth \
         -setalias "Steve's Class 1 CA" -out trust.pem
 
+Check if any certificates in a chain are due to expire within the next 30 days
+(returns zero if none will expire, nonzero if any will expire):
+
+ openssl x509 -in chain.pem -multi -checkend $[3600*24*30] \
+        && echo 'perform renewal' || echo 'renewal unnecessary'
+
 =head1 NOTES
 
 The conversion to UTF8 format used with the name options assumes that

diff --git a/test/recipes/25-test_x509.t b/test/recipes/25-test_x509.t
index 1b343392aa8f986cde44a275c0eef1c6f7367c98..665ea164c6384d5dec5895cebacaf55f2afb132d 100644 (file)
--- a/test/recipes/25-test_x509.t
+++ b/test/recipes/25-test_x509.t
@@ -17,7 +17,7 @@ use File::Compare qw/compare_text/;
 
 setup("test_x509");
 
-plan tests => 140;
+plan tests => 150;
 
 # Prevent MSys2 filename munging for arguments that look like file paths but
 # aren't
@@ -630,3 +630,75 @@ SKIP: {
 
     ok(run(test(["x509_test", $psscert])), "running x509_test");
 }
+
+# Tests for -checkend including -multi
+# Discussed in https://github.com/openssl/openssl/pull/29155
+
+my $c_early = "c-early.pem";
+my $c_late = "c-late.pem";
+my $c_chain = "c-chain.pem";
+my $c_key = srctop_file(@certs, 'ca-key.pem');
+ok(run(app(["openssl", "x509", "-new", "-key", $c_key, "-subj", "/CN=EARLY",
+            "-extfile", $extfile, "-days", "100", "-text", "-out", $c_early]))
+&& run(app(["openssl", "x509", "-new", "-key", $c_key, "-subj", "/CN=LATE",
+            "-extfile", $extfile, "-days", "200", "-text", "-out", $c_late])));
+my $c_time = Time::Piece->gmtime->epoch;
+my $delta_early = Time::Piece->strptime(
+                    get_field($c_early, "Not After "),
+                       "%b %d %T %Y %Z")->epoch - $c_time;
+my $delta_late = Time::Piece->strptime(
+                    get_field($c_late, "Not After "),
+                        "%b %d %T %Y %Z")->epoch - $c_time;
+sub mkchain {
+    open(my $out, ">:raw", $c_chain) or die;
+    foreach my $fn (@_) {
+        open(my $in, "<:raw", $fn) or die;
+        print {$out} <$in>;
+        close($in);
+    }
+    close($out);
+    return 0;
+}
+# Single + not expiring
+ok(run(app(["openssl", "x509", "-checkend", $delta_early - 3600,
+            "-in", $c_early])),
+    "Single cert + not expiring in -checkend window");
+# Single + expiring
+ok(!run(app(["openssl", "x509", "-checkend", $delta_early + 3600,
+            "-in", $c_early])),
+    "Single cert + expiring in -checkend window");
+# Single + expiring at boundary
+# Test may fail erroneously due to sequential now() calls
+# See https://github.com/openssl/openssl/pull/29155
+my $delta_exact = Time::Piece->strptime( get_field($c_early, "Not After "),
+                    "%b %d %T %Y %Z")->epoch - Time::Piece->gmtime->epoch;
+ok(!run(app(["openssl", "x509", "-checkend", $delta_exact, "-in", $c_early])),
+    "Single cert + expiring at -checkend boundary");
+# Multi + none expiring
+mkchain($c_early, $c_late, $c_late);
+ok(run(app(["openssl", "x509", "-multi", "-checkend",
+            $delta_early - 3600, "-in", $c_chain])),
+    "Multi cert + none expiring in -checkend window");
+# Multi + 1st expiring
+mkchain($c_early, $c_late, $c_late);
+ok(!run(app(["openssl", "x509", "-multi", "-checkend",
+             $delta_early + 3600, "-in", $c_chain])),
+    "Multi cert + 1st expiring in -checkend window");
+# Multi + 2nd expiring
+mkchain($c_late, $c_early, $c_late);
+ok(!run(app(["openssl", "x509", "-multi", "-checkend",
+             $delta_early + 3600, "-in", $c_chain])),
+    "Multi cert + 2nd expiring in -checkend window");
+# Multi + 3rd expiring
+mkchain($c_late, $c_late, $c_early);
+ok(!run(app(["openssl", "x509", "-multi", "-checkend",
+             $delta_late - 3600, "-in", $c_chain])),
+    "Multi cert + 3rd expiring in -checkend window");
+# Multi + all expiring
+mkchain($c_early, $c_late, $c_early);
+ok(!run(app(["openssl", "x509", "-multi", "-checkend",
+             $delta_late + 3600, "-in", $c_chain])),
+    "Multi cert + all expiring in -checkend window");
+# Bad parse still returns non-zero
+ok(!run(app(["openssl", "x509", "-checkend", "60", "-in", $c_key])),
+    "Bad parse with -checkend returns non-zero");