Skip to content

Commit

Permalink
Validate only the relevant CA in certificate chain
Browse files Browse the repository at this point in the history
WE2-913

Signed-off-by: Mihkel Kivisild [email protected]
  • Loading branch information
Mihkel Kivisild committed Aug 26, 2024
1 parent b6a1943 commit d1805de
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 76 deletions.
22 changes: 13 additions & 9 deletions src/certificate/CertificateValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateExpiredException;
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateNotYetValidException;
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateNotTrustedException;
use web_eid\web_eid_authtoken_validation_php\util\DefaultClock;

final class CertificateValidator
{
Expand All @@ -59,18 +60,17 @@ public static function certificateIsValidOnDate(X509 $subjectCertificate, DateTi
}
}

public static function trustedCACertificatesAreValidOnDate(TrustedCertificates $trustedCertificates, DateTime $date): void
{
foreach ($trustedCertificates->getCertificates() as $cert) {
self::certificateIsValidOnDate($cert, $date, "Trusted CA");
}
}

/**
* @copyright 2022 Petr Muzikant [email protected]
*/
public static function validateIsSignedByTrustedCA(X509 $certificate, TrustedCertificates $trustedCertificates): X509
public static function validateIsValidAndSignedByTrustedCA(
X509 $certificate,
TrustedCertificates $trustedCertificates,
): X509
{
$now = DefaultClock::getInstance()->now();
self::certificateIsValidOnDate($certificate, $now, "User");

foreach ($trustedCertificates->getCertificates() as $trustedCertificate) {
$certificate->loadCA(
$trustedCertificate->saveX509($trustedCertificate->getCurrentCert(), X509::FORMAT_PEM)
Expand All @@ -79,7 +79,11 @@ public static function validateIsSignedByTrustedCA(X509 $certificate, TrustedCer

if ($certificate->validateSignature()) {
$chain = $certificate->getChain();
return end($chain);
$trustedCACert = end($chain);

// Verify that the trusted CA cert is presently valid before returning the result.
self::certificateIsValidOnDate($trustedCACert, $now, "Trusted CA");
return $trustedCACert;
}

throw new CertificateNotTrustedException($certificate);
Expand Down
2 changes: 1 addition & 1 deletion src/exceptions/CertificateNotYetValidException.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ class CertificateNotYetValidException extends AuthTokenException
{
public function __construct(string $subject)
{
parent::__construct($subject . " certificate is not valid yet");
parent::__construct($subject . " certificate is not yet valid");
}
}
1 change: 0 additions & 1 deletion src/validator/AuthTokenValidatorImpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ public function __construct(AuthTokenValidationConfiguration $configuration, Log
$this->trustedCACertificates = CertificateValidator::buildTrustFromCertificates($configuration->getTrustedCACertificates());

$this->simpleSubjectCertificateValidators = new SubjectCertificateValidatorBatch(
new SubjectCertificateExpiryValidator($this->trustedCACertificates, $logger),
new SubjectCertificatePurposeValidator($logger),
new SubjectCertificatePolicyValidator($this->configuration->getDisallowedSubjectCertificatePolicies(), $logger)
);
Expand Down
53 changes: 0 additions & 53 deletions src/validator/certvalidators/SubjectCertificateExpiryValidator.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use phpseclib3\File\X509;
use web_eid\web_eid_authtoken_validation_php\certificate\CertificateValidator;
use Psr\Log\LoggerInterface;
use web_eid\web_eid_authtoken_validation_php\util\DefaultClock;

final class SubjectCertificateTrustedValidator implements SubjectCertificateValidator
{
Expand All @@ -43,12 +44,12 @@ public function __construct(TrustedCertificates $trustedCACertificates, LoggerIn

public function validate(X509 $subjectCertificate): void
{
$this->subjectCertificateIssuerCertificate = CertificateValidator::validateIsSignedByTrustedCA(
$this->subjectCertificateIssuerCertificate = CertificateValidator::validateIsValidAndSignedByTrustedCA(
$subjectCertificate,
$this->trustedCACertificates
);

$this->logger?->debug("Subject certificate is signed by a trusted CA");
$this->logger?->debug("Subject certificate is valid and signed by a trusted CA");
}

public function getSubjectCertificateIssuerCertificate(): X509
Expand Down
2 changes: 1 addition & 1 deletion src/validator/ocsp/service/AiaOcspService.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function validateResponderCertificate(X509 $cert, DateTime $producedAt):
CertificateValidator::certificateIsValidOnDate($cert, $producedAt, "AIA OCSP responder");
// Trusted certificates' validity has been already verified in validateCertificateExpiry().
OcspResponseValidator::validateHasSigningExtension($cert);
CertificateValidator::validateIsSignedByTrustedCA($cert, $this->trustedCACertificates);
CertificateValidator::validateIsValidAndSignedByTrustedCA($cert, $this->trustedCACertificates);
}

private static function getOcspAiaUrlFromCertificate(X509 $certificate): Uri
Expand Down
2 changes: 1 addition & 1 deletion tests/certificate/CertificateValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function testWhenCertificateDateValid(): void
public function testWhenCertificateNotValidYet(): void
{
$this->expectException(CertificateNotYetValidException::class);
$this->expectExceptionMessage("User certificate is not valid yet");
$this->expectExceptionMessage("User certificate is not yet valid");

$cert = Certificates::getJaakKristjanEsteid2018Cert();
$this->assertNull(CertificateValidator::certificateIsValidOnDate($cert, new DateTime("20.01.2000 16:00:00"), "User"));
Expand Down
11 changes: 11 additions & 0 deletions tests/testutil/AuthTokenValidators.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ private static function getAuthTokenValidatorBuilder(string $uri, array $certifi
->withTrustedCertificateAuthorities(...$certificates);
}

public static function getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA(): AuthTokenValidator
{
return self::getAuthTokenValidator(
self::TOKEN_ORIGIN_URL,
...CertificateLoader::loadCertificatesFromResources(
__DIR__ . "/../_resources/TEST_of_ESTEID2018.cer",
__DIR__ . "/../_resources/TEST_of_SK_OCSP_RESPONDER_2020.cer"
)
);
}

public static function getAuthTokenValidatorWithDisallowedESTEIDPolicy(): AuthTokenValidator
{
return (self::getAuthTokenValidatorBuilder(self::TOKEN_ORIGIN_URL, self::getCACertificates()))
Expand Down
22 changes: 19 additions & 3 deletions tests/validator/AuthTokenCertificateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,18 @@ public function testWhenUserCertificateIsNotYetValidThenValidationFails()
$this->mockDate("2018-10-17");

$this->expectException(CertificateNotYetValidException::class);
$this->expectExceptionMessage("User certificate is not yet valid");

$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
}

public function testWhenTrustedCaCertificateIsNotYetValidThenValidationFails()
public function testWhenTrustedCaCertificateIsNotYetValidThenUserCertValidationFails()
{
$this->mockDate("2018-08-17");

$this->expectException(CertificateNotYetValidException::class);
$this->expectExceptionMessage("User certificate is not yet valid");

$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
}

Expand All @@ -226,17 +230,29 @@ public function testWhenUserCertificateIsNoLongerValidThenValidationFails()
$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
}

public function testWhenTrustedCaCertificateIsNoLongerValidThenValidationFails()
// In this case both CA and user certificate have expired, we expect the user certificate to be checked first.
public function testWhenTrustedCaCertificateIsNoLongerValidThenUserCertValidationFails()
{
$this->mockDate("2033-10-19");

$this->expectException(CertificateExpiredException::class);
$this->expectExceptionMessage("Trusted CA certificate has expired");
$this->expectExceptionMessage("User certificate has expired");
$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
}

public function testWhenUnrelatedCACertificateIsExpiredThenValidationSucceeds(): void
{
$this->mockDate("2024-07-01");

$this->expectNotToPerformAssertions();
$validatorWithExpiredUnrelatedTrustedCA = AuthTokenValidators::getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA();

$validatorWithExpiredUnrelatedTrustedCA->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
}

public function testWhenCertificateIsRevokedThenOcspCheckFails(): void
{
$this->markTestSkipped("A new designated test OCSP responder certificate was issued whose validity period no longer overlaps with the revoked certificate");
$this->mockDate("2020-01-01");
$validatorWithOcspCheck = AuthTokenValidators::getAuthTokenValidatorWithOcspCheck();
$token = $this->replaceTokenField(self::AUTH_TOKEN, "unverifiedCertificate", self::REVOKED_CERT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,20 @@

namespace web_eid\web_eid_authtoken_validation_php\validator\certvalidators;

use DateTime;
use ReflectionProperty;
use phpseclib3\File\X509;
use PHPUnit\Framework\TestCase;
use ReflectionProperty;
use web_eid\ocsp_php\OcspResponse;
use web_eid\ocsp_php\util\AsnUtil;
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateOCSPCheckFailedException;
use web_eid\web_eid_authtoken_validation_php\testutil\Certificates;
use web_eid\web_eid_authtoken_validation_php\testutil\Dates;
use web_eid\web_eid_authtoken_validation_php\testutil\Logger;
use web_eid\web_eid_authtoken_validation_php\testutil\OcspServiceMaker;
use web_eid\web_eid_authtoken_validation_php\testutil\Certificates;
use web_eid\web_eid_authtoken_validation_php\util\TrustedCertificates;
use web_eid\web_eid_authtoken_validation_php\testutil\OcspServiceMaker;
use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspClient;
use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspClientImpl;
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateOCSPCheckFailedException;

class SubjectCertificateNotRevokedValidatorTest extends TestCase
{
Expand All @@ -56,6 +58,11 @@ protected function setUp(): void
$this->estEid2018Cert = Certificates::getJaakKristjanEsteid2018Cert();
}

protected function tearDown(): void
{
Dates::resetMockedCertificateValidatorDate();
}

public function testWhenValidAiaOcspResponderConfigurationThenSucceeds(): void
{
$this->expectNotToPerformAssertions();
Expand Down Expand Up @@ -99,7 +106,7 @@ public function testWhenOcspRequestFailsThenThrows(): void
$this->expectException(UserCertificateOCSPCheckFailedException::class);
$this->expectExceptionMessage("User certificate revocation check has failed: The requested URL returned error: 404");

$ocspServiceProvider = OcspServiceMaker::getDesignatedOcspServiceProvider(true, "https://web-eid-test.free.beeceptor.com");
$ocspServiceProvider = OcspServiceMaker::getDesignatedOcspServiceProvider(true, "http://demo.sk.ee/ocsps");
$validator = new SubjectCertificateNotRevokedValidator($this->trustedValidator, self::$ocspClient, $ocspServiceProvider);
$validator->validate($this->estEid2018Cert);
}
Expand Down Expand Up @@ -214,6 +221,8 @@ public function request($url, $request): OcspResponse

public function testWhenOcspResponseCaNotTrustedThenThrows(): void
{
Dates::setMockedCertificateValidatorDate(new DateTime("2021-03-01"));

$this->expectException(UserCertificateOCSPCheckFailedException::class);
$this->expectExceptionMessage("User certificate revocation check has failed: Exception: Certificate C=EE, O=AS Sertifitseerimiskeskus, OU=OCSP, CN=TEST of SK OCSP RESPONDER 2020/[email protected] is not trusted");

Expand Down

0 comments on commit d1805de

Please sign in to comment.