diff --git a/application/clicommands/CheckCommand.php b/application/clicommands/CheckCommand.php index 0c369d9c..75e5d2da 100644 --- a/application/clicommands/CheckCommand.php +++ b/application/clicommands/CheckCommand.php @@ -92,7 +92,16 @@ public function hostAction() $validFrom ->columns([new Expression('MAX(GREATEST(%s, %s))', ['valid_from', 'issuer_certificate.valid_from'])]) ->getSelectBase() + // Reset the where clause generated within the createSubQuery() method. ->resetWhere() + // If the issuer of the current cert is an intermediate one, then we should take its valid_to + // timestamp into account unconditionally, ... + ->where(new Expression("sub_certificate_issuer_certificate.self_signed = 'n'")) + // ... otherwise, since we don't enforce the subject_hash of the certificates to be unambiguous, + // we might have more than one self-singed/trusted CA with the same CN and hash but different validity + // timestamps, and in such situations we need to make sure that we are joining to the correct CA that + // is part of the chain, and not some random one that seems to have the same subject_hash. + ->orWhere(new Expression('sub_certificate_link.certificate_id = sub_certificate_issuer_certificate.id')) ->where(new Expression('sub_certificate_link.certificate_chain_id = target_chain.id')); // Sub query for `valid_to` column @@ -102,14 +111,30 @@ public function hostAction() ->getSelectBase() // Reset the where clause generated within the createSubQuery() method. ->resetWhere() + // If the issuer of the current cert is an intermediate one, then we should take its valid_to + // timestamp into account unconditionally, ... + ->where(new Expression("sub_certificate_issuer_certificate.self_signed = 'n'")) + // ... otherwise, since we don't enforce the subject_hash of the certificates to be unambiguous, + // we might have more than one self-singed/trusted CA with the same CN and hash but different validity + // timestamps, and in such situations we need to make sure that we are joining to the correct CA that + // is part of the chain, and not some random one that seems to have the same subject_hash. + ->orWhere(new Expression('sub_certificate_link.certificate_id = sub_certificate_issuer_certificate.id')) ->where(new Expression('sub_certificate_link.certificate_chain_id = target_chain.id')); list($validFromSelect, $_) = $validFrom->dump(); list($validToSelect, $_) = $validTo->dump(); $targets ->withColumns([ - 'valid_from' => new Expression($validFromSelect), - 'valid_to' => new Expression($validToSelect) + // If the current chains invalid reason contains "unable to get local issuer certificate", those + // valid_{to,from} subqueries will yield null timestamps, since they're using an inner join on the + // issuer, so coalescing it with its actual timestamp not produce bogus outputs like: + // CRITICAL - bogus: unable to get local issuer certificate; bogus has expired since 19976 days etc... + 'valid_from' => new Expression( + sprintf('COALESCE((%s), target_chain_certificate.valid_from)', $validFromSelect) + ), + 'valid_to' => new Expression( + sprintf('COALESCE((%s), target_chain_certificate.valid_to)', $validToSelect) + ) ]) ->getSelectBase() ->where(new Expression('target_chain_link.order = 0')); diff --git a/library/X509/Job.php b/library/X509/Job.php index 1e0b3f73..bebf57bb 100644 --- a/library/X509/Job.php +++ b/library/X509/Job.php @@ -721,9 +721,11 @@ protected function processChain($target, $chain) ->columns(['id']) ->filter(Filter::equal('subject_hash', $lastCertInfo[1])) ->filter(Filter::equal('self_signed', true)) + // If there are multiple CAs with the same subject hash, pick the newly imported one. + ->orderBy('ctime', SORT_DESC) ->first(); - if ($rootCa && $rootCa->id !== $lastCertInfo[0]) { + if ($rootCa && $rootCa->id !== (int) $lastCertInfo[0]) { $this->db->update( 'x509_certificate_chain', ['length' => count($chain) + 1], diff --git a/phpstan-baseline-common.neon b/phpstan-baseline-common.neon index 651eb564..6d8c88ce 100644 --- a/phpstan-baseline-common.neon +++ b/phpstan-baseline-common.neon @@ -105,11 +105,6 @@ parameters: count: 1 path: application/controllers/CertificateController.php - - - message: "#^Parameter \\#1 \\$cert of method Icinga\\\\Module\\\\X509\\\\CertificateDetails\\:\\:setCert\\(\\) expects Icinga\\\\Module\\\\X509\\\\Model\\\\X509Certificate, Icinga\\\\Module\\\\X509\\\\Model\\\\X509Certificate\\|null given\\.$#" - count: 1 - path: application/controllers/CertificateController.php - - message: "#^Parameter \\#2 \\$value of static method ipl\\\\Stdlib\\\\Filter\\:\\:equal\\(\\) expects array\\|bool\\|float\\|int\\|string, mixed given\\.$#" count: 1 @@ -155,31 +150,21 @@ parameters: count: 1 path: application/controllers/ChainController.php - - - message: "#^Cannot access property \\$target on Icinga\\\\Module\\\\X509\\\\Model\\\\X509CertificateChain\\|null\\.$#" - count: 3 - path: application/controllers/ChainController.php - - message: "#^Method Icinga\\\\Module\\\\X509\\\\Controllers\\\\ChainController\\:\\:indexAction\\(\\) has no return type specified\\.$#" count: 1 path: application/controllers/ChainController.php - - message: "#^Offset 'invalid_reason' does not exist on Icinga\\\\Module\\\\X509\\\\Model\\\\X509CertificateChain\\|null\\.$#" - count: 1 + message: "#^Parameter \\#2 \\$value of static method ipl\\\\Stdlib\\\\Filter\\:\\:equal\\(\\) expects array\\|bool\\|float\\|int\\|string, mixed given\\.$#" + count: 2 path: application/controllers/ChainController.php - - message: "#^Offset 'valid' does not exist on Icinga\\\\Module\\\\X509\\\\Model\\\\X509CertificateChain\\|null\\.$#" + message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" count: 1 path: application/controllers/ChainController.php - - - message: "#^Parameter \\#2 \\$value of static method ipl\\\\Stdlib\\\\Filter\\:\\:equal\\(\\) expects array\\|bool\\|float\\|int\\|string, mixed given\\.$#" - count: 2 - path: application/controllers/ChainController.php - - message: "#^Method Icinga\\\\Module\\\\X509\\\\Controllers\\\\ConfigController\\:\\:backendAction\\(\\) has no return type specified\\.$#" count: 1