tls: revert disable RC4 and cipher lists changes #25511
Conversation
This reverts commit 67d9a56. This commit actually reverts both 67d9a56 and 02a549e (both related to ciphers list changes). It does it in one commit because reverting 02a549e results in an empty commit. These changes are not yet ready to be released, and before they are we want to be able to publish new releases. We're reverting them so that we can submit a new PR that will contain all these changes plus what's necessary to be able to land them properly. Conflicts: src/node.cc
|
This had already been discussed with @jasnell and others during our weekly meeting, but I'd rather give a heads up than bypassing the PR/review process. |
|
Are you hoping to get the updated patch in for the release next week. ie in addition to reverting also get the updated change in ? |
|
@mhdawson I'll try to, but I don't think I'll have time for that. |
|
@joyent/node-collaborators Can I get your feedback on this? This would block releasing v0.10.39 with the OpenSSL upgrade. |
|
I can not figure out the reason why RC4 is back after reading discussions in this PR. Is it for backward compatibility in node-v0.10.x? Or is there another reason to cause an issue with this patch that was applied? |
|
@shigeki The reason why I want to revert these changes before doing a new v0.10.x release is that they are not quite ready yet. Another PR was submitted to fix most of the concerns we had, but it hasn't been approved and thus hasn't landed yet. Instead of rushing to land that PR, I suggest to get a new release with the OpenSSL upgrade out of the door asap, and land it for the next release. Please let me know if that clarifies things. |
|
@shigeki Let me know if my answer clarifies things, I'm waiting for a review in this PR to start the release process for v0.10.39 with the OpenSSL upgrade. However, I don't want to force anyone to approve it, if you still have any concern, please feel free to raise them :) |
|
/cc @mhdawson @joyent/node-tsc @joyent/node-collaborators |
|
Julien, I'm back from the camping trip but not officially back from
|
|
@jasnell No worries, I'm not expecting you to review code when you're on vacation :) Thank you very much for the note though and enjoy your time off 👍 |
|
@misterdjules Thanks. I've got it that we need some treatments before deprecation of RC4. LGTM. |
This reverts commit 67d9a56. This commit actually reverts both 67d9a56 and 02a549e (both related to ciphers list changes). It does it in one commit because reverting 02a549e results in an empty commit. These changes are not yet ready to be released, and before they are we want to be able to publish new releases. We're reverting them so that we can submit a new PR that will contain all these changes plus what's necessary to be able to land them properly. Conflicts: src/node.cc PR: nodejs#25511 PR-URL: nodejs#25511 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
|
Landed in dcb7ef2. |
This reverts commit 67d9a56. This commit actually reverts both 67d9a56 and 02a549e (both related to ciphers list changes). It does it in one commit because reverting 02a549e results in an empty commit. These changes are not yet ready to be released, and before they are we want to be able to publish new releases. We're reverting them so that we can submit a new PR that will contain all these changes plus what's necessary to be able to land them properly. Conflicts: src/node.cc PR: nodejs#25511 PR-URL: nodejs#25511 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
This reverts commit 67d9a56. This commit actually reverts both 67d9a56 and 02a549e (both related to ciphers list changes). It does it in one commit because reverting 02a549e results in an empty commit. These changes are not yet ready to be released, and before they are we want to be able to publish new releases. We're reverting them so that we can submit a new PR that will contain all these changes plus what's necessary to be able to land them properly. Conflicts: src/node.cc PR: nodejs#25511 PR-URL: nodejs#25511 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
This reverts commit 67d9a56.
This commit actually reverts both
67d9a56 and
02a549e (both related to ciphers list
changes). It does it in one commit because reverting
02a549e results in an empty commit.
These changes are not yet ready to be released, and before they are we
want to be able to publish new releases. We're reverting them so that we
can submit a new PR that will contain all these changes plus what's
necessary to be able to land them properly.
Conflicts:
src/node.cc
/cc @joyent/node-tsc