Skip to content

Commit 182e4e0

Browse files
author
Antti Risteli
committed
Replace createSecurePair with TLSSocket + TCP server
This is stupid. Not how to properly replace createSecurePair. SecurePair fires ‘secure’ event synchronously. Using a couple of sockets instead of it means that the flow becomes more asynchronous. Therefore socket ‘end’ can not be used directly to control the Connection state, the data first needs to pass through the TCP server and TLSSocket.
1 parent e2152d4 commit 182e4e0

File tree

3 files changed

+86
-37
lines changed

3 files changed

+86
-37
lines changed

src/connection.js

+1-8
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,6 @@ class Connection extends EventEmitter {
357357

358358
this.reset = this.reset.bind(this);
359359
this.socketClose = this.socketClose.bind(this);
360-
this.socketEnd = this.socketEnd.bind(this);
361360
this.socketConnect = this.socketConnect.bind(this);
362361
this.socketError = this.socketError.bind(this);
363362
this.requestTimeout = this.requestTimeout.bind(this);
@@ -691,12 +690,11 @@ class Connection extends EventEmitter {
691690

692691
this.socket = socket;
693692
this.socket.on('error', this.socketError);
694-
this.socket.on('close', this.socketClose);
695-
this.socket.on('end', this.socketEnd);
696693
this.messageIo = new MessageIO(this.socket, this.config.options.packetSize, this.debug);
697694
this.messageIo.on('data', (data) => { this.dispatchEvent('data', data); });
698695
this.messageIo.on('message', () => { this.dispatchEvent('message'); });
699696
this.messageIo.on('secure', this.emit.bind(this, 'secure'));
697+
this.messageIo.on('close', this.socketClose);
700698

701699
this.socketConnect();
702700
});
@@ -815,11 +813,6 @@ class Connection extends EventEmitter {
815813
return this.dispatchEvent('socketConnect');
816814
}
817815

818-
socketEnd() {
819-
this.debug.log('socket ended');
820-
return this.transitionTo(this.STATE.FINAL);
821-
}
822-
823816
socketClose() {
824817
this.debug.log('connection to ' + this.config.server + ':' + this.config.options.port + ' closed');
825818
if (this.state === this.STATE.REROUTING) {

src/message-io.js

+83-28
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const tls = require('tls');
22
const crypto = require('crypto');
3+
const net = require('net');
34
const EventEmitter = require('events').EventEmitter;
45
const Transform = require('readable-stream').Transform;
56

@@ -50,6 +51,51 @@ class ReadablePacketStream extends Transform {
5051
}
5152
}
5253

54+
class TLSHandler extends EventEmitter {
55+
constructor(secureContext) {
56+
super();
57+
const self = this;
58+
self.server = net.createServer();
59+
60+
self.server.listen(0, '127.0.0.1', () => {
61+
self.cleartext = tls.connect({
62+
host: '127.0.0.1',
63+
port: self.server.address().port,
64+
secureContext: secureContext,
65+
rejectUnauthorized: false
66+
});
67+
self.cleartext.on('secureConnect', () => {
68+
self.emit('secure');
69+
self.cleartext.write('');
70+
});
71+
});
72+
73+
const encryptedOnQueue = [];
74+
self.encrypted = {
75+
on: (event, cb) => {
76+
encryptedOnQueue.push([event, cb]);
77+
}
78+
};
79+
80+
self.server.on('connection', (socket) => {
81+
self.encrypted = socket;
82+
encryptedOnQueue.forEach(([event, cb]) => {
83+
self.encrypted.on(event, cb);
84+
});
85+
self.server.close();
86+
});
87+
}
88+
89+
destroy() {
90+
if (this.encrypted && this.encrypted.destroy) {
91+
this.encrypted.destroy();
92+
}
93+
if (this.cleartext && this.cleartext.destroy) {
94+
this.cleartext.destroy();
95+
}
96+
}
97+
}
98+
5399
module.exports = class MessageIO extends EventEmitter {
54100
constructor(socket, _packetSize, debug) {
55101
super();
@@ -70,6 +116,10 @@ module.exports = class MessageIO extends EventEmitter {
70116

71117
this.socket.pipe(this.packetStream);
72118
this.packetDataSize = this._packetSize - packetHeaderLength;
119+
120+
this.socket.on('close', () => {
121+
this.emit('close');
122+
});
73123
}
74124

75125
packetSize(packetSize) {
@@ -84,54 +134,59 @@ module.exports = class MessageIO extends EventEmitter {
84134
startTls(credentialsDetails, hostname, trustServerCertificate) {
85135
const credentials = tls.createSecureContext ? tls.createSecureContext(credentialsDetails) : crypto.createCredentials(credentialsDetails);
86136

87-
this.securePair = tls.createSecurePair(credentials);
137+
this.tlsHandler = new TLSHandler(credentials);
138+
88139
this.tlsNegotiationComplete = false;
89140

90-
this.securePair.on('secure', () => {
91-
const cipher = this.securePair.cleartext.getCipher();
141+
this.tlsHandler.on('secure', () => {
142+
const cipher = this.tlsHandler.cleartext.getCipher();
92143

93144
if (!trustServerCertificate) {
94-
let verifyError = this.securePair.ssl.verifyError();
95-
96-
// Verify that server's identity matches it's certificate's names
97-
if (!verifyError) {
98-
verifyError = tls.checkServerIdentity(hostname, this.securePair.cleartext.getPeerCertificate());
99-
}
100-
101-
if (verifyError) {
102-
this.securePair.destroy();
103-
this.socket.destroy(verifyError);
145+
if (!this.tlsHandler.cleartext.authorized) {
146+
this.tlsHandler.destroy();
147+
this.socket.destroy(this.tlsHandler.cleartext.authorizationError);
104148
return;
105149
}
106150
}
107151

108152
this.debug.log('TLS negotiated (' + cipher.name + ', ' + cipher.version + ')');
109-
this.emit('secure', this.securePair.cleartext);
153+
this.emit('secure', this.tlsHandler.cleartext);
110154
this.encryptAllFutureTraffic();
111155
});
112156

113-
this.securePair.encrypted.on('data', (data) => {
157+
this.tlsHandler.encrypted.on('data', (data) => {
114158
this.sendMessage(TYPE.PRELOGIN, data);
115159
});
116-
117-
// On Node >= 0.12, the encrypted stream automatically starts spewing out
118-
// data once we attach a `data` listener. But on Node <= 0.10.x, this is not
119-
// the case. We need to kick the cleartext stream once to get the
120-
// encrypted end of the secure pair to emit the TLS handshake data.
121-
this.securePair.cleartext.write('');
122160
}
123161

124162
encryptAllFutureTraffic() {
125163
this.socket.unpipe(this.packetStream);
126-
this.securePair.encrypted.removeAllListeners('data');
127-
this.socket.pipe(this.securePair.encrypted);
128-
this.securePair.encrypted.pipe(this.socket);
129-
this.securePair.cleartext.pipe(this.packetStream);
164+
this.tlsHandler.encrypted.removeAllListeners('data');
165+
this.socket.pipe(this.tlsHandler.encrypted);
166+
this.socket.removeAllListeners('close');
167+
this.socket.on('close', () => {
168+
this.tlsHandler.encrypted.end();
169+
});
170+
this.socket.on('error', () => {
171+
this.tlsHandler.encrypted.end();
172+
});
173+
this.tlsHandler.cleartext.on('close', () => {
174+
this.emit('close');
175+
});
176+
this.tlsHandler.encrypted.pipe(this.socket);
177+
this.tlsHandler.cleartext.pipe(this.packetStream);
130178
this.tlsNegotiationComplete = true;
179+
if (!this.socket.destroyed) {
180+
// the old SecurePair worked synchronously and fired the
181+
// 'secure' event before the packet was handled by
182+
// RedablePacketStream. this is not the case anymore so
183+
// emit 'message' manually to fire SENT_TLSSSLNEGOTIATION.message again
184+
this.emit('message');
185+
}
131186
}
132187

133188
tlsHandshakeData(data) {
134-
this.securePair.encrypted.write(data);
189+
this.tlsHandler.encrypted.write(data);
135190
}
136191

137192
// TODO listen for 'drain' event when socket.write returns false.
@@ -168,8 +223,8 @@ module.exports = class MessageIO extends EventEmitter {
168223

169224
sendPacket(packet) {
170225
this.logPacket('Sent', packet);
171-
if (this.securePair && this.tlsNegotiationComplete) {
172-
this.securePair.cleartext.write(packet.buffer);
226+
if (this.tlsHandler && this.tlsNegotiationComplete) {
227+
this.tlsHandler.cleartext.write(packet.buffer);
173228
} else {
174229
this.socket.write(packet.buffer);
175230
}

test/integration/connection-retry-test.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ exports['connection retry tests'] = {
4545
connection.on('end', (info) => {
4646
test.done();
4747
});
48-
},
48+
}
49+
,
4950

5051
'no retries on non-transient errors': function(test) {
5152
const config = getConfig();

0 commit comments

Comments
 (0)