Skip to content

Commit cc2f8f0

Browse files
SpomkySpomky
committed
Merge branch '5.2.x' into 5.3.x
* 5.2.x: fix: normalize host-only allowed origins to https:// scheme (#822) fix: pass topOriginValidator to CheckTopOrigin in requestCeremony() (#821) fix: enforce HTTPS scheme check in CheckAllowedOrigins fallback path (#820) fix: regenerate PHPStan baseline to fix CI on 5.2.x fix: add PHPStan type annotations for parse_url() return values in CheckAllowedOrigins Merge commit from fork fix: set trust anchor when validating certificate path to support intermediate CA certificates (#793) # Conflicts: # phpstan-baseline.neon # src/webauthn/src/CeremonyStep/CeremonyStepManagerFactory.php # src/webauthn/src/CeremonyStep/CheckAllowedOrigins.php
2 parents ea3c06d + bf3baac commit cc2f8f0

File tree

4 files changed

+96
-45
lines changed

4 files changed

+96
-45
lines changed

‎src/webauthn/src/CeremonyStep/CeremonyStepManagerFactory.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,12 @@ public function requestCeremony(): CeremonyStepManager
125125
new CheckChallenge(),
126126
$this->allowedOrigins === null ? new CheckOrigin(
127127
$this->securedRelyingPartyId ?? []
128-
) : new CheckAllowedOrigins($this->allowedOrigins, $this->allowSubdomains),
129-
new CheckTopOrigin(),
128+
) : new CheckAllowedOrigins(
129+
$this->allowedOrigins,
130+
$this->allowSubdomains,
131+
$this->securedRelyingPartyId ?? []
132+
),
133+
new CheckTopOrigin($this->topOriginValidator),
130134
new CheckRelyingPartyIdIdHash(),
131135
new CheckUserWasPresent(),
132136
new CheckUserVerification(),
@@ -176,7 +180,11 @@ private function buildCreationCeremony(bool $requireUserPresence): CeremonyStepM
176180
new CheckChallenge(),
177181
$this->allowedOrigins === null ? new CheckOrigin(
178182
$this->securedRelyingPartyId ?? []
179-
) : new CheckAllowedOrigins($this->allowedOrigins, $this->allowSubdomains),
183+
) : new CheckAllowedOrigins(
184+
$this->allowedOrigins,
185+
$this->allowSubdomains,
186+
$this->securedRelyingPartyId ?? []
187+
),
180188
new CheckTopOrigin($this->topOriginValidator),
181189
new CheckRelyingPartyIdIdHash(),
182190
new CheckUserWasPresent($requireUserPresence),

‎src/webauthn/src/CeremonyStep/CheckAllowedOrigins.php‎

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,40 +23,35 @@
2323
final readonly class CheckAllowedOrigins implements CeremonyStep
2424
{
2525
/**
26-
* Full origin entries (scheme://host[:port]) from allowed origins that include a scheme.
26+
* Full origin entries (scheme://host[:port]) from allowed origins.
2727
*
2828
* @var string[]
2929
*/
3030
private array $fullOrigins;
3131

32-
/**
33-
* Host-only entries from allowed origins without a scheme (backward compatibility).
34-
*
35-
* @var string[]
36-
*/
37-
private array $hostOrigins;
38-
3932
/**
4033
* @param string[] $allowedOrigins
34+
* @param string[] $securedRelyingPartyId RP IDs that are allowed to use HTTP (e.g. localhost for development)
4135
*/
4236
public function __construct(
4337
array $allowedOrigins,
44-
private bool $allowSubdomains = false
38+
private bool $allowSubdomains = false,
39+
private array $securedRelyingPartyId = [],
4540
) {
4641
$fullOrigins = [];
47-
$hostOrigins = [];
4842
foreach ($allowedOrigins as $allowedOrigin) {
4943
$parsed = parse_url($allowedOrigin);
5044
$parsed !== false || throw new InvalidArgumentException(sprintf('Invalid origin: %s', $allowedOrigin));
5145
if (isset($parsed['scheme'], $parsed['host'])) {
5246
$fullOrigins[] = self::buildOrigin($parsed['scheme'], $parsed['host'], $parsed['port'] ?? null);
5347
} else {
54-
$hostOrigins[] = $parsed['host'] ?? $allowedOrigin;
48+
// Host-only entries are normalized to https:// since WebAuthn requires TLS
49+
$host = $parsed['host'] ?? $allowedOrigin;
50+
$fullOrigins[] = self::buildOrigin('https', $host, null);
5551
}
5652
}
5753

5854
$this->fullOrigins = array_unique($fullOrigins);
59-
$this->hostOrigins = array_unique($hostOrigins);
6055
}
6156

6257
public function process(
@@ -83,7 +78,7 @@ public function process(
8378
);
8479
$originHost = $parsedOrigin['host'] ?? $C->origin;
8580

86-
$hasAllowedOrigins = count($this->fullOrigins) !== 0 || count($this->hostOrigins) !== 0;
81+
$hasAllowedOrigins = count($this->fullOrigins) !== 0;
8782

8883
if ($hasAllowedOrigins) {
8984
// Full origin match (scheme + host + port)
@@ -98,15 +93,8 @@ public function process(
9893
}
9994
}
10095

101-
// Host-only match (backward compatibility for entries without scheme)
102-
if (in_array($originHost, $this->hostOrigins, true)) {
103-
return;
104-
}
105-
10696
// Subdomain matching
107-
$isFullOriginSubdomain = $this->isSubdomainOfFullOrigins($parsedOrigin);
108-
$isHostSubdomain = $this->isSubdomain($this->hostOrigins, $originHost);
109-
$isSubDomain = $isFullOriginSubdomain || $isHostSubdomain;
97+
$isSubDomain = $this->isSubdomainOfFullOrigins($parsedOrigin);
11098

11199
if ($this->allowSubdomains && $isSubDomain) {
112100
return;
@@ -121,6 +109,13 @@ public function process(
121109

122110
$rpId = $publicKeyCredentialOptions->rpId ?? $publicKeyCredentialOptions->rp->id ?? $host;
123111
$facetId = $this->getFacetId($rpId, $publicKeyCredentialOptions->extensions, $authData->extensions);
112+
113+
if (! in_array($facetId, $this->securedRelyingPartyId, true)) {
114+
$scheme = $parsedOrigin['scheme'] ?? '';
115+
$scheme === 'https' || throw AuthenticatorResponseVerificationException::create(
116+
'Invalid scheme. HTTPS required.'
117+
);
118+
}
124119
$facetId !== '' || throw AuthenticatorResponseVerificationException::create(
125120
'Invalid origin. Unable to determine the facet ID.'
126121
);
@@ -134,11 +129,7 @@ public function process(
134129
if (! $this->allowSubdomains && $isSubDomains) {
135130
throw AuthenticatorResponseVerificationException::create('Invalid origin. Subdomains are not allowed.');
136131
}
137-
138-
$scheme = $parsedOrigin['scheme'] ?? '';
139-
$scheme === 'https' || throw AuthenticatorResponseVerificationException::create(
140-
'Invalid scheme. HTTPS required.'
141-
);
132+
throw AuthenticatorResponseVerificationException::create('Invalid origin.');
142133
}
143134

144135
/**
@@ -217,17 +208,4 @@ private function getFacetId(
217208

218209
return (is_string($appId) && $wasUsed === true) ? $appId : $rpId;
219210
}
220-
221-
/**
222-
* @param string[] $origins
223-
*/
224-
private function isSubdomain(array $origins, string $domain): bool
225-
{
226-
foreach ($origins as $allowedOrigin) {
227-
if ($this->isSubdomainOf($domain, $allowedOrigin)) {
228-
return true;
229-
}
230-
}
231-
return false;
232-
}
233211
}

‎src/webauthn/src/CeremonyStep/CheckTopOrigin.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,8 @@ public function process(
4343
throw AuthenticatorResponseVerificationException::create('The response is not cross-origin.');
4444
}
4545
if ($this->topOriginValidator === null) {
46-
(new HostTopOriginValidator($host))->validate($topOrigin);
47-
} else {
48-
$this->topOriginValidator->validate($topOrigin);
46+
return;
4947
}
48+
$this->topOriginValidator->validate($topOrigin);
5049
}
5150
}

‎tests/library/Functional/CheckAllowedOriginsTest.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,72 @@ public function emptyAllowedOriginsWithoutSubdomainsAndValidHost(): void
196196
static::assertTrue(true); // if no exception, test passes
197197
}
198198

199+
#[Test]
200+
public function differentPortIsRejected(): void
201+
{
202+
// PoC from GHSA-f7pm-6hr8-7ggm: different port on same host must be rejected
203+
// C.origin = https://webauthn.spomky-labs.com (port 443)
204+
// Allowed = https://webauthn.spomky-labs.com:8443 (port 8443)
205+
$this->expectException(AuthenticatorResponseVerificationException::class);
206+
$this->expectExceptionMessage('Invalid origin');
207+
208+
$checkOrigins = new CheckAllowedOrigins(['https://webauthn.spomky-labs.com:8443']);
209+
$publicKeyCredentialSource = $this->getPublicKeyCredentialSource();
210+
$publicKeyCredentialRequestOptions = $this->getPublicKeyCredentialRequestOptions();
211+
$publicKeyCredential = $this->getPublicKeyCredential();
212+
213+
$checkOrigins->process(
214+
$publicKeyCredentialSource,
215+
$publicKeyCredential->response,
216+
$publicKeyCredentialRequestOptions,
217+
null,
218+
'webauthn.spomky-labs.com',
219+
);
220+
}
221+
222+
#[Test]
223+
public function explicitDefaultPortMatchesImplicitPort(): void
224+
{
225+
// https://webauthn.spomky-labs.com:443 should match https://webauthn.spomky-labs.com
226+
$checkOrigins = new CheckAllowedOrigins(['https://webauthn.spomky-labs.com:443']);
227+
$publicKeyCredentialSource = $this->getPublicKeyCredentialSource();
228+
$publicKeyCredentialRequestOptions = $this->getPublicKeyCredentialRequestOptions();
229+
$publicKeyCredential = $this->getPublicKeyCredential();
230+
231+
$checkOrigins->process(
232+
$publicKeyCredentialSource,
233+
$publicKeyCredential->response,
234+
$publicKeyCredentialRequestOptions,
235+
null,
236+
'webauthn.spomky-labs.com',
237+
);
238+
239+
static::assertTrue(true);
240+
}
241+
242+
#[Test]
243+
public function httpSchemeIsRejectedWhenHttpsIsConfigured(): void
244+
{
245+
// Allowed = https://webauthn.spomky-labs.com
246+
// C.origin = https://webauthn.spomky-labs.com (matches, but testing that http:// would not)
247+
// We test by configuring http:// and verifying it rejects https:// origin
248+
$this->expectException(AuthenticatorResponseVerificationException::class);
249+
$this->expectExceptionMessage('Invalid origin');
250+
251+
$checkOrigins = new CheckAllowedOrigins(['http://webauthn.spomky-labs.com']);
252+
$publicKeyCredentialSource = $this->getPublicKeyCredentialSource();
253+
$publicKeyCredentialRequestOptions = $this->getPublicKeyCredentialRequestOptions();
254+
$publicKeyCredential = $this->getPublicKeyCredential();
255+
256+
$checkOrigins->process(
257+
$publicKeyCredentialSource,
258+
$publicKeyCredential->response,
259+
$publicKeyCredentialRequestOptions,
260+
null,
261+
'webauthn.spomky-labs.com',
262+
);
263+
}
264+
199265
#[Test]
200266
public function emptyAllowedOriginsWithSubdomainsAndInvalidHost(): void
201267
{

0 commit comments

Comments
 (0)