From dcf61148086ccff7aab3608b83b4fb1ac446ba92 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Tue, 31 Aug 2021 21:58:38 -0700 Subject: [PATCH 1/5] Add new (tentative) allowUnrecognizedAAGUID to MDS --- .../server/src/services/metadataService.ts | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/server/src/services/metadataService.ts b/packages/server/src/services/metadataService.ts index 58bfba5a..cda31991 100644 --- a/packages/server/src/services/metadataService.ts +++ b/packages/server/src/services/metadataService.ts @@ -46,6 +46,7 @@ export class BaseMetadataService { private mdsCache: { [url: string]: CachedMDS } = {}; private statementCache: { [aaguid: string]: CachedBLOBEntry } = {}; private state: SERVICE_STATE = SERVICE_STATE.DISABLED; + private allowUnrecognizedAAGUID = false; /** * Prepare the service to handle remote MDS servers and/or cache local metadata statements. @@ -54,9 +55,16 @@ export class BaseMetadataService { opts: { mdsServers?: string[]; statements?: MetadataStatement[]; + // TODO: What to call a flag that means "don't error out when an aaguid doesn't have metadata" + // Not entirely satisfied with this name + allowUnrecognizedAAGUID?: boolean; } = {}, ): Promise { - const { mdsServers = [defaultURLMDS], statements } = opts; + const { + mdsServers = [defaultURLMDS], + statements, + allowUnrecognizedAAGUID = false, + } = opts; this.setState(SERVICE_STATE.REFRESHING); @@ -104,6 +112,8 @@ export class BaseMetadataService { // log('info', `Downloaded ${cacheDiff} statements from ${numServers} metadata servers`); } + this.allowUnrecognizedAAGUID = allowUnrecognizedAAGUID; + this.setState(SERVICE_STATE.READY); } @@ -133,8 +143,12 @@ export class BaseMetadataService { const cachedStatement = this.statementCache[aaguid]; if (!cachedStatement) { - // TODO: FIDO conformance requires this, but it seems excessive for WebAuthn. Investigate - // later + // Allow registration verification to continue without using metadata + if (this.allowUnrecognizedAAGUID) { + return; + } + + // FIDO conformance requires RP's to only support AAGUID's that have metadata statements throw new Error(`No metadata statement found for aaguid "${aaguid}"`); } From 65e4881b1cb9fa46406ea664e1e70aec0dded111 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Tue, 31 Aug 2021 21:58:47 -0700 Subject: [PATCH 2/5] Add test for flag --- packages/server/src/services/metadataService.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/server/src/services/metadataService.test.ts b/packages/server/src/services/metadataService.test.ts index d1984f53..0f4c0691 100644 --- a/packages/server/src/services/metadataService.test.ts +++ b/packages/server/src/services/metadataService.test.ts @@ -79,6 +79,18 @@ describe('Method: getStatement()', () => { expect(err).not.toBeUndefined(); } }); + + test('should return undefined after initialization on AAGUID with no statement and allowUnrecognizedAAGUID is true', async () => { + await MetadataService.initialize({ + mdsServers: [], + statements: [], + allowUnrecognizedAAGUID: true, + }); + + const statement = await MetadataService.getStatement('not-a-real-aaguid'); + + expect(statement).toBeUndefined(); + }); }); const localStatementAAGUID = '91dfead7-959e-4475-ad26-9b0d482be089'; From bb193c61ffe40f757c10ee556150d509290b6856 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Wed, 1 Sep 2021 08:44:51 -0700 Subject: [PATCH 3/5] Switch to verificationMode enum pattern --- .../src/services/metadataService.test.ts | 4 +-- .../server/src/services/metadataService.ts | 33 ++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/server/src/services/metadataService.test.ts b/packages/server/src/services/metadataService.test.ts index 0f4c0691..b4b056eb 100644 --- a/packages/server/src/services/metadataService.test.ts +++ b/packages/server/src/services/metadataService.test.ts @@ -80,11 +80,11 @@ describe('Method: getStatement()', () => { } }); - test('should return undefined after initialization on AAGUID with no statement and allowUnrecognizedAAGUID is true', async () => { + test('should return undefined after initialization on AAGUID with no statement and verificationMode is "permissive"', async () => { await MetadataService.initialize({ mdsServers: [], statements: [], - allowUnrecognizedAAGUID: true, + verificationMode: 'permissive', }); const statement = await MetadataService.getStatement('not-a-real-aaguid'); diff --git a/packages/server/src/services/metadataService.ts b/packages/server/src/services/metadataService.ts index cda31991..9b2b12b6 100644 --- a/packages/server/src/services/metadataService.ts +++ b/packages/server/src/services/metadataService.ts @@ -36,6 +36,8 @@ enum SERVICE_STATE { READY, } +type VerificationMode = 'permissive' | 'strict'; + /** * A basic service for coordinating interactions with the FIDO Metadata Service. This includes BLOB * download and parsing, and on-demand requesting and caching of individual metadata statements. @@ -46,24 +48,31 @@ export class BaseMetadataService { private mdsCache: { [url: string]: CachedMDS } = {}; private statementCache: { [aaguid: string]: CachedBLOBEntry } = {}; private state: SERVICE_STATE = SERVICE_STATE.DISABLED; - private allowUnrecognizedAAGUID = false; + private verificationMode: VerificationMode = 'strict'; /** * Prepare the service to handle remote MDS servers and/or cache local metadata statements. + * + * **Options:** + * + * @param opts.mdsServers An array of URLs to FIDO Alliance Metadata Service + * (version 3.0)-compatible servers. Defaults to the official FIDO MDS server + * @param opts.statements An array of local metadata statements + * @param opts.verificationMode How MetadataService will handle unregistered AAGUIDs. Defaults to + * `"strict"` which throws errors when an unregistered AAGUID is encountered during registration. + * Set to `"permissive"` to allow registration by authenticators with unregistered AAGUIDs */ async initialize( opts: { mdsServers?: string[]; statements?: MetadataStatement[]; - // TODO: What to call a flag that means "don't error out when an aaguid doesn't have metadata" - // Not entirely satisfied with this name - allowUnrecognizedAAGUID?: boolean; + verificationMode?: VerificationMode; } = {}, ): Promise { const { mdsServers = [defaultURLMDS], statements, - allowUnrecognizedAAGUID = false, + verificationMode, } = opts; this.setState(SERVICE_STATE.REFRESHING); @@ -112,7 +121,9 @@ export class BaseMetadataService { // log('info', `Downloaded ${cacheDiff} statements from ${numServers} metadata servers`); } - this.allowUnrecognizedAAGUID = allowUnrecognizedAAGUID; + if (verificationMode) { + this.verificationMode = verificationMode; + } this.setState(SERVICE_STATE.READY); } @@ -143,13 +154,13 @@ export class BaseMetadataService { const cachedStatement = this.statementCache[aaguid]; if (!cachedStatement) { - // Allow registration verification to continue without using metadata - if (this.allowUnrecognizedAAGUID) { - return; + if (this.verificationMode === 'strict') { + // FIDO conformance requires RP's to only support AAGUID's that have metadata statements + throw new Error(`No metadata statement found for aaguid "${aaguid}"`); } - // FIDO conformance requires RP's to only support AAGUID's that have metadata statements - throw new Error(`No metadata statement found for aaguid "${aaguid}"`); + // Allow registration verification to continue without using metadata + return; } // If the statement points to an MDS API, check the MDS' nextUpdate to see if we need to refresh From 2037b3220549f576795a07be131ac43358d02c30 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Wed, 1 Sep 2021 08:54:09 -0700 Subject: [PATCH 4/5] Clean up some docs --- packages/server/src/services/metadataService.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/server/src/services/metadataService.ts b/packages/server/src/services/metadataService.ts index 9b2b12b6..86cd10d9 100644 --- a/packages/server/src/services/metadataService.ts +++ b/packages/server/src/services/metadataService.ts @@ -36,6 +36,8 @@ enum SERVICE_STATE { READY, } +// Allow MetadataService to accommodate unregistered AAGUIDs ("permissive"), or only allow +// registered AAGUIDs ("strict"). Currently primarily impacts how `getStatement()` operates type VerificationMode = 'permissive' | 'strict'; /** @@ -59,8 +61,9 @@ export class BaseMetadataService { * (version 3.0)-compatible servers. Defaults to the official FIDO MDS server * @param opts.statements An array of local metadata statements * @param opts.verificationMode How MetadataService will handle unregistered AAGUIDs. Defaults to - * `"strict"` which throws errors when an unregistered AAGUID is encountered during registration. - * Set to `"permissive"` to allow registration by authenticators with unregistered AAGUIDs + * `"strict"` which throws errors during registration response verification when an + * unregistered AAGUID is encountered. Set to `"permissive"` to allow registration by + * authenticators with unregistered AAGUIDs */ async initialize( opts: { @@ -129,7 +132,7 @@ export class BaseMetadataService { } /** - * Get a metadata statement for a given aaguid. Defaults to returning a cached statement. + * Get a metadata statement for a given AAGUID. * * This method will coordinate updating the cache as per the `nextUpdate` property in the initial * BLOB download. From 1455149878bd67cf92c1b1c5b14e5fdffd583e19 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Wed, 1 Sep 2021 09:04:34 -0700 Subject: [PATCH 5/5] Clarify strict mode verification error --- packages/server/src/services/metadataService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/services/metadataService.ts b/packages/server/src/services/metadataService.ts index 86cd10d9..3eac8a18 100644 --- a/packages/server/src/services/metadataService.ts +++ b/packages/server/src/services/metadataService.ts @@ -158,7 +158,7 @@ export class BaseMetadataService { if (!cachedStatement) { if (this.verificationMode === 'strict') { - // FIDO conformance requires RP's to only support AAGUID's that have metadata statements + // FIDO conformance requires RP's to only support registered AAGUID's throw new Error(`No metadata statement found for aaguid "${aaguid}"`); }