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

OCSPValidator improvements #6

Merged
merged 3 commits into from
Dec 22, 2023
Merged
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
9 changes: 7 additions & 2 deletions Resources/translations/validators+intl-icu.en.xliff
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
<target>The certificate host "{provided}" contains an invalid public-suffix wildcard pattern "{suffix_pattern}".</target>
</trans-unit>
<trans-unit id="13">
<source>Unable to resolve CA of certificate "{name}".</source>
<target>Unable to resolve CA of certificate "{name}".</target>
<source>Unable to resolve the CA of certificate "{name}", issued by {parent}.</source>
<target>Unable to resolve the CA of certificate "{name}", issued by {parent}.</target>
</trans-unit>

<!-- Revocation reason -->
Expand Down Expand Up @@ -88,6 +88,11 @@
<source>it is known or suspected that aspects of the AA validated in the attribute certificate have been compromised.</source>
<target>it is known or suspected that aspects of the AA validated in the attribute certificate have been compromised.</target>
</trans-unit>

<trans-unit id="22">
<source>Too many CAs were provided. A maximum of 4 is accepted.</source>
<target>Too many CAs were provided. A maximum of 4 is accepted.</target>
</trans-unit>
</body>
</file>
</xliff>
10 changes: 6 additions & 4 deletions src/CAResolverImpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace Rollerworks\Component\X509Validator;

use Rollerworks\Component\X509Validator\Violation\MissingCAExtension;
use Rollerworks\Component\X509Validator\Violation\ToManyCAsProvided;
use Rollerworks\Component\X509Validator\Violation\TooManyCAsProvided;
use Rollerworks\Component\X509Validator\Violation\UnableToResolveParent;

class CAResolverImpl implements CAResolver
Expand All @@ -35,8 +35,8 @@ public function resolve(string $certificate, array $caList): ?CA
{
// Safety check to prevent DoS attacks
// Normally only two parents are used, more than three is exceptional
if (\count($caList) > 3) {
throw new ToManyCAsProvided();
if (\count($caList) > 4) {
throw new TooManyCAsProvided();
}

$certData = $this->extractor->extractRawData($certificate, '', true);
Expand Down Expand Up @@ -84,7 +84,9 @@ private function resolveCA(string $certificate, array $caList): CA
return new CA($contents);
}

throw new UnableToResolveParent($this->extractor->extractRawData($certificate)->commonName);
$x509Info = $this->extractor->extractRawData($certificate);

throw new UnableToResolveParent($x509Info->commonName, $x509Info->allFields['issuer']['commonName']);
}

private function validateCA(X509Info $data): void
Expand Down
4 changes: 4 additions & 0 deletions src/OCSPValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public function __construct(
X509DataExtractor $dataExtractor = null,
Ocsp $ocsp = null
) {
if ($httpClient === null && ! class_exists(HttpClient::class)) {
throw new \LogicException(sprintf('The "%s" class requires a "%s" instance, or that the Symfony HttpClient is available. Try running "composer require symfony/http-client".', self::class, HttpClientInterface::class));
}

$this->extractor = $dataExtractor ?? new X509DataExtractor();
$this->httpClient = $httpClient ?? HttpClient::create();
$this->logger = $logger ?? new NullLogger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@

use Rollerworks\Component\X509Validator\Violation;

final class ToManyCAsProvided extends Violation
final class TooManyCAsProvided extends Violation
{
public function __construct()
{
parent::__construct('To many CAs were provided. A maximum of 3 is accepted.');
parent::__construct('Too many CAs were provided. A maximum of 4 is accepted.');
}

public function getTranslatorMsg(): string
{
return 'tls.violation.to_many_ca_provided';
return 'Too many CAs were provided. A maximum of 4 is accepted.';
}
}
12 changes: 4 additions & 8 deletions src/Violation/UnableToResolveParent.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,18 @@

final class UnableToResolveParent extends Violation
{
private readonly string $name;

public function __construct(string $name, int $code = 1)
public function __construct(private readonly string $name, private readonly string $issuer, int $code = 1)
{
parent::__construct(sprintf('Unable to resolve the parent CA of certificate "%s".', $name), $code);

$this->name = $name;
parent::__construct(sprintf('Unable to resolve the parent CA of certificate "%s", issued by "%s".', $name, $issuer), $code);
}

public function getTranslatorMsg(): string
{
return 'Unable to resolve the CA of certificate "{name}".';
return 'Unable to resolve the CA of certificate "{name}", issued by {parent}.';
}

public function getParameters(): array
{
return ['name' => $this->name];
return ['name' => $this->name, 'parent' => $this->issuer];
}
}
2 changes: 1 addition & 1 deletion src/X509DataExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function extractRawData(string $contents, string $name = '', bool $withPu
}

try {
$fingerprint = openssl_x509_fingerprint($x509Read, $rawData['signatureTypeSN']) ?: '';
$fingerprint = @openssl_x509_fingerprint($x509Read, $rawData['signatureTypeSN']) ?: '';
} catch (\Throwable) {
$fingerprint = '';
}
Expand Down
33 changes: 31 additions & 2 deletions tests/CAResolverImplTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use Rollerworks\Component\X509Validator\CA;
use Rollerworks\Component\X509Validator\CAResolverImpl as CAResolver;
use Rollerworks\Component\X509Validator\Violation\MissingCAExtension;
use Rollerworks\Component\X509Validator\Violation\ToManyCAsProvided;
use Rollerworks\Component\X509Validator\Violation\TooManyCAsProvided;
use Rollerworks\Component\X509Validator\Violation\UnableToResolveParent;
use Rollerworks\Component\X509Validator\Violation\UnprocessablePEM;

Expand Down Expand Up @@ -98,7 +98,7 @@ public function it_fails_with_to_many_cas_provided(): void
{
$resolver = new CAResolver();

$this->expectException(ToManyCAsProvided::class);
$this->expectException(TooManyCAsProvided::class);

$resolver->resolve(
<<<'CERT'
Expand Down Expand Up @@ -240,6 +240,35 @@ public function it_fails_with_to_many_cas_provided(): void
j6tJLp07kzQoH3jOlOrHvdPJbRzeXDLz
-----END CERTIFICATE-----
CERT,
'ca45' => <<<'CERT'
-----BEGIN CERTIFICATE-----
MIIElDCCA3ygAwIBAgIQAf2j627KdciIQ4tyS8+8kTANBgkqhkiG9w0BAQsFADBh
MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3
d3cuZGlnaWNlcnQuY29tMSAwHgYDVQQDExdEaWdpQ2VydCBHbG9iYWwgUm9vdCBD
QTAeFw0xMzAzMDgxMjAwMDBaFw0yMzAzMDgxMjAwMDBaME0xCzAJBgNVBAYTAlVT
MRUwEwYDVQQKEwxEaWdpQ2VydCBJbmMxJzAlBgNVBAMTHkRpZ2lDZXJ0IFNIQTIg
U2VjdXJlIFNlcnZlciBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
ANyuWJBNwcQwFZA1W248ghX1LFy949v/cUP6ZCWA1O4Yok3wZtAKc24RmDYXZK83
nf36QYSvx6+M/hpzTc8zl5CilodTgyu5pnVILR1WN3vaMTIa16yrBvSqXUu3R0bd
KpPDkC55gIDvEwRqFDu1m5K+wgdlTvza/P96rtxcflUxDOg5B6TXvi/TC2rSsd9f
/ld0Uzs1gN2ujkSYs58O09rg1/RrKatEp0tYhG2SS4HD2nOLEpdIkARFdRrdNzGX
kujNVA075ME/OV4uuPNcfhCOhkEAjUVmR7ChZc6gqikJTvOX6+guqw9ypzAO+sf0
/RR3w6RbKFfCs/mC/bdFWJsCAwEAAaOCAVowggFWMBIGA1UdEwEB/wQIMAYBAf8C
AQAwDgYDVR0PAQH/BAQDAgGGMDQGCCsGAQUFBwEBBCgwJjAkBggrBgEFBQcwAYYY
aHR0cDovL29jc3AuZGlnaWNlcnQuY29tMHsGA1UdHwR0MHIwN6A1oDOGMWh0dHA6
Ly9jcmwzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydEdsb2JhbFJvb3RDQS5jcmwwN6A1
oDOGMWh0dHA6Ly9jcmw0LmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydEdsb2JhbFJvb3RD
QS5jcmwwPQYDVR0gBDYwNDAyBgRVHSAAMCowKAYIKwYBBQUHAgEWHGh0dHBzOi8v
d3d3LmRpZ2ljZXJ0LmNvbS9DUFMwHQYDVR0OBBYEFA+AYRyCMWHVLyjnjUY4tCzh
xtniMB8GA1UdIwQYMBaAFAPeUDVW0Uy7ZvCj4hsbw5eyPdFVMA0GCSqGSIb3DQEB
CwUAA4IBAQAjPt9L0jFCpbZ+QlwaRMxp0Wi0XUvgBCFsS+JtzLHgl4+mUwnNqipl
5TlPHoOlblyYoiQm5vuh7ZPHLgLGTUq/sELfeNqzqPlt/yGFUzZgTHbO7Djc1lGA
8MXW5dRNJ2Srm8c+cftIl7gzbckTB+6WohsYFfZcTEDts8Ls/3HB40f/1LkAtDdC
2iDJ6m6K7hQGrn2iWZiIqBtvLfTyyRRfJs8sjX7tN8Cp1Tm5gr8ZDOo0rwAhaPit
c+LJMto4JQtV05od8GiG7S5BNO98pVAdvzr508EIDObtHopYJeS4d60tbvVS3bR0
j6tJLp07kzQoH3jOlOrHvdPJbRzeXDLz
-----END CERTIFICATE-----
CERT,
]
);
}
Expand Down
Loading