Skip to content

Commit d79568c

Browse files
Mihkel Kivisildmrts
Mihkel Kivisild
authored and
mrts
committed
Validate only the relevant CA in certificate chain
WE2-913 Signed-off-by: Mihkel Kivisild [email protected]
1 parent b6a1943 commit d79568c

File tree

10 files changed

+63
-76
lines changed

10 files changed

+63
-76
lines changed

‎src/certificate/CertificateValidator.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateExpiredException;
3434
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateNotYetValidException;
3535
use web_eid\web_eid_authtoken_validation_php\exceptions\CertificateNotTrustedException;
36+
use web_eid\web_eid_authtoken_validation_php\util\DefaultClock;
3637

3738
final class CertificateValidator
3839
{
@@ -59,18 +60,17 @@ public static function certificateIsValidOnDate(X509 $subjectCertificate, DateTi
5960
}
6061
}
6162

62-
public static function trustedCACertificatesAreValidOnDate(TrustedCertificates $trustedCertificates, DateTime $date): void
63-
{
64-
foreach ($trustedCertificates->getCertificates() as $cert) {
65-
self::certificateIsValidOnDate($cert, $date, "Trusted CA");
66-
}
67-
}
68-
6963
/**
7064
* @copyright 2022 Petr Muzikant [email protected]
7165
*/
72-
public static function validateIsSignedByTrustedCA(X509 $certificate, TrustedCertificates $trustedCertificates): X509
66+
public static function validateIsValidAndSignedByTrustedCA(
67+
X509 $certificate,
68+
TrustedCertificates $trustedCertificates,
69+
): X509
7370
{
71+
$now = DefaultClock::getInstance()->now();
72+
self::certificateIsValidOnDate($certificate, $now, "User");
73+
7474
foreach ($trustedCertificates->getCertificates() as $trustedCertificate) {
7575
$certificate->loadCA(
7676
$trustedCertificate->saveX509($trustedCertificate->getCurrentCert(), X509::FORMAT_PEM)
@@ -79,7 +79,11 @@ public static function validateIsSignedByTrustedCA(X509 $certificate, TrustedCer
7979

8080
if ($certificate->validateSignature()) {
8181
$chain = $certificate->getChain();
82-
return end($chain);
82+
$trustedCACert = end($chain);
83+
84+
// Verify that the trusted CA cert is presently valid before returning the result.
85+
self::certificateIsValidOnDate($trustedCACert, $now, "Trusted CA");
86+
return $trustedCACert;
8387
}
8488

8589
throw new CertificateNotTrustedException($certificate);

‎src/exceptions/CertificateNotYetValidException.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ class CertificateNotYetValidException extends AuthTokenException
3131
{
3232
public function __construct(string $subject)
3333
{
34-
parent::__construct($subject . " certificate is not valid yet");
34+
parent::__construct($subject . " certificate is not yet valid");
3535
}
3636
}

‎src/validator/AuthTokenValidatorImpl.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public function __construct(AuthTokenValidationConfiguration $configuration, Log
7373
$this->trustedCACertificates = CertificateValidator::buildTrustFromCertificates($configuration->getTrustedCACertificates());
7474

7575
$this->simpleSubjectCertificateValidators = new SubjectCertificateValidatorBatch(
76-
new SubjectCertificateExpiryValidator($this->trustedCACertificates, $logger),
7776
new SubjectCertificatePurposeValidator($logger),
7877
new SubjectCertificatePolicyValidator($this->configuration->getDisallowedSubjectCertificatePolicies(), $logger)
7978
);

‎src/validator/certvalidators/SubjectCertificateExpiryValidator.php‎

Lines changed: 0 additions & 53 deletions
This file was deleted.

‎src/validator/certvalidators/SubjectCertificateTrustedValidator.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use phpseclib3\File\X509;
2929
use web_eid\web_eid_authtoken_validation_php\certificate\CertificateValidator;
3030
use Psr\Log\LoggerInterface;
31+
use web_eid\web_eid_authtoken_validation_php\util\DefaultClock;
3132

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

4445
public function validate(X509 $subjectCertificate): void
4546
{
46-
$this->subjectCertificateIssuerCertificate = CertificateValidator::validateIsSignedByTrustedCA(
47+
$this->subjectCertificateIssuerCertificate = CertificateValidator::validateIsValidAndSignedByTrustedCA(
4748
$subjectCertificate,
4849
$this->trustedCACertificates
4950
);
5051

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

5455
public function getSubjectCertificateIssuerCertificate(): X509

‎src/validator/ocsp/service/AiaOcspService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function validateResponderCertificate(X509 $cert, DateTime $producedAt):
6868
CertificateValidator::certificateIsValidOnDate($cert, $producedAt, "AIA OCSP responder");
6969
// Trusted certificates' validity has been already verified in validateCertificateExpiry().
7070
OcspResponseValidator::validateHasSigningExtension($cert);
71-
CertificateValidator::validateIsSignedByTrustedCA($cert, $this->trustedCACertificates);
71+
CertificateValidator::validateIsValidAndSignedByTrustedCA($cert, $this->trustedCACertificates);
7272
}
7373

7474
private static function getOcspAiaUrlFromCertificate(X509 $certificate): Uri

‎tests/certificate/CertificateValidatorTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function testWhenCertificateDateValid(): void
4242
public function testWhenCertificateNotValidYet(): void
4343
{
4444
$this->expectException(CertificateNotYetValidException::class);
45-
$this->expectExceptionMessage("User certificate is not valid yet");
45+
$this->expectExceptionMessage("User certificate is not yet valid");
4646

4747
$cert = Certificates::getJaakKristjanEsteid2018Cert();
4848
$this->assertNull(CertificateValidator::certificateIsValidOnDate($cert, new DateTime("20.01.2000 16:00:00"), "User"));

‎tests/testutil/AuthTokenValidators.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ private static function getAuthTokenValidatorBuilder(string $uri, array $certifi
6565
->withTrustedCertificateAuthorities(...$certificates);
6666
}
6767

68+
public static function getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA(): AuthTokenValidator
69+
{
70+
return self::getAuthTokenValidator(
71+
self::TOKEN_ORIGIN_URL,
72+
...CertificateLoader::loadCertificatesFromResources(
73+
__DIR__ . "/../_resources/TEST_of_ESTEID2018.cer",
74+
__DIR__ . "/../_resources/TEST_of_SK_OCSP_RESPONDER_2020.cer"
75+
)
76+
);
77+
}
78+
6879
public static function getAuthTokenValidatorWithDisallowedESTEIDPolicy(): AuthTokenValidator
6980
{
7081
return (self::getAuthTokenValidatorBuilder(self::TOKEN_ORIGIN_URL, self::getCACertificates()))

‎tests/validator/AuthTokenCertificateTest.php‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,18 @@ public function testWhenUserCertificateIsNotYetValidThenValidationFails()
206206
$this->mockDate("2018-10-17");
207207

208208
$this->expectException(CertificateNotYetValidException::class);
209+
$this->expectExceptionMessage("User certificate is not yet valid");
210+
209211
$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
210212
}
211213

212-
public function testWhenTrustedCaCertificateIsNotYetValidThenValidationFails()
214+
public function testWhenTrustedCaCertificateIsNotYetValidThenUserCertValidationFails()
213215
{
214216
$this->mockDate("2018-08-17");
215217

216218
$this->expectException(CertificateNotYetValidException::class);
219+
$this->expectExceptionMessage("User certificate is not yet valid");
220+
217221
$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
218222
}
219223

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

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

233238
$this->expectException(CertificateExpiredException::class);
234-
$this->expectExceptionMessage("Trusted CA certificate has expired");
239+
$this->expectExceptionMessage("User certificate has expired");
235240
$this->validator->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
236241
}
237242

243+
public function testWhenUnrelatedCACertificateIsExpiredThenValidationSucceeds(): void
244+
{
245+
$this->mockDate("2024-07-01");
246+
247+
$this->expectNotToPerformAssertions();
248+
$validatorWithExpiredUnrelatedTrustedCA = AuthTokenValidators::getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA();
249+
250+
$validatorWithExpiredUnrelatedTrustedCA->validate($this->validAuthToken, self::VALID_CHALLENGE_NONCE);
251+
}
252+
238253
public function testWhenCertificateIsRevokedThenOcspCheckFails(): void
239254
{
255+
$this->markTestSkipped("A new designated test OCSP responder certificate was issued whose validity period no longer overlaps with the revoked certificate");
240256
$this->mockDate("2020-01-01");
241257
$validatorWithOcspCheck = AuthTokenValidators::getAuthTokenValidatorWithOcspCheck();
242258
$token = $this->replaceTokenField(self::AUTH_TOKEN, "unverifiedCertificate", self::REVOKED_CERT);

‎tests/validator/certvalidators/SubjectCertificateNotRevokedValidatorTest.php‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,20 @@
2424

2525
namespace web_eid\web_eid_authtoken_validation_php\validator\certvalidators;
2626

27+
use DateTime;
28+
use ReflectionProperty;
2729
use phpseclib3\File\X509;
2830
use PHPUnit\Framework\TestCase;
29-
use ReflectionProperty;
3031
use web_eid\ocsp_php\OcspResponse;
3132
use web_eid\ocsp_php\util\AsnUtil;
32-
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateOCSPCheckFailedException;
33-
use web_eid\web_eid_authtoken_validation_php\testutil\Certificates;
33+
use web_eid\web_eid_authtoken_validation_php\testutil\Dates;
3434
use web_eid\web_eid_authtoken_validation_php\testutil\Logger;
35-
use web_eid\web_eid_authtoken_validation_php\testutil\OcspServiceMaker;
35+
use web_eid\web_eid_authtoken_validation_php\testutil\Certificates;
3636
use web_eid\web_eid_authtoken_validation_php\util\TrustedCertificates;
37+
use web_eid\web_eid_authtoken_validation_php\testutil\OcspServiceMaker;
3738
use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspClient;
3839
use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspClientImpl;
40+
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateOCSPCheckFailedException;
3941

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

61+
protected function tearDown(): void
62+
{
63+
Dates::resetMockedCertificateValidatorDate();
64+
}
65+
5966
public function testWhenValidAiaOcspResponderConfigurationThenSucceeds(): void
6067
{
6168
$this->expectNotToPerformAssertions();
@@ -99,7 +106,7 @@ public function testWhenOcspRequestFailsThenThrows(): void
99106
$this->expectException(UserCertificateOCSPCheckFailedException::class);
100107
$this->expectExceptionMessage("User certificate revocation check has failed: The requested URL returned error: 404");
101108

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

215222
public function testWhenOcspResponseCaNotTrustedThenThrows(): void
216223
{
224+
Dates::setMockedCertificateValidatorDate(new DateTime("2021-03-01"));
225+
217226
$this->expectException(UserCertificateOCSPCheckFailedException::class);
218227
$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");
219228

0 commit comments

Comments
 (0)