Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate only the relevant CA in certificate chain #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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