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

Fix rejectUnauthorized: undefined in tls.connect, requestCert in tls.createServer #5917

Closed
2 tasks
indutny opened this issue Mar 26, 2016 · 5 comments
Closed
2 tasks
Labels
tls Issues and PRs related to the tls subsystem.

Comments

@indutny
Copy link
Member

indutny commented Mar 26, 2016

  • Version: master
  • Platform: all
  • Subsystem: tls

There are two usability issues in tls module:

  • tls.connect treats rejectUnauthorized !== true as a false value, even more rejectUnauthorized: undefined is treated as a false. Clearly it should be completely reversed, only when rejectUnauthorized is false - it should be disabled. (This is semver-major)
  • tls.createServer should change default value of requestCert to true. (Note: we should verify that it still works 😉 ) (semver-major as well)
@indutny indutny added tls Issues and PRs related to the tls subsystem. good first issue Issues that are suitable for first-time contributors. labels Mar 26, 2016
@bnoordhuis
Copy link
Member

tls.createServer should change default value of requestCert to true.

Did you mean rejectUnauthorized? requestCert is for making the server request client certificates.

@ghaiklor
Copy link
Contributor

@indutny can I remove _tls_legacy.js as well? Since is semver-major change.

@jhamhader
Copy link
Contributor

@ghaiklor @indutny see #5924

@ghaiklor
Copy link
Contributor

@jhamhader see #5923 . Your PR has conflicts, better to split removing _tls_legacy and usability fixes to different PR.

UPD: I see, conflicts resolved.

@Fishrock123 Fishrock123 removed the good first issue Issues that are suitable for first-time contributors. label May 5, 2016
@addaleax
Copy link
Member

I think this is fixed now that #5923 has landed, feel free to re-open if I’m mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants