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

https: client support for TLS keylog events #30053

Closed
wants to merge 1 commit into from

Conversation

sam-github
Copy link
Contributor

The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible. Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Oct 21, 2019
lib/_http_agent.js Outdated Show resolved Hide resolved
doc/api/https.md Outdated Show resolved Hide resolved
lib/_http_agent.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

test/parallel/test-https-agent-keylog.js Outdated Show resolved Hide resolved
test/parallel/test-https-agent-keylog.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

lib/_http_agent.js Show resolved Hide resolved
lib/_http_agent.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible.  Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.
@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 20, 2019
@Trott
Copy link
Member

Trott commented Nov 20, 2019

Landed in b0cf62b

@Trott Trott closed this Nov 20, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 20, 2019
The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible.  Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.

PR-URL: nodejs#30053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@sam-github sam-github deleted the agent-keylog branch November 20, 2019 16:01
MylesBorins pushed a commit that referenced this pull request Nov 21, 2019
The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible.  Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.

PR-URL: #30053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Nov 21, 2019
targos pushed a commit that referenced this pull request Jan 13, 2020
The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible.  Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.

PR-URL: #30053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mildsunrise
Copy link
Member

Thanks for this change! 💙
@kolontsov With this and #30055, I think we can fully deprecate sslkeylog now

BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
The keylog event is implemented on TLS sockets, but client HTTPS uses
TLS sockets managed by an agent, so accessing the underlying socket
before the TLS handshake completed was not possible.  Note that server
HTTPS already supports the keylog event because it inherits from the TLS
server.

PR-URL: #30053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants