Skip to content

debugger: validate sec-websocket-accept response header - #39357

Merged
Trott merged 2 commits into
nodejs:masterfrom
Trott:validate-websocket
Jul 18, 2021
Merged

Trott merged 2 commits into
nodejs:masterfrom
Trott:validate-websocket

Conversation

@Trott

@Trott Trott commented Jul 11, 2021

Copy link
Copy Markdown
Member

First commit by @copperwall:

debugger: validate sec-websocket-accept response header

This addresses a TODO to validate that the sec-websocket-accept header
in the websocket handshake response is valid. To do this we need to
append the Websocket GUID to the original key sent in sec-websocket-key,
sha1 hash it, and then compare the base64 encoding with the value sent
in the sec-websocket-accept response header.

If they don't match, an error is thrown.

Second commit:

test: add test for websocket secret verification in debugger

Refs: nodejs/node-inspect#93

@nodejs-github-bot nodejs-github-bot added debugger Issues and PRs related to the Node.js command-line debugger. needs-ci PRs that need a full CI run. labels Jul 11, 2021
@Trott
Trott force-pushed the validate-websocket branch from 5e5edc5 to dfa5c74 Compare July 11, 2021 22:08
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 11, 2021
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 11, 2021
@nodejs-github-bot

This comment has been minimized.

@Trott
Trott force-pushed the validate-websocket branch from dfa5c74 to 9fe423a Compare July 11, 2021 23:14
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Is "WebSocket" the correct capitalization of the word? I could be wrong, but thought I'd ask since you're using "Websocket" throughout this PR.

@Trott
Trott force-pushed the validate-websocket branch from 9fe423a to 819846c Compare July 13, 2021 03:55
@Trott

Trott commented Jul 13, 2021 •

Copy link
Copy Markdown
Member Author

Is "WebSocket" the correct capitalization of the word? I could be wrong, but thought I'd ask since you're using "Websocket" throughout this PR.

Fixed it in the commit messages and in the code added here.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 13, 2021