Skip to content
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

systemClockOffset not applied to events sent over websocket connection #6217

Closed
3 tasks done
esauerbo opened this issue Jun 24, 2024 · 5 comments
Closed
3 tasks done
Assignees
Labels
bug This issue is a bug. p1 This is a high priority issue

Comments

@esauerbo
Copy link

esauerbo commented Jun 24, 2024

Checkboxes for prior research

Describe the bug

Manually setting the systemClockOffset allows me to successfully establish a websocket connection, but an InvalidSignatureException is returned once events are sent over the connection.

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Chrome 126

Reproduction Steps

Reproduction branch: https://github.com/esauerbo/rekognition-sdk-repro/tree/clockSkew

  1. Checkout to the clockSkew branch
  2. npm install
  3. Get credentials and add to .env
  4. npm run dev
  5. Set system clock to one hour ahead (without changing time zone)
  6. Check network tab for start-face-liveness-session network request. Observe that connection is established successfully but after client sends event, server sends InvalidSignatureException
Screenshot 2024-06-24 at 11 48 36 AM

Observed Behavior

Received error for invalid signature exception

Expected Behavior

systemClockOffset should be applied to events sent over websocket connection.

Possible Solution

This may be relevant, offset doesn't seem to be getting applied.

Additional Information/Context

No response

@esauerbo esauerbo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 24, 2024
@kuhe kuhe self-assigned this Jun 25, 2024
@kuhe kuhe added p1 This is a high priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jun 25, 2024
@kuhe
Copy link
Contributor

kuhe commented Jun 25, 2024

related #5015

@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Jun 26, 2024
@kuhe
Copy link
Contributor

kuhe commented Jun 26, 2024

Due to the inability to read the date header in the upgrade response (#6221),

to make use of the fix in #6227, the systemClockOffset must be supplied to the RekognitionStreaming client at initialization.

Example:

import { Rekognition } from '@aws-sdk/client-rekognition';
import { RekognitionStreaming } from '@aws-sdk/client-rekognitionstreaming';

const rekognition = new Rekognition({ ... });

// 1. make a request first, so that the client automatically corrects
// its config.systemClockOffset value if needed.
await rekognition.createFaceLivenessSession({ ... });

// 2. pass that value to the streaming client.
const rekognitionStreaming = new RekognitionStreaming({ 
  systemClockOffset: rekognition.config.systemClockOffset
});

@thaddmt
Copy link
Contributor

thaddmt commented Jun 26, 2024

@kuhe thanks for the recommendation but I believe that solution will not work for most rekognition consumers as it is recommended that the createFaceLivenessSession call be made in the developers backend where as the streaming client should be called from the customer's device.

I'm guessing in the example you posted above after calling the createFaceLivenessSession API the systemClockOffset value is set if a time difference is detected?

@kuhe
Copy link
Contributor

kuhe commented Jun 27, 2024

This was released in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.605.0.

solution will not work for most rekognition consumers>

They need to know their system clock offset. We've established that the upgrade response headers are not an option.

I'm guessing in the example you posted above after calling the createFaceLivenessSession API the systemClockOffset value is set if a time difference is detected?

Yes.

@kuhe kuhe closed this as completed Jun 27, 2024
@kuhe kuhe removed the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Jun 27, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. p1 This is a high priority issue
Projects
None yet
Development

No branches or pull requests

3 participants