Skip to content

Commit

Permalink
tls: renegotiate should take care of its own state
Browse files Browse the repository at this point in the history
In the initial version of this test there were two zero-length writes to
force tls state to cycle. The second is not necessary, at least not now,
but the first was. The renegotiate() API should ensure that packet
exchange takes place, not its users, so move the zero-length write into
tls.

See: #14239
See: b1909d3a70f9

PR-URL: #25997
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
sam-github authored and addaleax committed Feb 13, 2019
1 parent 5198297 commit 8495a78
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
3 changes: 3 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,9 @@ TLSSocket.prototype.renegotiate = function(options, callback) {
this._requestCert = requestCert;
this._rejectUnauthorized = rejectUnauthorized;
}
// Ensure that we'll cycle through internal openssl's state
this.write('');

if (!this._handle.renegotiate()) {
if (callback) {
process.nextTick(callback, new ERR_TLS_RENEGOTIATE());
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-tls-disable-renegotiation.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ server.listen(0, common.mustCall(() => {
port
};
const client = tls.connect(options, common.mustCall(() => {
client.write('');
// Negotiation is still permitted for this first
// attempt. This should succeed.
let ok = client.renegotiate(options, common.mustCall((err) => {
Expand All @@ -56,7 +55,6 @@ server.listen(0, common.mustCall(() => {
// data event on the server. After that data
// is received, disableRenegotiation is called.
client.write('data', common.mustCall(() => {
client.write('');
// This second renegotiation attempt should fail
// and the callback should never be invoked. The
// server will simply drop the connection after
Expand Down

0 comments on commit 8495a78

Please sign in to comment.