feat: enable clockSkew correction by default (#459)

Author: trivikr

packages/middleware-signing/src/configurations.ts

@@ -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>;
@@ -32,6 +37,7 @@ export interface AwsAuthResolvedConfig {
   credentials: Provider<Credentials>;
   signer: RequestSigner;
   signingEscapePath: boolean;
+  systemClockOffset: number;
 }
 export function resolveAwsAuthConfig<T>(
   input: T & AwsAuthInputConfig & PreviouslyResolved
@@ -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({

packages/middleware-signing/src/middleware.spec.ts

@@ -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
@@ -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);
+      });
+    });
+  });
 });

packages/middleware-signing/src/middleware.ts

@@ -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> {
@@ -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;
     };
 }