From 76dbea150c0563b6033e274997bf1924c23eabd8 Mon Sep 17 00:00:00 2001 From: Mihkel Kivisild Date: Tue, 29 Oct 2024 10:58:20 +0200 Subject: [PATCH 1/6] Add tests for nonces that contain null bytes, control characters, high-byte values or an ASN.1 OID WE2-879 Signed-off-by: Mart Somermaa --- tests/ocsp/OcspRequestTest.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/ocsp/OcspRequestTest.php b/tests/ocsp/OcspRequestTest.php index f37324a..f50c8fb 100644 --- a/tests/ocsp/OcspRequestTest.php +++ b/tests/ocsp/OcspRequestTest.php @@ -101,4 +101,29 @@ public function testWhenGetNonceExtensionSuccess(): void $this->assertEquals("nonce", $request->getNonceExtension()); } + + public function testBinaryNonce(): void + { + // Create a nonce with a mixture of potentially problematic bytes, + // null bytes, control characters and high-byte values (above 0x7F), + // which are common sources of string decoding problems. + $nonce = "\0\1\2\3\4\5\6\7\x08\x09\x10\0" + . "\x0A\x0D\x1B" + . "\xE2\x82\xAC" + . "\xFF"; + $request = new OcspRequest(); + $request->addNonceExtension($nonce); + + $this->assertEquals($nonce, $request->getNonceExtension()); + } + + public function testOidNonce(): void + { + // Create a nonce that contains DER-encoded OID for SHA-256: 2.16.840.1.101.3.4.2.1. + $nonce = "\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x01"; + $request = new OcspRequest(); + $request->addNonceExtension($nonce); + + $this->assertEquals($nonce, $request->getNonceExtension()); + } } From c062aa5ebe3942713be3272e9160e6ad37fb68a4 Mon Sep 17 00:00:00 2001 From: Mihkel Kivisild Date: Tue, 29 Oct 2024 11:09:50 +0200 Subject: [PATCH 2/6] Fix malformed OCSP request: - Remove empty arrays from request - Array will be created when value is added to it - Always having an empty array leads to additional nodes in request that are considered malformed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kai Hölscher 51371415+hoels@users.noreply.github.com Co-authored-by: Mihkel Kivisild --- src/ocsp/OcspRequest.php | 12 ++++-------- tests/ocsp/OcspRequestTest.php | 2 -- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/ocsp/OcspRequest.php b/src/ocsp/OcspRequest.php index 2d7f4ff..df4e264 100644 --- a/src/ocsp/OcspRequest.php +++ b/src/ocsp/OcspRequest.php @@ -32,7 +32,7 @@ class OcspRequest { - private array $ocspRequest = []; + private array $ocspRequest; public function __construct() { @@ -41,8 +41,6 @@ public function __construct() $this->ocspRequest = [ "tbsRequest" => [ "version" => "v1", - "requestList" => [], - "requestExtensions" => [], ], ]; } @@ -62,18 +60,16 @@ public function addNonceExtension(string $nonce): void "critical" => false, "extnValue" => ASN1::encodeDER($nonce, ['type' => ASN1::TYPE_OCTET_STRING]), ]; - $this->ocspRequest["tbsRequest"][ - "requestExtensions" - ][] = $nonceExtension; + $this->ocspRequest["tbsRequest"]["requestExtensions"][] = $nonceExtension; } /** * @copyright 2022 Petr Muzikant pmuzikant@email.cz */ - public function getNonceExtension(): string + public function getNonceExtension(): ?string { // TODO: the ?? '' is here only for v1.0 API compatibility. Remove this in version 1.2 and change the return type to ?string. - return AsnUtil::decodeNonceExtension($this->ocspRequest["tbsRequest"]["requestExtensions"]) ?? ''; + return AsnUtil::decodeNonceExtension($this->ocspRequest["tbsRequest"]["requestExtensions"] ?? []); } public function getEncodeDer(): string diff --git a/tests/ocsp/OcspRequestTest.php b/tests/ocsp/OcspRequestTest.php index f50c8fb..cbcc705 100644 --- a/tests/ocsp/OcspRequestTest.php +++ b/tests/ocsp/OcspRequestTest.php @@ -39,8 +39,6 @@ private function getRequest(): array return [ 'tbsRequest' => [ 'version' => 'v1', - 'requestList' => [], - 'requestExtensions' => [], ], ]; } From 98d4801ad7c66de08e9f1763adf65538185a3a14 Mon Sep 17 00:00:00 2001 From: Mihkel Kivisild Date: Tue, 29 Oct 2024 14:48:22 +0200 Subject: [PATCH 3/6] Harmonizing the PHP library with Java library WE2-971 Signed-off-by: Mihkel Kivisild --- example/src/Auth.php | 9 ++++----- src/authtoken/WebEidAuthToken.php | 13 ------------- src/certificate/CertificateData.php | 7 +++---- src/validator/AuthTokenSignatureValidator.php | 12 ++++++------ src/validator/ocsp/OcspRequestBuilder.php | 4 ++++ src/validator/ocsp/OcspServiceProvider.php | 4 +++- src/validator/ocsp/OcspUrl.php | 4 ++++ src/validator/ocsp/service/AiaOcspService.php | 4 ++++ tests/authtoken/WebEidAuthTokenTest.php | 1 - tests/certificate/CertificateDataTest.php | 14 ++++++++------ tests/validator/AuthTokenAlgorithmTest.php | 4 ++-- 11 files changed, 38 insertions(+), 38 deletions(-) diff --git a/example/src/Auth.php b/example/src/Auth.php index 9d1d2e8..160262e 100644 --- a/example/src/Auth.php +++ b/example/src/Auth.php @@ -88,14 +88,13 @@ public function getNonce() private function getPrincipalNameFromCertificate(X509 $userCertificate): string { + $givenName = CertificateData::getSubjectGivenName($userCertificate); $surname = CertificateData::getSubjectSurname($userCertificate); - $givenname = CertificateData::getSubjectGivenName($userCertificate); - if ($surname && $givenname) { - $principalName = $givenname . " " . $surname; + if ($givenName && $surname) { + return $givenName . " " . $surname; } else { - $principalName = CertificateData::getSubjectCN($userCertificate); + return CertificateData::getSubjectCN($userCertificate); } - return $principalName; } /** diff --git a/src/authtoken/WebEidAuthToken.php b/src/authtoken/WebEidAuthToken.php index 1529868..3cf1e19 100644 --- a/src/authtoken/WebEidAuthToken.php +++ b/src/authtoken/WebEidAuthToken.php @@ -48,10 +48,6 @@ class WebEidAuthToken * @var string Format */ private ?string $format = null; - /** - * @var string App version - */ - private ?string $appVersion = null; public function __construct(string $authenticationTokenJSON) { @@ -76,10 +72,6 @@ public function __construct(string $authenticationTokenJSON) if (isset($jsonDecoded['format'])) { $this->format = $this->filterString('format', $jsonDecoded['format']); } - // appVersion - if (isset($jsonDecoded['appVersion'])) { - $this->appVersion = $this->filterString('appVersion', $jsonDecoded['appVersion']); - } } public function getUnverifiedCertificate(): ?string @@ -102,11 +94,6 @@ public function getFormat(): ?string return $this->format; } - public function getAppVersion(): ?string - { - return $this->appVersion; - } - private function filterString(string $key, $data): string { $type = gettype($data); diff --git a/src/certificate/CertificateData.php b/src/certificate/CertificateData.php index 666ad7a..e05a1ba 100644 --- a/src/certificate/CertificateData.php +++ b/src/certificate/CertificateData.php @@ -80,15 +80,14 @@ public static function getSubjectCountryCode(X509 $certificate): ?string /** * Get specified subject field from x509 certificate * - * @return string + * @return ?string */ private static function getField(X509 $certificate, string $fieldId): ?string { $result = $certificate->getSubjectDNProp($fieldId); if ($result) { - return join(" ", $result); - } - else { + return join(", ", $result); + } else { return null; } } diff --git a/src/validator/AuthTokenSignatureValidator.php b/src/validator/AuthTokenSignatureValidator.php index 0e4a026..b3fa1ba 100644 --- a/src/validator/AuthTokenSignatureValidator.php +++ b/src/validator/AuthTokenSignatureValidator.php @@ -54,19 +54,19 @@ public function __construct(Uri $siteOrigin) public function validate(string $algorithm, string $signature, $publicKey, string $currentChallengeNonce): void { - $this->requireNotEmpty($algorithm, "algorithm"); - $this->requireNotEmpty($signature, "signature"); + if (empty($currentChallengeNonce)) { + throw new ChallengeNullOrEmptyException(); + } if (is_null($publicKey)) { throw new InvalidArgumentException("Public key is null"); } - if (empty($currentChallengeNonce)) { - throw new ChallengeNullOrEmptyException(); - } + $this->requireNotEmpty($algorithm, "algorithm"); + $this->requireNotEmpty($signature, "signature"); if (!in_array($algorithm, self::ALLOWED_SIGNATURE_ALGORITHMS)) { - throw new AuthTokenParseException("Invalid signature algorithm"); + throw new AuthTokenParseException("Unsupported signature algorithm"); } $decodedSignature = base64_decode($signature); diff --git a/src/validator/ocsp/OcspRequestBuilder.php b/src/validator/ocsp/OcspRequestBuilder.php index 1662f59..0ea5d70 100644 --- a/src/validator/ocsp/OcspRequestBuilder.php +++ b/src/validator/ocsp/OcspRequestBuilder.php @@ -26,6 +26,7 @@ namespace web_eid\web_eid_authtoken_validation_php\validator\ocsp; +use InvalidArgumentException; use web_eid\web_eid_authtoken_validation_php\ocsp\OcspRequest; use web_eid\web_eid_authtoken_validation_php\util\SecureRandom; @@ -58,6 +59,9 @@ public function enableOcspNonce(bool $ocspNonceEnabled): OcspRequestBuilder public function build(): OcspRequest { $ocspRequest = new OcspRequest(); + if (is_null($this->certificateId)) { + throw new InvalidArgumentException("Certificate Id must not be null"); + } $ocspRequest->addCertificateId($this->certificateId); if ($this->ocspNonceEnabled) { diff --git a/src/validator/ocsp/OcspServiceProvider.php b/src/validator/ocsp/OcspServiceProvider.php index f464210..8277d9d 100644 --- a/src/validator/ocsp/OcspServiceProvider.php +++ b/src/validator/ocsp/OcspServiceProvider.php @@ -26,6 +26,7 @@ namespace web_eid\web_eid_authtoken_validation_php\validator\ocsp; +use InvalidArgumentException; use phpseclib3\File\X509; use web_eid\web_eid_authtoken_validation_php\validator\ocsp\service\AiaOcspService; use web_eid\web_eid_authtoken_validation_php\validator\ocsp\service\AiaOcspServiceConfiguration; @@ -41,7 +42,8 @@ class OcspServiceProvider public function __construct(?DesignatedOcspServiceConfiguration $designatedOcspServiceConfiguration, AiaOcspServiceConfiguration $aiaOcspServiceConfiguration) { $this->designatedOcspService = !is_null($designatedOcspServiceConfiguration) ? new DesignatedOcspService($designatedOcspServiceConfiguration) : null; - $this->aiaOcspServiceConfiguration = $aiaOcspServiceConfiguration; + $this->aiaOcspServiceConfiguration = $aiaOcspServiceConfiguration ?? throw new InvalidArgumentException("AIA Ocsp Service Configuration must not be null"); + } /** diff --git a/src/validator/ocsp/OcspUrl.php b/src/validator/ocsp/OcspUrl.php index 6b08c90..539c249 100644 --- a/src/validator/ocsp/OcspUrl.php +++ b/src/validator/ocsp/OcspUrl.php @@ -28,6 +28,7 @@ use phpseclib3\File\X509; use GuzzleHttp\Psr7\Uri; use Exception; +use InvalidArgumentException; final class OcspUrl { @@ -43,6 +44,9 @@ public function __construct() */ public static function getOcspUri(X509 $certificate): ?Uri { + if (is_null($certificate)) { + throw new InvalidArgumentException("Certificate must not be null"); + } $authorityInformationAccess = $certificate->getExtension("id-pe-authorityInfoAccess"); if ($authorityInformationAccess) { foreach ($authorityInformationAccess as $accessDescription) { diff --git a/src/validator/ocsp/service/AiaOcspService.php b/src/validator/ocsp/service/AiaOcspService.php index 5b52ec4..a938ee2 100644 --- a/src/validator/ocsp/service/AiaOcspService.php +++ b/src/validator/ocsp/service/AiaOcspService.php @@ -35,6 +35,7 @@ use DateTime; use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspResponseValidator; use Exception; +use InvalidArgumentException; /** * An OCSP service that uses the responders from the Certificates' Authority Information Access (AIA) extension. @@ -48,6 +49,9 @@ class AiaOcspService implements OcspService public function __construct(AiaOcspServiceConfiguration $configuration, X509 $certificate) { + if (is_null($configuration)) { + throw new InvalidArgumentException("Configuration cannot be null"); + } $this->url = self::getOcspAiaUrlFromCertificate($certificate); $this->trustedCACertificates = $configuration->getTrustedCACertificates(); $this->supportsNonce = !in_array($this->url->jsonSerialize(), $configuration->getNonceDisabledOcspUrls()->getUrlsArray()); diff --git a/tests/authtoken/WebEidAuthTokenTest.php b/tests/authtoken/WebEidAuthTokenTest.php index 5027ff9..bdc76a4 100644 --- a/tests/authtoken/WebEidAuthTokenTest.php +++ b/tests/authtoken/WebEidAuthTokenTest.php @@ -48,7 +48,6 @@ public function testValidateAuthTokenParameters(): void $this->assertEquals("RS256", $authToken->getAlgorithm()); $this->assertEquals("HBjNXIaUskXbfhzYQHvwjKDUWfNu4yxXZh", $authToken->getSignature()); $this->assertEquals("web-eid:1.0", $authToken->getFormat()); - $this->assertEquals("https://web-eid.eu/web-eid-app/releases/v2.0.0", $authToken->getAppVersion()); } public function testWhenNotAuthToken(): void diff --git a/tests/certificate/CertificateDataTest.php b/tests/certificate/CertificateDataTest.php index 0be5c92..4b36d94 100644 --- a/tests/certificate/CertificateDataTest.php +++ b/tests/certificate/CertificateDataTest.php @@ -49,20 +49,22 @@ public function testWhenOrganizationCertificateThenSubjectCNAndIdCodeAndCountryC $this->assertEquals("EE", CertificateData::getSubjectCountryCode($cert)); } - public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreNull(): void + public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreEmpty(): void { $cert = Certificates::getOrganizationCert(); - $this->assertEquals(null, CertificateData::getSubjectGivenName($cert)); - $this->assertEquals(null, CertificateData::getSubjectSurname($cert)); + $givenName = CertificateData::getSubjectGivenName($cert); + $surname = CertificateData::getSubjectSurname($cert); + $this->assertEmpty($givenName); + $this->assertEmpty($surname); } public function testWhenOrganizationCertificateThenSucceeds(): void { $cert = Certificates::getOrganizationCert(); + $givenName = CertificateData::getSubjectGivenName($cert); $surname = CertificateData::getSubjectSurname($cert); - $givenname = CertificateData::getSubjectGivenName($cert); - if ($surname && $givenname) { - $principalName = $givenname . " " . $surname; + if ($givenName && $surname) { + $principalName = $givenName . " " . $surname; } else { $principalName = CertificateData::getSubjectCN($cert); } diff --git a/tests/validator/AuthTokenAlgorithmTest.php b/tests/validator/AuthTokenAlgorithmTest.php index 3a8c1bd..fbe51b7 100644 --- a/tests/validator/AuthTokenAlgorithmTest.php +++ b/tests/validator/AuthTokenAlgorithmTest.php @@ -35,7 +35,7 @@ public function testWhenAlgorithmNoneThenValidationFails(): void $authToken = $this->replaceTokenField(self::VALID_AUTH_TOKEN, "algorithm", "NONE"); $this->expectException(AuthTokenParseException::class); - $this->expectExceptionMessage("Invalid signature algorithm"); + $this->expectExceptionMessage("Unsupported signature algorithm"); $this->validator->validate($authToken, self::VALID_AUTH_TOKEN); } @@ -53,7 +53,7 @@ public function testWhenAlgorithmInvalidThenParsingFails(): void $authToken = $this->replaceTokenField(self::VALID_AUTH_TOKEN, "algorithm", "\\u0000\\t\\ninvalid"); $this->expectException(AuthTokenParseException::class); - $this->expectExceptionMessage("Invalid signature algorithm"); + $this->expectExceptionMessage("Unsupported signature algorithm"); $this->validator->validate($authToken, self::VALID_AUTH_TOKEN); } } From ab3b0aed674da76a1bd11845c886c537093cc08d Mon Sep 17 00:00:00 2001 From: Mihkel Kivisild Date: Mon, 4 Nov 2024 12:27:17 +0200 Subject: [PATCH 4/6] Escaping HTML special characters in welcome.phtml WE2-970 Signed-off-by: Mihkel Kivisild --- example/tpl/welcome.phtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/tpl/welcome.phtml b/example/tpl/welcome.phtml index 409b8a3..6901bbe 100644 --- a/example/tpl/welcome.phtml +++ b/example/tpl/welcome.phtml @@ -1,2 +1,2 @@ -

Hello

+

Hello

Logout

\ No newline at end of file From 325edc9476e918d5f0024ae3d45da3c11e3ff8d0 Mon Sep 17 00:00:00 2001 From: Sven Mitt Date: Tue, 18 Mar 2025 13:45:33 +0200 Subject: [PATCH 5/6] Remove old dangling comment WE2-985 Signed-off-by: Sven Mitt --- src/ocsp/OcspRequest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ocsp/OcspRequest.php b/src/ocsp/OcspRequest.php index df4e264..9765d87 100644 --- a/src/ocsp/OcspRequest.php +++ b/src/ocsp/OcspRequest.php @@ -68,7 +68,6 @@ public function addNonceExtension(string $nonce): void */ public function getNonceExtension(): ?string { - // TODO: the ?? '' is here only for v1.0 API compatibility. Remove this in version 1.2 and change the return type to ?string. return AsnUtil::decodeNonceExtension($this->ocspRequest["tbsRequest"]["requestExtensions"] ?? []); } From 4f9851dee5a9716dd8135726b686b756377c11ec Mon Sep 17 00:00:00 2001 From: Mart Somermaa Date: Thu, 20 Mar 2025 14:46:44 +0200 Subject: [PATCH 6/6] Merge organization certificate tests WE2-999 Signed-off-by: Mart Somermaa --- tests/certificate/CertificateDataTest.php | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/certificate/CertificateDataTest.php b/tests/certificate/CertificateDataTest.php index 4b36d94..30d0224 100644 --- a/tests/certificate/CertificateDataTest.php +++ b/tests/certificate/CertificateDataTest.php @@ -49,27 +49,14 @@ public function testWhenOrganizationCertificateThenSubjectCNAndIdCodeAndCountryC $this->assertEquals("EE", CertificateData::getSubjectCountryCode($cert)); } - public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreEmpty(): void + public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreEmptyAndSubjectCNSucceeds(): void { $cert = Certificates::getOrganizationCert(); $givenName = CertificateData::getSubjectGivenName($cert); $surname = CertificateData::getSubjectSurname($cert); $this->assertEmpty($givenName); $this->assertEmpty($surname); - } - - public function testWhenOrganizationCertificateThenSucceeds(): void - { - $cert = Certificates::getOrganizationCert(); - $givenName = CertificateData::getSubjectGivenName($cert); - $surname = CertificateData::getSubjectSurname($cert); - if ($givenName && $surname) { - $principalName = $givenName . " " . $surname; - } else { - $principalName = CertificateData::getSubjectCN($cert); - } - + $principalName = CertificateData::getSubjectCN($cert); $this->assertEquals("Testijad.ee isikutuvastus", $principalName); - } }