Skip to content

Commit 8fb3981

Browse files
zll600Spomky
zll600
and
Spomky
authored
chore: simplify metadata statement check (#777)
* chore: simplify metadata statement check * refactor: enhance metadata statement verification logic and improve logging --------- Co-authored-by: Florent Morselli <florent.morselli@spomky-labs.com>
1 parent e15aea9 commit 8fb3981

File tree

2 files changed

+156
-65
lines changed

2 files changed

+156
-65
lines changed

‎.ci-tools/phpstan-baseline.neon‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4464,12 +4464,6 @@ parameters:
44644464
count: 1
44654465
path: ../src/webauthn/src/CeremonyStep/CheckMetadataStatement.php
44664466

4467-
-
4468-
rawMessage: 'Method Webauthn\CeremonyStep\CheckMetadataStatement::checkCertificateChain() has parameter $metadataStatement with a nullable type declaration.'
4469-
identifier: ergebnis.noParameterWithNullableTypeDeclaration
4470-
count: 1
4471-
path: ../src/webauthn/src/CeremonyStep/CheckMetadataStatement.php
4472-
44734467
-
44744468
rawMessage: 'Method Webauthn\CeremonyStep\CheckMetadataStatement::process() has parameter $userHandle with a nullable type declaration.'
44754469
identifier: ergebnis.noParameterWithNullableTypeDeclaration

‎src/webauthn/src/CeremonyStep/CheckMetadataStatement.php‎

Lines changed: 156 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -89,68 +89,155 @@ public function process(
8989
$attestedCredentialData !== null || throw AuthenticatorResponseVerificationException::create(
9090
'No attested credential data found'
9191
);
92+
93+
if (! $this->isAttestationVerificationRequested($publicKeyCredentialOptions)) {
94+
$this->logger->debug('No attestation verification requested by RP.');
95+
return;
96+
}
97+
98+
if ($attestationStatement->type === AttestationStatement::TYPE_NONE) {
99+
$this->logger->debug('None attestation format. No metadata verification required.');
100+
return;
101+
}
102+
92103
$aaguid = $attestedCredentialData->aaguid
93104
->__toString();
94-
if ($publicKeyCredentialOptions->attestation === null || $publicKeyCredentialOptions->attestation === PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE) {
95-
$this->logger->debug('No attestation is asked.');
96-
if ($aaguid === '00000000-0000-0000-0000-000000000000' && in_array(
97-
$attestationStatement->type,
98-
[AttestationStatement::TYPE_NONE, AttestationStatement::TYPE_SELF],
99-
true
100-
)) {
101-
$this->logger->debug('The Attestation Statement is anonymous.');
102-
$this->checkCertificateChain($attestationStatement, null);
103-
return;
104-
}
105+
if ($this->isNullAaguid($aaguid)) {
106+
$this->logger->debug('Null AAGUID detected. Skipping metadata verification.', [
107+
'reason' => 'Privacy placeholder or U2F device',
108+
]);
105109
return;
106110
}
107-
// If no Attestation Statement has been returned or if null AAGUID (=00000000-0000-0000-0000-000000000000)
108-
// => nothing to check
109-
if ($attestationStatement->type === AttestationStatement::TYPE_NONE) {
110-
$this->logger->debug('No attestation returned.');
111-
//No attestation is returned. We shall ensure that the AAGUID is a null one.
112-
//if ($aaguid !== '00000000-0000-0000-0000-000000000000') {
113-
//$this->logger->debug('Anonymization required. AAGUID and Attestation Statement changed.', [
114-
// 'aaguid' => $aaguid,
115-
// 'AttestationStatement' => $attestationStatement,
116-
//]);
117-
//$attestedCredentialData->aaguid = Uuid::fromString('00000000-0000-0000-0000-000000000000');
118-
// return;
119-
//}
111+
112+
if ($attestationStatement->type === AttestationStatement::TYPE_SELF) {
113+
$this->logger->debug('Self attestation detected.', [
114+
'aaguid' => $aaguid,
115+
]);
116+
$this->processSelfAttestation($aaguid);
120117
return;
121118
}
122-
if ($aaguid === '00000000-0000-0000-0000-000000000000') {
123-
//No need to continue if the AAGUID is null.
124-
// This could be the case e.g. with AnonCA type
119+
120+
$this->logger->debug('Processing attestation with full metadata validation.', [
121+
'type' => $attestationStatement->type,
122+
'aaguid' => $aaguid,
123+
]);
124+
$this->processWithMetadata($attestationStatement, $aaguid);
125+
}
126+
127+
/**
128+
* Check if RP requested attestation verification.
129+
* @see https://www.w3.org/TR/webauthn-3/#dom-attestationconveyancepreference-none
130+
*/
131+
private function isAttestationVerificationRequested(
132+
PublicKeyCredentialCreationOptions $publicKeyCredentialOptions
133+
): bool {
134+
return ! in_array(
135+
$publicKeyCredentialOptions->attestation,
136+
[null, PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE],
137+
true
138+
);
139+
}
140+
141+
/**
142+
* Check if AAGUID is all-zeros (privacy placeholder or U2F device).
143+
*
144+
* All-zeros AAGUID: either privacy placeholder or U2F device (which predates AAGUID).
145+
* 1) Privacy Placeholder indicates the authenticator does not provide detailed information.
146+
* 2) U2F device cannot provide useful AAGUID.
147+
*
148+
* So Metadata Statement lookup by AAGUID not possible.
149+
*
150+
* @see https://www.w3.org/TR/webauthn-3/#sctn-createCredential
151+
* @see https://fidoalliance.org/specs/fido-v2.2-ps-20250714/fido-client-to-authenticator-protocol-v2.2-ps-20250714.html#u2f-authenticatorMakeCredential-interoperability
152+
*/
153+
private function isNullAaguid(string $aaguid): bool
154+
{
155+
return $aaguid === '00000000-0000-0000-0000-000000000000';
156+
}
157+
158+
/**
159+
* Process self attestation: check MDS for compromised devices.
160+
*
161+
* Self attestation: authenticator uses credential key pair to sign attestation.
162+
* No attestation certificate chain to validate, but we still check MDS for compromised devices.
163+
*
164+
* @see https://www.w3.org/TR/webauthn-3/#self
165+
*/
166+
private function processSelfAttestation(string $aaguid): void
167+
{
168+
$metadataStatement = $this->metadataStatementRepository?->findOneByAAGUID($aaguid);
169+
if ($metadataStatement === null) {
170+
$this->logger->info('No metadata statement found for self attestation. Skipping MDS verification.', [
171+
'aaguid' => $aaguid,
172+
]);
125173
return;
126174
}
127-
//The MDS Repository is mandatory here
175+
176+
$this->logger->debug('Metadata statement found for self attestation. Checking status reports.', [
177+
'aaguid' => $aaguid,
178+
]);
179+
$this->checkStatusReport($aaguid);
180+
// Note: We do NOT check attestationTypes for self attestation as the authenticator
181+
// is using its credential key (not attestation certificate), which may differ from
182+
// the declared attestation types in metadata (which describe certificate-based attestation).
183+
}
184+
185+
/**
186+
* Process attestation with full metadata validation (Basic, AttCA, AnonCA).
187+
*/
188+
private function processWithMetadata(AttestationStatement $attestationStatement, string $aaguid): void
189+
{
128190
$this->metadataStatementRepository !== null || throw AuthenticatorResponseVerificationException::create(
129191
'The Metadata Statement Repository is mandatory when requesting attestation objects.'
130192
);
131193
$metadataStatement = $this->metadataStatementRepository->findOneByAAGUID($aaguid);
132-
// At this point, the Metadata Statement is mandatory
133194
$metadataStatement !== null || throw AuthenticatorResponseVerificationException::create(
134195
sprintf('The Metadata Statement for the AAGUID "%s" is missing', $aaguid)
135196
);
136-
// We check the last status report
197+
198+
$this->logger->debug('Metadata statement found. Starting validation.', [
199+
'aaguid' => $aaguid,
200+
'attestation_type' => $attestationStatement->type,
201+
]);
202+
137203
$this->checkStatusReport($aaguid);
138-
// We check the certificate chain (if any)
204+
$this->logger->debug('Status report verification completed.');
205+
139206
$this->checkCertificateChain($attestationStatement, $metadataStatement);
140-
// Check Attestation Type is allowed
141-
if (count($metadataStatement->attestationTypes) !== 0) {
142-
$type = $this->getAttestationType($attestationStatement);
143-
in_array(
144-
$type,
145-
$metadataStatement->attestationTypes,
146-
true
147-
) || throw AuthenticatorResponseVerificationException::create(
148-
sprintf(
149-
'Invalid attestation statement. The attestation type "%s" is not allowed for this authenticator.',
150-
$type
151-
)
152-
);
207+
$this->logger->debug('Certificate chain verification completed.');
208+
209+
$this->checkAttestationTypeIsAllowed($attestationStatement, $metadataStatement);
210+
$this->logger->debug('Attestation type verification completed.');
211+
}
212+
213+
/**
214+
* Check if the attestation type is allowed for this authenticator.
215+
*/
216+
private function checkAttestationTypeIsAllowed(
217+
AttestationStatement $attestationStatement,
218+
MetadataStatement $metadataStatement
219+
): void {
220+
if (count($metadataStatement->attestationTypes) === 0) {
221+
$this->logger->debug('No attestation types restrictions in metadata statement.');
222+
return;
153223
}
224+
225+
$type = $this->getAttestationType($attestationStatement);
226+
$this->logger->debug('Checking attestation type.', [
227+
'type' => $type,
228+
'allowed_types' => $metadataStatement->attestationTypes,
229+
]);
230+
231+
in_array(
232+
$type,
233+
$metadataStatement->attestationTypes,
234+
true
235+
) || throw AuthenticatorResponseVerificationException::create(
236+
sprintf(
237+
'Invalid attestation statement. The attestation type "%s" is not allowed for this authenticator.',
238+
$type
239+
)
240+
);
154241
}
155242

156243
private function getAttestationType(AttestationStatement $attestationStatement): string
@@ -169,29 +256,39 @@ private function checkStatusReport(string $aaguid): void
169256
$statusReports = $this->statusReportRepository === null ? [] : $this->statusReportRepository->findStatusReportsByAAGUID(
170257
$aaguid
171258
);
172-
if (count($statusReports) !== 0) {
173-
$lastStatusReport = end($statusReports);
174-
if ($lastStatusReport->isCompromised()) {
175-
throw AuthenticatorResponseVerificationException::create(
176-
'The authenticator is compromised and cannot be used'
177-
);
178-
}
259+
if (count($statusReports) === 0) {
260+
$this->logger->debug('No status reports found for authenticator.', [
261+
'aaguid' => $aaguid,
262+
]);
263+
return;
264+
}
265+
266+
$lastStatusReport = end($statusReports);
267+
$this->logger->debug('Status report found.', [
268+
'aaguid' => $aaguid,
269+
'status' => $lastStatusReport->status,
270+
]);
271+
272+
if ($lastStatusReport->isCompromised()) {
273+
$this->logger->warning('Authenticator is marked as compromised in MDS.', [
274+
'aaguid' => $aaguid,
275+
'status' => $lastStatusReport->status,
276+
]);
277+
throw AuthenticatorResponseVerificationException::create(
278+
'The authenticator is compromised and cannot be used'
279+
);
179280
}
180281
}
181282

182283
private function checkCertificateChain(
183284
AttestationStatement $attestationStatement,
184-
?MetadataStatement $metadataStatement
285+
MetadataStatement $metadataStatement
185286
): void {
186287
$trustPath = $attestationStatement->trustPath;
187-
if (! $trustPath instanceof CertificateTrustPath) {
188-
return;
189-
}
288+
$trustPath instanceof CertificateTrustPath || throw AuthenticatorResponseVerificationException::create(
289+
'Certificate trust path is required for attestation verification'
290+
);
190291
$authenticatorCertificates = $trustPath->certificates;
191-
if ($metadataStatement === null) {
192-
$this->certificateChainValidator?->check($authenticatorCertificates, []);
193-
return;
194-
}
195292
$trustedCertificates = CertificateToolbox::fixPEMStructures(
196293
$metadataStatement->attestationRootCertificates
197294
);

0 commit comments

Comments
 (0)