Skip to content

Commit 76129fd

Browse files
bitgoAaronbitgoAaron
committed
fix: web crypto strategy uses same hmac subject calc
use same calculateHMACSubject function for getting the subject to sign as regular hmac flows split up IndexedDB class to separate file & tests other small changes from review Ticket: CE-10122
1 parent e57bd5b commit 76129fd

7 files changed

Lines changed: 457 additions & 215 deletions

File tree

‎modules/sdk-api/src/bitgoAPI.ts‎

Lines changed: 87 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,84 @@ export class BitGoAPI implements BitGoBase {
377377
return this._authVersion;
378378
}
379379

380+
/**
381+
* Signs and sends a v2-authenticated request, then verifies the response HMAC.
382+
* Extracted from the req.then override in requestPatch to keep that method readable.
383+
*/
384+
private async _sendRequestWithHmac({
385+
req,
386+
method,
387+
url,
388+
data,
389+
strategyAuthenticated,
390+
onfulfilled,
391+
originalThen,
392+
}: {
393+
req: superagent.SuperAgentRequest;
394+
method: RequestMethods;
395+
url: string;
396+
data: string | undefined;
397+
strategyAuthenticated: boolean;
398+
onfulfilled: ((response: superagent.Response) => any) | null | undefined;
399+
originalThen: (onfulfilled: any, onrejected?: any) => Promise<any>;
400+
}): Promise<any> {
401+
if (this._token || strategyAuthenticated) {
402+
setRequestQueryString(req);
403+
404+
const requestProperties = await this._hmacAuthStrategy.calculateRequestHeaders({
405+
url: req.url,
406+
token: this._token ?? '',
407+
method,
408+
text: data || '',
409+
authVersion: this._authVersion,
410+
});
411+
req.set('Auth-Timestamp', requestProperties.timestamp.toString());
412+
413+
req.set('Authorization', 'Bearer ' + requestProperties.tokenHash);
414+
debug(
415+
'sending v2 %s request to %s with token %s',
416+
method,
417+
url,
418+
this._token?.substr(0, 8) ?? '(strategy-managed)'
419+
);
420+
421+
req.set('HMAC', requestProperties.hmac);
422+
}
423+
424+
if (this.getAdditionalHeadersCb) {
425+
const additionalHeaders = this.getAdditionalHeadersCb(method, url, data);
426+
for (const { key, value } of additionalHeaders) {
427+
req.set(key, value);
428+
}
429+
}
430+
431+
/**
432+
* Verify the response before calling the original onfulfilled handler,
433+
* and make sure onrejected is called if a verification error is encountered
434+
*/
435+
const newOnFulfilled = onfulfilled
436+
? async (response: superagent.Response) => {
437+
// HMAC verification is only allowed to be skipped in certain environments.
438+
// This is checked in the constructor, but checking it again at request time
439+
// will help prevent against tampering of this property after the object is created
440+
if (!this._hmacVerification && !common.Environments[this.getEnv()].hmacVerificationEnforced) {
441+
return onfulfilled(response);
442+
}
443+
444+
const verifiedResponse = await verifyResponseAsync(
445+
this,
446+
this._token,
447+
method,
448+
req,
449+
response,
450+
this._authVersion
451+
);
452+
return onfulfilled(verifiedResponse);
453+
}
454+
: null;
455+
return originalThen(newOnFulfilled);
456+
}
457+
380458
/**
381459
* This is a patching function which can apply our authorization
382460
* headers to any outbound request.
@@ -446,65 +524,15 @@ export class BitGoAPI implements BitGoBase {
446524

447525
const data = serializeRequestData(req);
448526

449-
const sendWithHmac = (async () => {
450-
if (this._token || strategyAuthenticated) {
451-
setRequestQueryString(req);
452-
453-
const requestProperties = await this._hmacAuthStrategy.calculateRequestHeaders({
454-
url: req.url,
455-
token: this._token ?? '',
456-
method,
457-
text: data || '',
458-
authVersion: this._authVersion,
459-
});
460-
req.set('Auth-Timestamp', requestProperties.timestamp.toString());
461-
462-
req.set('Authorization', 'Bearer ' + requestProperties.tokenHash);
463-
debug(
464-
'sending v2 %s request to %s with token %s',
465-
method,
466-
url,
467-
this._token?.substr(0, 8) ?? '(strategy-managed)'
468-
);
469-
470-
req.set('HMAC', requestProperties.hmac);
471-
}
472-
473-
if (this.getAdditionalHeadersCb) {
474-
const additionalHeaders = this.getAdditionalHeadersCb(method, url, data);
475-
for (const { key, value } of additionalHeaders) {
476-
req.set(key, value);
477-
}
478-
}
479-
480-
/**
481-
* Verify the response before calling the original onfulfilled handler,
482-
* and make sure onrejected is called if a verification error is encountered
483-
*/
484-
const newOnFulfilled = onfulfilled
485-
? async (response: superagent.Response) => {
486-
// HMAC verification is only allowed to be skipped in certain environments.
487-
// This is checked in the constructor, but checking it again at request time
488-
// will help prevent against tampering of this property after the object is created
489-
if (!this._hmacVerification && !common.Environments[this.getEnv()].hmacVerificationEnforced) {
490-
return onfulfilled(response);
491-
}
492-
493-
const verifiedResponse = await verifyResponseAsync(
494-
this,
495-
this._token,
496-
method,
497-
req,
498-
response,
499-
this._authVersion
500-
);
501-
return onfulfilled(verifiedResponse);
502-
}
503-
: null;
504-
return originalThen(newOnFulfilled);
505-
})();
506-
507-
return sendWithHmac.catch(onrejected);
527+
return this._sendRequestWithHmac({
528+
req,
529+
method,
530+
url,
531+
data,
532+
strategyAuthenticated,
533+
onfulfilled,
534+
originalThen,
535+
}).catch(onrejected);
508536
};
509537
return toBitgoRequest(req);
510538
}

‎modules/sdk-hmac/src/browser.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export * from './index';
2+
export * from './indexedDbTokenStore';
23
export * from './webCryptoStrategy';

‎modules/sdk-hmac/src/indexedDbTokenStore.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import type { CryptoSigning, ITokenStore } from './types';
2+
3+
const CRYPTO_DB_NAME = 'bitgo-auth';
4+
const CRYPTO_STORE_NAME = 'crypto-signing';
5+
const CRYPTO_RECORD_KEY = 'current';
6+
7+
function hasIndexedDB(): boolean {
8+
return typeof indexedDB !== 'undefined';
9+
}
10+
11+
function openCryptoDb(): Promise<IDBDatabase> {
12+
return new Promise((resolve, reject) => {
13+
const request = indexedDB.open(CRYPTO_DB_NAME, 1);
14+
request.onupgradeneeded = () => {
15+
const db = request.result;
16+
if (!db.objectStoreNames.contains(CRYPTO_STORE_NAME)) {
17+
db.createObjectStore(CRYPTO_STORE_NAME);
18+
}
19+
};
20+
request.onsuccess = () => resolve(request.result);
21+
request.onerror = () => reject(request.error);
22+
});
23+
}
24+
25+
async function withDb<T>(fn: (db: IDBDatabase) => Promise<T>): Promise<T> {
26+
const db = await openCryptoDb();
27+
try {
28+
return await fn(db);
29+
} finally {
30+
db.close();
31+
}
32+
}
33+
34+
async function persistCryptoSigning(signing: CryptoSigning): Promise<void> {
35+
if (!hasIndexedDB()) return;
36+
await withDb(
37+
(db) =>
38+
new Promise<void>((resolve, reject) => {
39+
const tx = db.transaction(CRYPTO_STORE_NAME, 'readwrite');
40+
tx.objectStore(CRYPTO_STORE_NAME).put(signing, CRYPTO_RECORD_KEY);
41+
tx.oncomplete = () => resolve();
42+
tx.onerror = () => reject(tx.error);
43+
})
44+
);
45+
}
46+
47+
async function loadCryptoSigning(): Promise<CryptoSigning | null> {
48+
if (!hasIndexedDB()) return null;
49+
return withDb(
50+
(db) =>
51+
new Promise<CryptoSigning | null>((resolve, reject) => {
52+
const tx = db.transaction(CRYPTO_STORE_NAME, 'readonly');
53+
const request = tx.objectStore(CRYPTO_STORE_NAME).get(CRYPTO_RECORD_KEY);
54+
request.onsuccess = () => resolve(request.result ?? null);
55+
request.onerror = () => reject(request.error);
56+
})
57+
);
58+
}
59+
60+
async function removeCryptoSigning(): Promise<void> {
61+
if (!hasIndexedDB()) return;
62+
await withDb(
63+
(db) =>
64+
new Promise<void>((resolve, reject) => {
65+
const tx = db.transaction(CRYPTO_STORE_NAME, 'readwrite');
66+
tx.objectStore(CRYPTO_STORE_NAME).delete(CRYPTO_RECORD_KEY);
67+
tx.oncomplete = () => resolve();
68+
tx.onerror = () => reject(tx.error);
69+
})
70+
);
71+
}
72+
73+
/**
74+
* Persists {@link CryptoSigning} material in the browser's IndexedDB.
75+
* The raw bearer token is never stored — only the non-extractable CryptoKey
76+
* and the SHA-256 token hash are persisted via the structured clone algorithm.
77+
*/
78+
export class IndexedDbTokenStore implements ITokenStore {
79+
async save(signing: CryptoSigning): Promise<void> {
80+
await persistCryptoSigning(signing);
81+
}
82+
83+
async load(): Promise<CryptoSigning | null> {
84+
return loadCryptoSigning();
85+
}
86+
87+
async remove(): Promise<void> {
88+
await removeCryptoSigning();
89+
}
90+
}

‎modules/sdk-hmac/src/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ export interface CalculateRequestHeadersOptions<T extends string | Buffer = stri
2828
authVersion: AuthVersion;
2929
}
3030

31+
export type CalculateRequestHeadersWebCryptoOptions<T extends string | Buffer = string> = Omit<
32+
CalculateRequestHeadersOptions<T>,
33+
'token'
34+
>;
35+
3136
export interface RequestHeaders {
3237
hmac: string;
3338
timestamp: number;

0 commit comments

Comments
 (0)