Skip to content

Commit

Permalink
fixing watermark / mitm vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
krpeacock committed Jul 22, 2024
1 parent 0315037 commit 81ad181
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 11 deletions.
3 changes: 2 additions & 1 deletion e2e/node/basic/watermark.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ test('replay attack', async () => {
expect(startValue3).toBe(1n);

const queryResponseIndex = indexOfQueryResponse(fetchProxy.history);
console.log(queryResponseIndex);

fetchProxy.replayFromHistory(queryResponseIndex);

Expand All @@ -128,5 +129,5 @@ test('replay attack', async () => {
);

// The agent should should have made 4 additional requests (3 retries + 1 original request)
expect(fetchProxy.calls).toBe(11);
expect(fetchProxy.calls).toBe(9);
}, 10_000);
4 changes: 2 additions & 2 deletions e2e/node/integration/actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ test("Legacy Agent interface should be accepted by Actor's createActor", async (
);

// Verify that update calls work
await actor.write(8n); //?
await actor.write(8n);
// Verify that query calls work
const count = await actor.read(); //?
const count = await actor.read();
expect(count).toBe(8n);
}, 15_000);
// TODO: tests for rejected, unknown time out
9 changes: 6 additions & 3 deletions packages/agent/src/actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describe('makeActor', () => {
expect(reply).toEqual(canisterDecodedReturnValue);
expect(replyUpdate).toEqual(canisterDecodedReturnValue);
expect(replyWithHttpDetails.result).toEqual(canisterDecodedReturnValue);
replyWithHttpDetails.httpDetails['requestDetails']; //?
replyWithHttpDetails.httpDetails['requestDetails'];
expect(replyWithHttpDetails.httpDetails).toMatchInlineSnapshot(`
{
"headers": [],
Expand Down Expand Up @@ -330,7 +330,7 @@ describe('makeActor', () => {
`);
expect(replyUpdateWithHttpDetails.result).toEqual(canisterDecodedReturnValue);

replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); //?
replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array();

expect(replyUpdateWithHttpDetails.httpDetails).toMatchSnapshot();
});
Expand Down Expand Up @@ -382,12 +382,15 @@ test('v3 call', async () => {

const idlFactory = ({ IDL }) => {
return IDL.Service({
write: IDL.Func([IDL.Nat], [], []),
inc_read: IDL.Func([], [IDL.Nat], []),
});
};
const actor = Actor.createActor(idlFactory, {
canisterId: Principal.fromText('bkyz2-fmaaa-aaaaa-qaaaq-cai'),
agent,
});
await actor.inc_read(); //?
await actor.write(0n);
const result = await actor.inc_read();
expect(result).toBe(1n);
});
1 change: 1 addition & 0 deletions packages/agent/src/agent/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export interface SubmitResponse {
error_code?: string;
reject_code: number;
reject_message: string;
// Available in a v3 call response
certificate?: ArrayBuffer;
} | null;
headers: HttpHeaderField[];
Expand Down
15 changes: 10 additions & 5 deletions packages/agent/src/agent/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ export class HttpAgent implements Agent {
transformedRequest = await id.transformRequest(transformedRequest);

const body = cbor.encode(transformedRequest.body);

const backoff = this.#backoffStrategy();
// Attempt v3 sync call
const requestSync = this.#requestAndRetry({
request: () =>
Expand All @@ -472,18 +472,17 @@ export class HttpAgent implements Agent {
...transformedRequest.request,
body,
}),
backoff: this.#backoffStrategy(),
backoff,
tries: 0,
});

this.log.print(
`fetching "/api/v2/canister/${ecid.toText()}/call" with request:`,
`fetching "/api/v3/canister/${ecid.toText()}/call" with request:`,
transformedRequest,
);

// Run both in parallel. The fetch is quite expensive, so we have plenty of time to
// calculate the requestId locally.
const backoff = this.#backoffStrategy();
// const request = this.#requestAndRetry({
// request: () =>
// this.#fetch('' + new URL(`/api/v2/canister/${ecid.toText()}/call`, this.host), {
Expand All @@ -502,6 +501,12 @@ export class HttpAgent implements Agent {
response.status === 200 && responseBuffer.byteLength > 0 ? cbor.decode(responseBuffer) : null
) as SubmitResponse['response']['body'];

// Update the watermark with the latest time from consensus
if (responseBody?.certificate) {
const time = await this.parseTimeFromResponse({ certificate: responseBody.certificate });
this.#waterMark = time;
}

return {
requestId,
response: {
Expand Down Expand Up @@ -969,7 +974,7 @@ export class HttpAgent implements Agent {
return decodedResponse;
}

public async parseTimeFromResponse(response: ReadStateResponse): Promise<number> {
public async parseTimeFromResponse(response: { certificate: ArrayBuffer }): Promise<number> {
let tree: HashTree;
if (response.certificate) {
const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate);
Expand Down

0 comments on commit 81ad181

Please sign in to comment.