Skip to content

Commit

Permalink
Fix issuer subject_hash ambiguities
Browse files Browse the repository at this point in the history
  • Loading branch information
yhabteab committed Sep 10, 2024
1 parent d797b57 commit ab0edff
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
17 changes: 17 additions & 0 deletions application/clicommands/CheckCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -102,6 +111,14 @@ 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();
Expand Down
2 changes: 2 additions & 0 deletions library/X509/Job.php
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,8 @@ 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 !== (int) $lastCertInfo[0]) {
Expand Down

0 comments on commit ab0edff

Please sign in to comment.