Skip to content

Commit 8f151af

Browse files
Mihkel Kivisildmrts
Mihkel Kivisild
authored and
mrts
committed
fix: use system time in OcspResponseValidator.validateCertificateStatusUpdateTime()
WE2-868 Signed-off-by: Mihkel Kivisild [email protected]
1 parent d79568c commit 8f151af

11 files changed

+266
-58
lines changed

‎README.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,11 @@ The following additional configuration options are available in `AuthTokenValida
298298

299299
- `withNonceDisabledOcspUrls(URI ...$urls)` – adds the given URLs to the list of OCSP responder access location URLs for which the nonce protocol extension will be disabled. Some OCSP responders don't support the nonce extension.
300300

301+
- `withAllowedOcspResponseTimeSkew(int $allowedTimeSkew)` – sets the allowed time skew for OCSP response's `thisUpdate` and `nextUpdate` times to allow discrepancies between the system clock and the OCSP responder's clock or revocation updates that are not published in real time. The default allowed time skew is 15 minutes. The relatively long default is specifically chosen to account for one particular OCSP responder that used CRLs for authoritative revocation info, these CRLs were updated every 15 minutes.
302+
303+
- `withMaxOcspResponseThisUpdateAge(int $maxThisUpdateAge)` – sets the maximum age for the OCSP response's `thisUpdate` time before it is considered too old to rely on. The default maximum age is 2 minutes.
304+
305+
301306
Extended configuration example:
302307

303308
```php

‎src/validator/AuthTokenValidationConfiguration.php‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ final class AuthTokenValidationConfiguration
3939
private ?Uri $siteOrigin = null;
4040
private array $trustedCACertificates = [];
4141
private bool $isUserCertificateRevocationCheckWithOcspEnabled = true;
42-
private int $ocspRequestTimeout = 5;
42+
private int $ocspRequestTimeout = 5; // In seconds
43+
private int $allowedOcspResponseTimeSkew = 15; // In minutes
44+
private int $maxOcspResponseThisUpdateAge = 2; // In minutes
4345
private array $disallowedSubjectCertificatePolicies;
4446
private UriCollection $nonceDisabledOcspUrls;
4547
private ?DesignatedOcspServiceConfiguration $designatedOcspServiceConfiguration = null;
@@ -94,6 +96,26 @@ public function setOcspRequestTimeout(int $ocspRequestTimeout): void
9496
$this->ocspRequestTimeout = $ocspRequestTimeout;
9597
}
9698

99+
public function getAllowedOcspResponseTimeSkew(): int
100+
{
101+
return $this->allowedOcspResponseTimeSkew;
102+
}
103+
104+
public function setAllowedOcspResponseTimeSkew(int $allowedOcspResponseTimeSkew): void
105+
{
106+
$this->allowedOcspResponseTimeSkew = $allowedOcspResponseTimeSkew;
107+
}
108+
109+
public function getMaxOcspResponseThisUpdateAge(): int
110+
{
111+
return $this->maxOcspResponseThisUpdateAge;
112+
}
113+
114+
public function setMaxOcspResponseThisUpdateAge(int $maxOcspResponseThisUpdateAge): void
115+
{
116+
$this->maxOcspResponseThisUpdateAge = $maxOcspResponseThisUpdateAge;
117+
}
118+
97119
public function getDesignatedOcspServiceConfiguration(): ?DesignatedOcspServiceConfiguration
98120
{
99121
return $this->designatedOcspServiceConfiguration;
@@ -132,6 +154,8 @@ public function validate(): void
132154
}
133155

134156
DateAndTime::requirePositiveDuration($this->ocspRequestTimeout, "OCSP request timeout");
157+
DateAndTime::requirePositiveDuration($this->allowedOcspResponseTimeSkew, "Allowed OCSP response time-skew");
158+
DateAndTime::requirePositiveDuration($this->maxOcspResponseThisUpdateAge, "Max OCSP response thisUpdate age");
135159
}
136160

137161
/**

‎src/validator/AuthTokenValidatorBuilder.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,40 @@ public function withOcspRequestTimeout(int $ocspRequestTimeout): AuthTokenValida
126126
$this->logger?->debug("OCSP request timeout set to " . $ocspRequestTimeout);
127127
return $this;
128128
}
129+
130+
/**
131+
* Sets the allowed time skew for OCSP response's thisUpdate and nextUpdate times.
132+
* This parameter is used to allow discrepancies between the system clock and the OCSP responder's clock,
133+
* which may occur due to clock drift, network delays or revocation updates that are not published in real time.
134+
* <p>
135+
* This is an optional configuration parameter, the default is 15 minutes.
136+
* The relatively long default is specifically chosen to account for one particular OCSP responder that used
137+
* CRLs for authoritative revocation info, these CRLs were updated every 15 minutes.
138+
*
139+
* @param integer $allowedTimeSkew the allowed time skew
140+
* @return AuthTokenValidatorBuilder the builder instance for method chaining.
141+
*/
142+
public function withAllowedOcspResponseTimeSkew(int $allowedTimeSkew) : AuthTokenValidatorBuilder
143+
{
144+
$this->configuration->setAllowedOcspResponseTimeSkew($allowedTimeSkew);
145+
$this->logger?->debug("Allowed OCSP response time skew set to " . $allowedTimeSkew);
146+
return $this;
147+
}
148+
149+
/**
150+
* Sets the maximum age of the OCSP response's thisUpdate time before it is considered too old.
151+
* <p>
152+
* This is an optional configuration parameter, the default is 2 minutes.
153+
*
154+
* @param integer $maxThisUpdateAge the maximum age of the OCSP response's thisUpdate time
155+
* @return AuthTokenValidatorBuilder the builder instance for method chaining.
156+
*/
157+
public function withMaxOcspResponseThisUpdateAge(int $maxThisUpdateAge) : AuthTokenValidatorBuilder
158+
{
159+
$this->configuration->setMaxOcspResponseThisUpdateAge($maxThisUpdateAge);
160+
$this->logger?->debug("Maximum OCSP response thisUpdate age set to " . $maxThisUpdateAge);
161+
return $this;
162+
}
129163

130164
/**
131165
* Adds the given URLs to the list of OCSP URLs for which the nonce protocol extension will be disabled.

‎src/validator/AuthTokenValidatorImpl.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,12 @@ private function getCertTrustValidators(): SubjectCertificateValidatorBatch
172172
if ($this->configuration->isUserCertificateRevocationCheckWithOcspEnabled()) {
173173
$validatorBatch->addOptional(
174174
$this->configuration->isUserCertificateRevocationCheckWithOcspEnabled(),
175-
new SubjectCertificateNotRevokedValidator($certTrustedValidator, $this->ocspClient, $this->ocspServiceProvider, $this->logger)
175+
new SubjectCertificateNotRevokedValidator($certTrustedValidator,
176+
$this->ocspClient,
177+
$this->ocspServiceProvider,
178+
$this->configuration->getAllowedOcspResponseTimeSkew(),
179+
$this->configuration->getMaxOcspResponseThisUpdateAge(),
180+
$this->logger)
176181
);
177182
}
178183

‎src/validator/certvalidators/SubjectCertificateNotRevokedValidator.php‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,22 @@ final class SubjectCertificateNotRevokedValidator implements SubjectCertificateV
4545
private SubjectCertificateTrustedValidator $trustValidator;
4646
private OcspClient $ocspClient;
4747
private OcspServiceProvider $ocspServiceProvider;
48-
49-
public function __construct(SubjectCertificateTrustedValidator $trustValidator, OcspClient $ocspClient, OcspServiceProvider $ocspServiceProvider, LoggerInterface $logger = null)
48+
private int $allowedOcspResponseTimeSkew;
49+
private int $maxOcspResponseThisUpdateAge;
50+
51+
public function __construct(SubjectCertificateTrustedValidator $trustValidator,
52+
OcspClient $ocspClient,
53+
OcspServiceProvider $ocspServiceProvider,
54+
int $allowedOcspResponseTimeSkew,
55+
int $maxOcspResponseThisUpdateAge,
56+
LoggerInterface $logger = null)
5057
{
5158
$this->logger = $logger;
5259
$this->trustValidator = $trustValidator;
5360
$this->ocspClient = $ocspClient;
5461
$this->ocspServiceProvider = $ocspServiceProvider;
62+
$this->allowedOcspResponseTimeSkew = $allowedOcspResponseTimeSkew;
63+
$this->maxOcspResponseThisUpdateAge = $maxOcspResponseThisUpdateAge;
5564
}
5665

5766
public function validate(X509 $subjectCertificate): void
@@ -130,15 +139,14 @@ private function verifyOcspResponse(OcspResponse $response, OcspService $ocspSer
130139

131140
$producedAt = $basicResponse->getProducedAt();
132141
$ocspService->validateResponderCertificate($responderCert, $producedAt);
133-
134142
// 5. The time at which the status being indicated is known to be
135143
// correct (thisUpdate) is sufficiently recent.
136144
//
137145
// 6. When available, the time at or before which newer information will
138146
// be available about the status of the certificate (nextUpdate) is
139147
// greater than the current time.
140148

141-
OcspResponseValidator::validateCertificateStatusUpdateTime($basicResponse, $producedAt);
149+
OcspResponseValidator::validateCertificateStatusUpdateTime($basicResponse, $this->allowedOcspResponseTimeSkew, $this->maxOcspResponseThisUpdateAge);
142150

143151
// Now we can accept the signed response as valid and validate the certificate status.
144152
OcspResponseValidator::validateSubjectCertificateStatus($response);

‎src/validator/ocsp/OcspClientImpl.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ public function request(Uri $uri, string $encodedOcspRequest): OcspResponse
7575
$responseJson = json_encode($response->getResponse(), JSON_INVALID_UTF8_IGNORE);
7676
$this->logger?->debug("OCSP response: " . $responseJson);
7777

78+
echo $uri;
79+
7880
if ($info["content_type"] !== self::OCSP_RESPONSE_TYPE) {
7981
throw new UserCertificateOCSPCheckFailedException("OCSP response content type is not " . self::OCSP_RESPONSE_TYPE);
8082
}

‎src/validator/ocsp/OcspResponseValidator.php‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateOCSPCheckFailedException;
3535
use web_eid\web_eid_authtoken_validation_php\exceptions\UserCertificateRevokedException;
3636
use web_eid\web_eid_authtoken_validation_php\util\DateAndTime;
37+
use web_eid\web_eid_authtoken_validation_php\util\DefaultClock;
3738

3839
final class OcspResponseValidator
3940
{
@@ -44,9 +45,7 @@ final class OcspResponseValidator
4445
* https://oidref.com/1.3.6.1.5.5.7.3.9.
4546
*/
4647
private const OCSP_SIGNING = "id-kp-OCSPSigning";
47-
48-
private const ALLOWED_TIME_SKEW = 15;
49-
48+
private const ERROR_PREFIX = "Certificate status update time check failed: ";
5049
public function __construct()
5150
{
5251
throw new BadFunctionCallException("Utility class");
@@ -72,7 +71,7 @@ public static function validateResponseSignature(OcspBasicResponse $basicRespons
7271
}
7372
}
7473

75-
public static function validateCertificateStatusUpdateTime(OcspBasicResponse $basicResponse, DateTime $producedAt): void
74+
public static function validateCertificateStatusUpdateTime(OcspBasicResponse $basicResponse, int $allowedOcspResponseTimeSkew, int $maxOcspResponseThisUpdateAge): void
7675
{
7776
// From RFC 2560, https://www.ietf.org/rfc/rfc2560.txt:
7877
// 4.2.2. Notes on OCSP Responses
@@ -83,20 +82,37 @@ public static function validateCertificateStatusUpdateTime(OcspBasicResponse $ba
8382
// SHOULD be considered unreliable.
8483
// If nextUpdate is not set, the responder is indicating that newer
8584
// revocation information is available all the time.
86-
87-
$notAllowedBefore = (clone $producedAt)->sub(new DateInterval('PT' . self::ALLOWED_TIME_SKEW . 'S'));
88-
$notAllowedAfter = (clone $producedAt)->add(new DateInterval('PT' . self::ALLOWED_TIME_SKEW . 'S'));
85+
$now = DefaultClock::getInstance()->now();
86+
$earliestAcceptableTimeSkew = (clone $now)->sub(new DateInterval('PT' . $allowedOcspResponseTimeSkew . 'M'));
87+
$latestAcceptableTimeSkew = (clone $now)->add(new DateInterval('PT' . $allowedOcspResponseTimeSkew . 'M'));
88+
$minimumValidThisUpdateTime = (clone $now)->sub(new DateInterval('PT' . $maxOcspResponseThisUpdateAge . 'M'));
8989

9090
$thisUpdate = $basicResponse->getThisUpdate();
91+
if ($thisUpdate > $latestAcceptableTimeSkew) {
92+
throw new UserCertificateOCSPCheckFailedException(self::ERROR_PREFIX .
93+
"thisUpdate '" . DateAndTime::toUtcString($thisUpdate) . "' is too far in the future, " .
94+
"latest allowed: '" . DateAndTime::toUtcString($latestAcceptableTimeSkew) . "'");
95+
}
96+
97+
if ($thisUpdate < $minimumValidThisUpdateTime) {
98+
throw new UserCertificateOCSPCheckFailedException(self::ERROR_PREFIX .
99+
"thisUpdate '" . DateAndTime::toUtcString($thisUpdate) . "' is too old, " .
100+
"minimum time allowed: '" . DateAndTime::toUtcString($minimumValidThisUpdateTime) . "'");
101+
}
102+
91103
$nextUpdate = $basicResponse->getNextUpdate();
104+
if (is_null($nextUpdate)) {
105+
return;
106+
}
92107

93-
if ($notAllowedAfter < $thisUpdate || $notAllowedBefore > (!is_null($nextUpdate) ? $nextUpdate : $thisUpdate)) {
108+
if ($nextUpdate < $earliestAcceptableTimeSkew) {
109+
throw new UserCertificateOCSPCheckFailedException(self::ERROR_PREFIX .
110+
"nextUpdate '" . DateAndTime::toUtcString($nextUpdate) . "' is in the past");
111+
}
94112

95-
throw new UserCertificateOCSPCheckFailedException("Certificate status update time check failed: " .
96-
"notAllowedBefore: " . DateAndTime::toUtcString($notAllowedBefore) .
97-
", notAllowedAfter: " . DateAndTime::toUtcString($notAllowedAfter) .
98-
", thisUpdate: " . DateAndTime::toUtcString($thisUpdate) .
99-
", nextUpdate: " . DateAndTime::toUtcString($nextUpdate));
113+
if ($nextUpdate < $thisUpdate) {
114+
throw new UserCertificateOCSPCheckFailedException(self::ERROR_PREFIX .
115+
"nextUpdate '" . DateAndTime::toUtcString($nextUpdate) . "' is before thisUpdate '" . DateAndTime::toUtcString($thisUpdate) . "'");
100116
}
101117
}
102118

‎tests/testutil/AuthTokenValidators.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ public static function getAuthTokenValidatorWithDesignatedOcspCheck()
5858
return (self::getAuthTokenValidatorBuilder(self::TOKEN_ORIGIN_URL, self::getCACertificates()))->withDesignatedOcspServiceConfiguration(OcspServiceMaker::getDesignatedOcspServiceConfiguration())->build();
5959
}
6060

61+
public static function getDefaultAuthTokenValidatorBuilder(): AuthTokenValidatorBuilder
62+
{
63+
return self::getAuthTokenValidatorBuilder(self::TOKEN_ORIGIN_URL, self::getCACertificates());
64+
}
65+
6166
private static function getAuthTokenValidatorBuilder(string $uri, array $certificates): AuthTokenValidatorBuilder
6267
{
6368
return (new AuthTokenValidatorBuilder(new Logger()))

‎tests/validator/AuthTokenValidatorBuilderTest.php‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@
2424

2525
namespace web_eid\web_eid_authtoken_validation_php\validator;
2626

27-
use GuzzleHttp\Psr7\Exception\MalformedUriException;
28-
use PHPUnit\Framework\TestCase;
29-
use InvalidArgumentException;
3027
use GuzzleHttp\Psr7\Uri;
31-
use web_eid\web_eid_authtoken_validation_php\testutil\AuthTokenValidators;
28+
use InvalidArgumentException;
29+
use PHPUnit\Framework\TestCase;
30+
use GuzzleHttp\Psr7\Exception\MalformedUriException;
3231
use web_eid\web_eid_authtoken_validation_php\testutil\Logger;
32+
use web_eid\web_eid_authtoken_validation_php\testutil\AuthTokenValidators;
3333

3434
class AuthTokenValidatorBuilderTest extends TestCase
3535
{
@@ -90,4 +90,28 @@ public function testValidatorOriginNotValidSyntax(): void
9090
$this->expectExceptionMessage("Unable to parse URI: https:///ria.ee");
9191
AuthTokenValidators::getAuthTokenValidator("https:///ria.ee");
9292
}
93+
94+
public function testInvalidOcspResponseTimeSkew() : void
95+
{
96+
$builderWithInvalidResponseTimeSkew = AuthTokenValidators::getDefaultAuthTokenValidatorBuilder()->withAllowedOcspResponseTimeSkew(-1);
97+
$this->expectException(InvalidArgumentException::class);
98+
$this->expectExceptionMessage("Allowed OCSP response time-skew must be greater than zero");
99+
$builderWithInvalidResponseTimeSkew->build();
100+
}
101+
102+
public function testInvalidMaxOcspResponseThisUpdateAge() : void
103+
{
104+
$builderWithInvalidMaxOcspResponseThisUpdateAge = AuthTokenValidators::getDefaultAuthTokenValidatorBuilder()->withMaxOcspResponseThisUpdateAge(0);
105+
$this->expectException(InvalidArgumentException::class);
106+
$this->expectExceptionMessage("Max OCSP response thisUpdate age must be greater than zero");
107+
$builderWithInvalidMaxOcspResponseThisUpdateAge->build();
108+
}
109+
110+
public function testInvalidOcspRequestTimeout() : void
111+
{
112+
$builderWithInvalidOcspRequestTimeout = AuthTokenValidators::getDefaultAuthTokenValidatorBuilder()->withOcspRequestTimeout(-1);
113+
$this->expectException(InvalidArgumentException::class);
114+
$this->expectExceptionMessage("OCSP request timeout must be greater than zero");
115+
$builderWithInvalidOcspRequestTimeout->build();
116+
}
93117
}

0 commit comments

Comments
 (0)