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

fix/513-cbor-x-type-issues #521

Merged
merged 2 commits into from
Feb 16, 2024
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: 4 additions & 5 deletions packages/server/build_npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ await build({
name: '@hexagon/base64',
version: '^1.1.27',
},
'https://deno.land/x/cbor@v1.5.2/index.js': {
name: 'cbor-x',
subPath: 'index-no-eval',
version: '^1.5.2',
},
'https://esm.sh/cross-fetch@4.0.0': {
name: 'cross-fetch',
version: '^4.0.0',
Expand All @@ -99,6 +94,10 @@ await build({
name: '@peculiar/asn1-android',
version: '^2.3.10',
},
'https://deno.land/x/tiny_cbor@0.2.2/index.ts': {
name: '@levischuck/tiny-cbor',
version: '^0.2.2',
},
// Mapping for '../../types/src/index.ts' in deps.ts
'../types/src/index.ts': {
name: '@simplewebauthn/types',
Expand Down
6 changes: 3 additions & 3 deletions packages/server/deno.lock
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@
"https://deno.land/std@0.198.0/testing/mock.ts": "4c52b8312d159179fdd9d9a1b35e342ee4e1a1248f29e5c7f57fb4011c3f55ed",
"https://deno.land/std@0.198.0/testing/time.ts": "a46fbfd61e6f011f15a63c8078399b1f7fa848d2c0c526f253b0535f5c3e7f45",
"https://deno.land/x/b64@1.1.27/src/base64.js": "9e10b98e4203d030bc913913a2e4683b4842aff337bc9ec2643a28dfe04b5fc4",
"https://deno.land/x/cbor@v1.5.2/decode.js": "ab5518450c1cc3d8e3be7a772de8008d2a4f92626630eed6fabd66a25f565526",
"https://deno.land/x/cbor@v1.5.2/encode.js": "80da1bb1c2936bba0b53e7e0945d73c5a55ed62ea659cf3d785514310b4e3d37",
"https://deno.land/x/cbor@v1.5.2/index.js": "cc8678819d77aa34b6fa9293658d85d5e53e53eaf555f85b0f98a8a18dbfaa12",
"https://deno.land/x/code_block_writer@12.0.0/mod.ts": "2c3448060e47c9d08604c8f40dee34343f553f33edcdfebbf648442be33205e5",
"https://deno.land/x/code_block_writer@12.0.0/utils/string_utils.ts": "60cb4ec8bd335bf241ef785ccec51e809d576ff8e8d29da43d2273b69ce2a6ff",
"https://deno.land/x/deno_cache@0.4.1/auth_tokens.ts": "5fee7e9155e78cedf3f6ff3efacffdb76ac1a76c86978658d9066d4fb0f7326e",
Expand Down Expand Up @@ -127,6 +124,9 @@
"https://deno.land/x/dnt@0.38.0/lib/utils.ts": "878b7ac7003a10c16e6061aa49dbef9b42bd43174853ebffc9b67ea47eeb11d8",
"https://deno.land/x/dnt@0.38.0/mod.ts": "b13349fe77847cf58e26b40bcd58797a8cec5d71b31a1ca567071329c8489de1",
"https://deno.land/x/dnt@0.38.0/transform.ts": "f68743a14cf9bf53bfc9c81073871d69d447a7f9e3453e0447ca2fb78926bb1d",
"https://deno.land/x/tiny_cbor@0.2.2/cbor/cbor.ts": "b10fc7cd694887111cbcb9da87f0235cc954ee377b7122af3bbaccd422460e72",
"https://deno.land/x/tiny_cbor@0.2.2/cbor/cbor_internal.ts": "0cde2b1d2cb08503b6f44c1789736376eac2f5d0ed10db9f73b16caa356c8bd7",
"https://deno.land/x/tiny_cbor@0.2.2/index.ts": "a45c8abd3874ab05dea33af17b2da88376364d6966fcbf2c5da5520a0a9b9d98",
"https://deno.land/x/ts_morph@18.0.0/bootstrap/mod.ts": "b53aad517f106c4079971fcd4a81ab79fadc40b50061a3ab2b741a09119d51e9",
"https://deno.land/x/ts_morph@18.0.0/bootstrap/ts_morph_bootstrap.js": "6645ac03c5e6687dfa8c78109dc5df0250b811ecb3aea2d97c504c35e8401c06",
"https://deno.land/x/ts_morph@18.0.0/common/DenoRuntime.ts": "6a7180f0c6e90dcf23ccffc86aa8271c20b1c4f34c570588d08a45880b7e172d",
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export type {
UserVerificationRequirement,
} from '../../types/src/index.ts';

// cbor (a.k.a. cbor-x in Node land)
export * as cborx from 'https://deno.land/x/cbor@v1.5.2/index.js';
// tiny_cbor (a.k.a. tiny-cbor in Node land)
export * as tinyCbor from 'https://deno.land/x/tiny_cbor@0.2.2/index.ts';

// b64 (a.k.a. @hexagon/base64 in Node land)
export { default as base64 } from 'https://deno.land/x/b64@1.1.27/src/base64.js';
Expand Down
24 changes: 5 additions & 19 deletions packages/server/src/helpers/iso/isoCBOR.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { cborx } from '../../deps.ts';
import { tinyCbor } from '../../deps.ts';

/**
* This encoder should keep CBOR data the same length when data is re-encoded
* Whatever CBOR encoder is used should keep CBOR data the same length when data is re-encoded
*
* MOST CRITICALLY, this means the following needs to be true of whatever CBOR library we use:
* - CBOR Map type values MUST decode to JavaScript Maps
Expand All @@ -10,10 +10,6 @@ import { cborx } from '../../deps.ts';
* So long as these requirements are maintained, then CBOR sequences can be encoded and decoded
* freely while maintaining their lengths for the most accurate pointer movement across them.
*/
const encoder = new cborx.Encoder({
mapsAsObjects: false,
tagUint8Array: false,
});

/**
* Decode and return the first item in a sequence of CBOR-encoded values
Expand All @@ -25,18 +21,8 @@ const encoder = new cborx.Encoder({
export function decodeFirst<Type>(input: Uint8Array): Type {
// Make a copy so we don't mutate the original
const _input = new Uint8Array(input);
const decoded = encoder.decodeMultiple(_input) as undefined | Type[];
const decoded = tinyCbor.decodePartialCBOR(_input, 0) as [Type, number];

if (decoded === undefined) {
throw new Error('CBOR input data was empty');
}

/**
* Typing on `decoded` is `void | []` which causes TypeScript to think that it's an empty array,
* and thus you can't destructure it. I'm ignoring that because the code works fine in JS, and
* so this should be a valid operation.
*/
// @ts-ignore 2493
const [first] = decoded;

return first;
Expand All @@ -45,6 +31,6 @@ export function decodeFirst<Type>(input: Uint8Array): Type {
/**
* Encode data to CBOR
*/
export function encode(input: unknown): Uint8Array {
return encoder.encode(input);
export function encode(input: tinyCbor.CBORType): Uint8Array {
return tinyCbor.encodeCBOR(input);
}
19 changes: 17 additions & 2 deletions packages/server/src/helpers/parseAuthenticatorData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,17 @@ export function parseAuthenticatorData(
const firstDecoded = isoCBOR.decodeFirst<COSEPublicKey>(
authData.slice(pointer),
);
const firstEncoded = Uint8Array.from(isoCBOR.encode(firstDecoded));
const firstEncoded = Uint8Array.from(
/**
* Casting to `Map` via `as unknown` here because TS doesn't make it possible to define Maps
* with discrete keys and properties with known types per pair, and CBOR libs typically parse
* CBOR Major Type 5 to `Map` because you can have numbers for keys. A `COSEPublicKey` can be
* generalized as "a Map with numbers for keys and either numbers or bytes for values" though.
* If this presumption falls apart then other parts of verification later on will fail so we
* should be safe doing this here.
*/
isoCBOR.encode(firstDecoded as unknown as Map<number, number | Uint8Array>),
);

if (foundBadCBOR) {
// Restore the bit we changed so that `authData` is the same as it came in and won't break
Expand All @@ -92,7 +102,12 @@ export function parseAuthenticatorData(
let extensionsDataBuffer: Uint8Array | undefined = undefined;

if (flags.ed) {
const firstDecoded = isoCBOR.decodeFirst(authData.slice(pointer));
/**
* Typing here feels a little sloppy but we're immediately CBOR-encoding this back to bytes to
* more diligently parse via `decodeAuthenticatorExtensions()` so :shrug:
*/
type AuthenticatorExtensionData = Map<string, Uint8Array>;
const firstDecoded = isoCBOR.decodeFirst<AuthenticatorExtensionData>(authData.slice(pointer));
extensionsDataBuffer = Uint8Array.from(isoCBOR.encode(firstDecoded));
extensionsData = decodeAuthenticatorExtensions(extensionsDataBuffer);
pointer += extensionsDataBuffer.byteLength;
Expand Down
Loading