Skip to content

[Converge] SSLv2/3 related commits #2514

Description

@rvagg

Continuing from nodejs/node-convergence-archive#20, marking against 4.0.0 milestone. Summary of that discussion is that SSLv2 and SSLv3 will not be reintroduced to v4, but there are some outstanding items raised by @misterdjules:

If I remember correctly, nodejs/node-v0.x-archive@69080f5 is a bug fix that is independent of SSLv2/SSLv3 support, and I would think we'd want to keep it.

nodejs/node-v0.x-archive@8d045a3 (and subsequent changes to the tests that it adds) has proven to be very valuable to catch issues when upgrading OpenSSL. It's actually what helped us find out about the problen that nodejs/node-v0.x-archive@69080f5 fixes, and others. So I would suggest to keep that too.

nodejs/node-v0.x-archive@d230fa9 fixes a minor but still real issue with documentation. I would also keep that.

@jasnell can you look back through that thread to see if there are any other commits missing from this list that should be included?

Activity

  1. added this to the 4.0.0 milestone on Aug 24, 2015
  2. added
    tlsIssues and PRs related to the tls subsystem.
    docIssues and PRs related to Node.js documentation.
    testIssues and PRs related to Node.js core tests and test infrastructure.
    and removed
    docIssues and PRs related to Node.js documentation.
    testIssues and PRs related to Node.js core tests and test infrastructure.
    on Aug 24, 2015
  3. Fishrock123 commented on Aug 24, 2015

    @Fishrock123
    Contributor

    @indutny / @shigeki could you look at the first two commits there?

  4. indutny commented on Aug 24, 2015

    @indutny
    Member

    I'm fine with these changes.

  5. shigeki commented on Aug 25, 2015

    @shigeki
    Contributor

    nodejs/node-v0.x-archive@69080f5 is the fix only for node-v0.10 and is no longer needed for 0.12 and 4.0. nodejs/node-v0.x-archive@8d045a3 has large test cases, give me sometime for check them.

  6. rvagg commented on Aug 25, 2015

    @rvagg
    MemberAuthor

    @shigeki I'm assigning this to you, can you make sure those land in master by the end of this week please and then close this issue

  7. shigeki commented on Aug 25, 2015

    @shigeki
    Contributor

    @rvagg Okay, no problem.

    Checking all test cases in nodejs/node-v0.x-archive@8d045a3, I think this is not necessary to be merged into 4.0 because 4.0 has no --enable-ssl2/ssl3 command line options and configurations of SSL options/methods for SSLv2/v3 are explicitly disabled so that the remaining test cases are only for TLSv1_methods.

    The doc fix of nodejs/node-v0.x-archive@d230fa9 is for the description that was introduced only in joyent/node as nodejs/node-v0.x-archive@1349b68 to allow SSLv2/v3. This description is not included in 4.0 so it's no longer is needed neither.

  8. Fishrock123 commented on Aug 25, 2015

    @Fishrock123
    Contributor

    so that the remaining test cases are only for TLSv1_methods.

    Are we sure those aren't useful? @misterdjules said they were in finding regressions, unless we already test all of that stuff?

  9. shigeki commented on Aug 26, 2015

    @shigeki
    Contributor

    We already have test/parallel/test-tls-no-sslv23.js and test/parallel/test-tls-no-sslv3.js that can check regressions. They are not included in joyent/node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

tlsIssues and PRs related to the tls subsystem.

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions