Skip to content

Commit fe671c9

Browse files
SiegriftSiegrift
authored
Do not validate Signed API responses from config (#301)
* Do not validate Signed API responses from config * Improve readability * Add tsreset to tsconfig build
1 parent f663e7a commit fe671c9

15 files changed

+131
-59
lines changed

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"@api3/ois": "^2.3.2",
5353
"@nomicfoundation/hardhat-ethers": "^3.0.5",
5454
"@nomicfoundation/hardhat-toolbox": "^5.0.0",
55+
"@total-typescript/ts-reset": "^0.5.1",
5556
"@types/jest": "^29.5.12",
5657
"@types/lodash": "^4.17.1",
5758
"@types/node": "^20.12.11",

‎pnpm-lock.yaml

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎reset.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Do not add any other lines of code to this file!
2+
import '@total-typescript/ts-reset';

‎scripts/create-docker-release.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const execSyncWithErrorHandling = (command: string) => {
2323
export const isMacOrWindows = () => process.platform === 'win32' || process.platform === 'darwin' || isWsl;
2424

2525
const main = () => {
26-
const { version } = JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8'));
26+
const { version } = JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')) as any;
2727
console.info(`Building docker image with semantic version ${version}...`);
2828

2929
if (isMacOrWindows()) {

‎scripts/create-npm-release.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ const main = () => {
4747
console.info('Making sure we have the latest version of the dependencies...');
4848
execSyncWithErrorHandling('pnpm install');
4949

50-
const currentVersion = JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')).version;
50+
const currentVersion = (JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')) as any).version;
5151
console.info(`Current version is ${currentVersion}...`);
5252

5353
console.info('Bumping root package.json version...');
5454
execSyncWithErrorHandling(`pnpm version --no-git-tag-version ${versionBump}`);
55-
const newVersion = JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')).version;
55+
const newVersion = (JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')) as any).version;
5656

5757
console.info('Updating versions in example files and fixtures...');
5858
const replacements = [

‎src/data-fetcher-loop/data-fetcher-loop.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ describe(dataFetcherLoopModule.runDataFetcher.name, () => {
1313
beforeEach(() => {
1414
initializeState();
1515
updateState((draft) => {
16-
draft.signedApiUrls = {
16+
draft.signedApiUrlsFromConfig = {
1717
'31337': { hardhat: ['http://127.0.0.1:8090/0xC04575A2773Da9Cd23853A69694e02111b2c4182'] },
1818
};
19+
draft.signedApiUrlsFromContract = {
20+
'31337': { hardhat: [] },
21+
};
1922
draft.activeDataFeedBeaconIds = {
2023
'31337': {
2124
hardhat: [

‎src/data-fetcher-loop/data-fetcher-loop.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ export const runDataFetcher = async () => {
6060
const state = getState();
6161
const {
6262
config: { signedDataFetchInterval },
63-
signedApiUrls,
63+
signedApiUrlsFromConfig,
64+
signedApiUrlsFromContract,
6465
activeDataFeedBeaconIds,
6566
} = state;
6667
const signedDataFetchIntervalMs = signedDataFetchInterval * 1000;
@@ -73,10 +74,17 @@ export const runDataFetcher = async () => {
7374
.flat(2)
7475
);
7576

77+
// Compute the set of URLs coming from the config. These are trusted and don't need to be verified.
78+
const trustedUrls = new Set(
79+
Object.values(signedApiUrlsFromConfig)
80+
.map((urlsPerProvider) => Object.values(urlsPerProvider))
81+
.flat(2)
82+
);
83+
7684
// Better to log the non-decomposed object to see which URL comes from which chain-provider group.
77-
logger.debug('Signed API URLs.', { signedApiUrls });
85+
logger.debug('Signed API URLs.', { signedApiUrlsFromConfig, signedApiUrlsFromContract });
7886
const urls = uniq(
79-
Object.values(signedApiUrls)
87+
[...Object.values(signedApiUrlsFromConfig), ...Object.values(signedApiUrlsFromContract)]
8088
.map((urlsPerProvider) => Object.values(urlsPerProvider))
8189
.flat(2)
8290
);
@@ -100,7 +108,10 @@ export const runDataFetcher = async () => {
100108
const signedDataForActiveBeacons = Object.entries(signedDataBatch).filter(([beaconId]) =>
101109
activeBeaconIds.has(beaconId as Hex)
102110
);
103-
const signedDataCount = await saveSignedData(signedDataForActiveBeacons as SignedDataRecordEntry[]);
111+
const signedDataCount = await saveSignedData(
112+
signedDataForActiveBeacons as SignedDataRecordEntry[],
113+
trustedUrls.has(url)
114+
);
104115
logger.info('Saved signed data from Signed API using a worker.', {
105116
url,
106117
duration: Date.now() - now,

‎src/data-fetcher-loop/signed-data-state.test.ts

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe(signedDataStateModule.saveSignedData.name, () => {
3535
jest.spyOn(signedDataVerifierPoolModule, 'getVerifier').mockResolvedValue(createMockSignedDataVerifier());
3636
const beaconId = deriveBeaconId(validSignedData.airnode, validSignedData.templateId) as Hex;
3737

38-
await signedDataStateModule.saveSignedData([[beaconId, validSignedData]]);
38+
await signedDataStateModule.saveSignedData([[beaconId, validSignedData]], false);
3939

4040
const signedData = signedDataStateModule.getSignedData(beaconId);
4141
expect(signedData).toStrictEqual(validSignedData);
@@ -54,7 +54,7 @@ describe(signedDataStateModule.saveSignedData.name, () => {
5454
});
5555
jest.spyOn(logger, 'debug');
5656

57-
await signedDataStateModule.saveSignedData([[beaconId, validSignedData]]);
57+
await signedDataStateModule.saveSignedData([[beaconId, validSignedData]], false);
5858

5959
expect(signedDataStateModule.getSignedData(beaconId)).toStrictEqual(storedSignedData);
6060
expect(logger.debug).toHaveBeenCalledTimes(1);
@@ -80,7 +80,7 @@ describe(signedDataStateModule.saveSignedData.name, () => {
8080
jest.spyOn(logger, 'warn');
8181
jest.spyOn(logger, 'error');
8282

83-
await signedDataStateModule.saveSignedData([[beaconId, futureSignedData]]);
83+
await signedDataStateModule.saveSignedData([[beaconId, futureSignedData]], false);
8484

8585
expect(signedDataStateModule.getSignedData(beaconId)).toBeUndefined();
8686
expect(logger.error).toHaveBeenCalledTimes(1);
@@ -108,7 +108,7 @@ describe(signedDataStateModule.saveSignedData.name, () => {
108108
jest.spyOn(logger, 'warn');
109109
jest.spyOn(logger, 'error');
110110

111-
await signedDataStateModule.saveSignedData([[beaconId, futureSignedData]]);
111+
await signedDataStateModule.saveSignedData([[beaconId, futureSignedData]], false);
112112

113113
expect(signedDataStateModule.getSignedData(deriveBeaconId(airnode, templateId) as Hex)).toStrictEqual(
114114
futureSignedData
@@ -139,13 +139,39 @@ describe(signedDataStateModule.saveSignedData.name, () => {
139139
};
140140
const beaconId = deriveBeaconId(airnode, templateId) as Hex;
141141

142-
await signedDataStateModule.saveSignedData([[beaconId, badSignedData]]);
142+
await signedDataStateModule.saveSignedData([[beaconId, badSignedData]], false);
143143

144144
expect(signedDataStateModule.getSignedData(deriveBeaconId(airnode, templateId) as Hex)).toBeUndefined();
145145
expect(logger.error).toHaveBeenCalledTimes(1);
146146
expect(logger.error).toHaveBeenCalledWith('Failed to verify signed data.', badSignedData);
147147
expect(logger.warn).toHaveBeenCalledTimes(0);
148148
});
149+
150+
it('only verifies the signed data for untrusted APIs', async () => {
151+
const templateId = generateRandomBytes(32);
152+
const airnode = ethers.Wallet.createRandom().address as Hex;
153+
jest.spyOn(signedDataVerifierPoolModule, 'getVerifier').mockResolvedValue(createMockSignedDataVerifier());
154+
jest.spyOn(logger, 'warn');
155+
jest.spyOn(logger, 'error');
156+
const beaconId = deriveBeaconId(airnode, templateId) as Hex;
157+
158+
// First save data for trusted API and expect it to be saved without verification.
159+
await signedDataStateModule.saveSignedData([[beaconId, validSignedData]], true);
160+
expect(signedDataVerifierPoolModule.getVerifier).not.toHaveBeenCalled();
161+
162+
// Then save data for untrusted API and expect it to be verified.
163+
const timestamp = Math.floor((Date.now() - 30 * 60 * 1000) / 1000).toString();
164+
const encodedValue = ethers.AbiCoder.defaultAbiCoder().encode(['int256'], [123n]);
165+
const otherSignedData = {
166+
airnode,
167+
encodedValue,
168+
signature: await signData(signer, templateId, timestamp, encodedValue),
169+
templateId,
170+
timestamp,
171+
};
172+
await signedDataStateModule.saveSignedData([[beaconId, otherSignedData]], false);
173+
expect(signedDataVerifierPoolModule.getVerifier).toHaveBeenCalledTimes(1);
174+
});
149175
});
150176

151177
describe(signedDataStateModule.purgeOldSignedData.name, () => {

‎src/data-fetcher-loop/signed-data-state.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,23 @@ const verifyTimestamp = ([beaconId, signedData]: SignedDataRecordEntry) => {
4040
return true;
4141
};
4242

43-
export const saveSignedData = async (signedDataBatch: SignedDataRecordEntry[]) => {
43+
export const saveSignedData = async (signedDataBatch: SignedDataRecordEntry[], isTrustedApi: boolean) => {
4444
// Filter out signed data with invalid timestamps or we already have a fresher signed data stored in state.
4545
signedDataBatch = signedDataBatch.filter((signedDataEntry) => verifyTimestamp(signedDataEntry));
4646
if (signedDataBatch.length === 0) return;
4747

48-
const verifier = await getVerifier();
49-
// We are skipping the whole batch even if there is only one invalid signed data. This is consistent with the Signed
50-
// API approach.
51-
const verificationResult = await verifier.verifySignedData(signedDataBatch);
52-
if (verificationResult !== true) {
53-
logger.error('Failed to verify signed data.', verificationResult);
54-
return;
48+
// Only verify the signed data if it is not coming from a trusted API.
49+
if (!isTrustedApi) {
50+
const verifier = await getVerifier();
51+
// We are skipping the whole batch even if there is only one invalid signed data. This is consistent with the Signed
52+
// API approach.
53+
const verificationResult = await verifier.verifySignedData(signedDataBatch);
54+
if (verificationResult !== true) {
55+
logger.error('Failed to verify signed data.', verificationResult);
56+
return;
57+
}
5558
}
59+
5660
updateState((draft) => {
5761
for (const [beaconId, signedData] of signedDataBatch) {
5862
draft.signedDatas[beaconId] = signedData;

‎src/state/state.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ const stateMock: State = {
3535
timestamp: 'something-silly',
3636
},
3737
},
38-
signedApiUrls: { '31337': { hardhat: ['http://127.0.0.1:8090/0xC04575A2773Da9Cd23853A69694e02111b2c4182'] } },
38+
signedApiUrlsFromConfig: {
39+
'31337': { hardhat: ['http://127.0.0.1:8090/0xC04575A2773Da9Cd23853A69694e02111b2c4182'] },
40+
},
41+
signedApiUrlsFromContract: { '31337': { hardhat: [] } },
3942
derivedSponsorWallets: {},
4043
deploymentTimestamp: '1687850583',
4144
activeDataFeedBeaconIds: { '31337': { hardhat: [] } },

0 commit comments

Comments
 (0)