Skip to content

Conversation

@sam-github
Copy link
Contributor

@sam-githubsam-github commented Jan 17, 2017

Direct use of tls.TLSSocket to start a TLS session over an existing TCP connection was documented.

However, to use this connection securely it is necessary to validate and
authenticate the peer's certificate, and the documented events and
properties are implemented only for TLSSockets returned by
tls.connect(). In order to create secure connections, additional
undocumented APIs must be used, and these APIs are being called right
now by npm modules.

Fix: #10555
Fix: #11467

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
Affected core subsystem(s)

tls,test,doc

@sam-githubsam-github added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem. labels Jan 17, 2017
addaleax
addaleax previously approved these changes Jan 17, 2017
@sam-github
Copy link
ContributorAuthor

PTAL @nodejs/crypto

@sam-github
Copy link
ContributorAuthor

PTAL @bnoordhuis@indutny@shigeki

@sam-github
Copy link
ContributorAuthor

@nodejs/crypto hasn't been reviewed yet, so I pushed docs for the APIs called by https://github.com/mattcg/starttls and other users, I'm sure. That last commit fixes #10555

Direct use of tls.TLSSocket to start a TLS session over an existing TCP connection was documented. However, to use this connection securely it is necessary to validate and authenticate the peer's certificate, and the documented events and properties are implemented only for TLSSockets returned by tls.connect(). In order to create secure connections, additional undocumented APIs must be used, and these APIs are being called right now by npm modules. Fix: nodejs#10555Fix: nodejs#11467
@jasnell
Copy link
Member

Updates on this one?

@jasnelljasnell added the stalled Issues and PRs that are stalled. label Mar 24, 2017
@fhinkel
Copy link
Member

There hasn't been any activity here. I'm closing this. Feel free to reopen if closed in error.

@fhinkelfhinkel closed this May 26, 2017
jasnell added a commit to jasnell/node that referenced this pull request Nov 1, 2018
deprecate the legacy undocumented `.ssl` alias for the `TLSSocket._handle` and document alternatives. Document how to properly use the `TLSSocket` constructor directly. Updated take on nodejs#10846Fixes: nodejs#10555
@sam-githubsam-github deleted the tls-conn-tests branch November 27, 2018 22:09
jasnell added a commit to jasnell/node that referenced this pull request Dec 31, 2020
Fixes: nodejs#10555 Refs: nodejs#10846 The `new tls.TLSSocket()` constructor does not set up all of the necessary lifecycle management or event handlers necessary for proper use. The `tls.connect()` method really should be the way that all `tls.TLSSocket()` instances are created. This commit begins the eventual phasing out of the `new tls.TLSSocket()` constructor with a doc-only deprecation. Signed-off-by: James M Snell <[email protected]>
jasnell added a commit to jasnell/node that referenced this pull request Apr 28, 2021
targos pushed a commit that referenced this pull request May 2, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
targos pushed a commit that referenced this pull request May 3, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
targos pushed a commit that referenced this pull request May 30, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
targos pushed a commit that referenced this pull request Jun 11, 2021
Fixes: #10555 Signed-off-by: James M Snell <[email protected]> Refs: #10846 PR-URL: #38447 Reviewed-By: Alba Mendez <[email protected]> Reviewed-By: Adrian Estrada <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.stalledIssues and PRs that are stalled.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@sam-github@shigeki@jasnell@fhinkel@addaleax@mscdex@nodejs-github-bot