Skip to content

feat: enable clockSkew correction by default #459

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

Merged
merged 9 commits into from
Nov 26, 2019
10 changes: 9 additions & 1 deletion packages/middleware-signing/src/configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@ export interface AwsAuthInputConfig {
signer?: RequestSigner;

/**
* Whether to escape request path when signing the request
* Whether to escape request path when signing the request.
*/
signingEscapePath?: boolean;

/**
* An offset value in milliseconds to apply to all signing times.
*/
systemClockOffset?: number;
}
interface PreviouslyResolved {
credentialDefaultProvider: (input: any) => Provider<Credentials>;
Expand All @@ -32,6 +37,7 @@ export interface AwsAuthResolvedConfig {
credentials: Provider<Credentials>;
signer: RequestSigner;
signingEscapePath: boolean;
systemClockOffset: number;
}
export function resolveAwsAuthConfig<T>(
input: T & AwsAuthInputConfig & PreviouslyResolved
Expand All @@ -40,8 +46,10 @@ export function resolveAwsAuthConfig<T>(
input.credentials || input.credentialDefaultProvider(input as any);
const normalizedCreds = normalizeProvider(credentials);
const signingEscapePath = input.signingEscapePath || false;
const systemClockOffset = input.systemClockOffset || 0;
return {
...input,
systemClockOffset,
signingEscapePath,
credentials: normalizedCreds,
signer: new SignatureV4({
Expand Down
123 changes: 120 additions & 3 deletions packages/middleware-signing/src/middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,24 @@ import { HttpRequest } from "@aws-sdk/protocol-http";

describe("SigningHandler", () => {
const noOpSigner: RequestSigner = {
sign: (request: HttpRequest) =>
sign: (request: HttpRequest, options: { signingDate: Date }) =>
Promise.resolve({
...request,
headers: {
...request.headers,
signed: "true"
signed: "true",
signingDateTime: options.signingDate.getTime()
}
})
} as any;
const noOpNext = jest.fn();
const noOpNext = jest.fn().mockReturnValue({ response: "" });

beforeEach(() => {
(noOpNext as any).mockClear();
});

it("should sign the request and pass it to the next handler", async () => {
expect.assertions(2);
const signingHandler = awsAuthMiddleware({ signer: noOpSigner } as any)(
noOpNext,
{} as any
Expand All @@ -35,4 +37,119 @@ describe("SigningHandler", () => {
expect(calls.length).toBe(1);
expect(calls[0][0].request.headers.signed).toBe("true");
});

it("should add systemClockOffset while signing the request", async () => {
expect.assertions(3);
const systemClockOffset = 1000000;
const now = Date.now();
const signingHandler = awsAuthMiddleware({
signer: noOpSigner,
systemClockOffset
} as any)(noOpNext, {} as any);
await signingHandler({
input: {},
request: new HttpRequest({
headers: {}
})
});

const { calls } = (noOpNext as any).mock;
expect(calls.length).toBe(1);
expect(calls[0][0].request.headers.signed).toBe("true");
// Using greater than to ensure there are no timing issues
expect(calls[0][0].request.headers.signingDateTime).toBeGreaterThan(
now + systemClockOffset - 1
);
});

describe("update systemClockOffset if there is clockSkew", () => {
// Set up clockSkew as abs(newSystemClockOffset - systemClockOffset) > 300000
const clockSkewPresent = [
{ current: 100000, new: 500000 },
{ current: -100000, new: 250000 },
{ current: 200000, new: -150000 },
{ current: -100000, new: -450000 }
];

clockSkewPresent.forEach(systemClockOffsetVal => {
it(`current systemClockOffset: ${systemClockOffsetVal.current}, new systemClockOffset: ${systemClockOffsetVal.new}`, async () => {
expect.assertions(3);
const systemClockOffset = systemClockOffsetVal.current;
const newSystemClockOffset = systemClockOffsetVal.new;
const options = {
signer: noOpSigner,
systemClockOffset
};
const signingHandler = awsAuthMiddleware(options as any)(
noOpNext,
{} as any
);
noOpNext.mockReturnValue({
response: {
headers: {
date: new Date(Date.now() + newSystemClockOffset).toString()
}
}
});

await signingHandler({
input: {},
request: new HttpRequest({
headers: {}
})
});

const { calls } = (noOpNext as any).mock;
expect(calls.length).toBe(1);
expect(calls[0][0].request.headers.signed).toBe("true");
expect(options.systemClockOffset).not.toBe(systemClockOffset);
});
});
});

describe("do not update systemClockOffset if there is no clockSkew", () => {
// Do not set up clockSkew as abs(newSystemClockOffset - systemClockOffset) < 300000
const clockSkewNotPresent = [
{ current: 100000, new: 250000 },
{ current: -100000, new: 50000 },
{ current: 50000, new: -150000 },
{ current: -100000, new: -150000 }
];

clockSkewNotPresent.forEach(systemClockOffsetVal => {
it(`current systemClockOffset: ${systemClockOffsetVal.current}, new systemClockOffset: ${systemClockOffsetVal.new}`, async () => {
expect.assertions(3);
const systemClockOffset = systemClockOffsetVal.current;
const newSystemClockOffset = systemClockOffsetVal.new;
const options = {
signer: noOpSigner,
systemClockOffset
};

const signingHandler = awsAuthMiddleware(options as any)(
noOpNext,
{} as any
);
noOpNext.mockReturnValue({
response: {
headers: {
date: new Date(Date.now() + newSystemClockOffset).toString()
}
}
});

await signingHandler({
input: {},
request: new HttpRequest({
headers: {}
})
});

const { calls } = (noOpNext as any).mock;
expect(calls.length).toBe(1);
expect(calls[0][0].request.headers.signed).toBe("true");
expect(options.systemClockOffset).toBe(systemClockOffset);
});
});
});
});
24 changes: 22 additions & 2 deletions packages/middleware-signing/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ import {
import { AwsAuthResolvedConfig } from "./configurations";
import { HttpRequest } from "@aws-sdk/protocol-http";

const isClockSkewed = (newServerTime: number, systemClockOffset: number) =>
Math.abs(getSkewCorrectedDate(systemClockOffset).getTime() - newServerTime) >=
300000;

const getSkewCorrectedDate = (systemClockOffset: number) =>
new Date(Date.now() + systemClockOffset);

export function awsAuthMiddleware<Input extends object, Output extends object>(
options: AwsAuthResolvedConfig
): FinalizeRequestMiddleware<Input, Output> {
Expand All @@ -20,10 +27,23 @@ export function awsAuthMiddleware<Input extends object, Output extends object>(
args: FinalizeHandlerArguments<Input>
): Promise<FinalizeHandlerOutput<Output>> {
if (!HttpRequest.isInstance(args.request)) return next(args);
return next({
const output = await next({
...args,
request: await options.signer.sign(args.request)
request: await options.signer.sign(args.request, {
signingDate: new Date(Date.now() + options.systemClockOffset)
})
});

const { headers } = output.response as any;
const dateHeader = headers && (headers.date || headers.Date);
if (dateHeader) {
const serverTime = Date.parse(dateHeader);
if (isClockSkewed(serverTime, options.systemClockOffset)) {
options.systemClockOffset = serverTime - Date.now();
}
}

return output;
};
}

Expand Down