Skip to content

Commit b175244

Browse files
committed
fix mTLS connection support, add tls tests (winstonjs#147)
1 parent 4b74b36 commit b175244

File tree

5 files changed

+256
-24
lines changed

5 files changed

+256
-24
lines changed

CHANGELOG.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
# CHANGELOG
22

3+
## Latest
4+
5+
- #[147], (@zenonhun) fix TLS connection support
6+
37
## v2.4.0 / 2020-01-01
48

59
- (@DABH) Node v12 support, fix node-unix-dgram issues, update dependencies
610
- #[115], (@pepakriz) TLS connection support
7-
- #[123], (@JeffTomlinson, @elliotttf) Handle oversize messages sent over UDP transports
11+
- #[123], (@JeffTomlinson, @elliotttf) Handle oversize messages sent over UDP transports
812
- #[116], (@pepakriz) Make socket options configurable
913
- #[122], (@cjbarth) Correct improper opening and closing of sockets
1014

@@ -20,4 +24,3 @@
2024
- #[108], (@vrza) Make winston 3 a peer dependency
2125
- #[102], (@stieg) Require winston >= 3 and add corresopnding note in readme
2226
- #[105], (@mohd-akram) Update dependencies for latest Node compatibility
23-

lib/winston-syslog.js

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ class Syslog extends Transport {
279279
this.socket.close();
280280
}
281281
}
282-
283282
this.emit('closed', this.socket);
284283
} else {
285284
attempt++;
@@ -339,14 +338,6 @@ class Syslog extends Transport {
339338
return this.connectDgram(callback);
340339
}
341340

342-
this.socket = /^tls[4|6]?$/.test(this.protocol)
343-
? new secNet.TLSSocket()
344-
: new net.Socket();
345-
this.socket.setKeepAlive(true);
346-
this.socket.setNoDelay();
347-
348-
this.setupEvents();
349-
350341
const connectConfig = Object.assign({}, this.protocolOptions, {
351342
host: this.host,
352343
port: this.port
@@ -356,7 +347,13 @@ class Syslog extends Transport {
356347
connectConfig.family = this.protocolFamily;
357348
}
358349

359-
this.socket.connect(connectConfig);
350+
this.socket = /^tls[4|6]?$/.test(this.protocol)
351+
? secNet.connect(connectConfig)
352+
: net.connect(connectConfig);
353+
this.socket.setKeepAlive(true);
354+
this.socket.setNoDelay();
355+
356+
this.setupEvents();
360357

361358
//
362359
// Indicate to the callee that the socket is not ready. This
@@ -396,7 +393,8 @@ class Syslog extends Transport {
396393
this.retries = 0;
397394
this.connected = true;
398395
})
399-
.on('error', function () {
396+
.on('error', (error) => {
397+
this.emit('error', error);
400398
//
401399
// TODO: Pass this error back up
402400
//
@@ -409,10 +407,12 @@ class Syslog extends Transport {
409407
const interval = Math.pow(2, this.retries);
410408
this.connected = false;
411409

412-
setTimeout(() => {
413-
this.retries++;
414-
this.socket.connect(this.port, this.host);
415-
}, interval * 1000);
410+
if (!this.socket.destroyed) {
411+
setTimeout(() => {
412+
this.retries++;
413+
this.socket.connect(this.port, this.host);
414+
}, interval * 1000);
415+
}
416416
})
417417
.on('timeout', () => {
418418
if (this.socket.destroy) {

package-lock.json

Lines changed: 21 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,14 @@
3232
"glossy": "^0.1.7"
3333
},
3434
"optionalDependencies": {
35-
"unix-dgram": "2.0.3"
35+
"unix-dgram": "^2.0.4"
3636
},
3737
"peerDependencies": {
3838
"winston": "^3.0.0"
3939
},
4040
"devDependencies": {
4141
"eslint-config-populist": "^4.2.0",
42+
"selfsigned": "^1.10.8",
4243
"sinon": "^8.0.2",
4344
"vows": "^0.8.3",
4445
"winston": "^3.0.0"

test/tls-test.js

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
const vows = require('vows');
2+
const assert = require('assert');
3+
const selfsigned = require('selfsigned');
4+
const tls = require('tls');
5+
const winston = require('winston');
6+
require('../lib/winston-syslog').Syslog;
7+
8+
// ----- Constants
9+
const HOST = 'localhost';
10+
const PORT = 10514;
11+
const PROMISE_TIMEOUT = 1000;
12+
13+
// ----- Helpers
14+
function wrapToPromise(target, event) {
15+
return new Promise((resolve, reject) => {
16+
const timeout = setTimeout(() => {
17+
reject('Timeout for event promise');
18+
}, PROMISE_TIMEOUT);
19+
target.on(event, (...args) => {
20+
clearTimeout(timeout);
21+
resolve(...args);
22+
});
23+
});
24+
}
25+
26+
function nodeMajorVersion() {
27+
return Number.parseInt(process.version.match(/^v(\d+\.\d+)/)[1], 10);
28+
}
29+
30+
// ----- Certificate handling
31+
function generateCertificate() {
32+
// Generate server and client certificates
33+
const attributes = [{ name: 'commonName', value: 'localhost' }];
34+
const x509 = selfsigned.generate(attributes, {
35+
days: 1,
36+
clientCertificate: true,
37+
extensions: [
38+
{
39+
name: 'keyUsage',
40+
keyCertSign: true,
41+
digitalSignature: true,
42+
nonRepudiation: true,
43+
keyEncipherment: true,
44+
dataEncipherment: true
45+
},
46+
{
47+
name: 'subjectAltName',
48+
altNames: [
49+
{
50+
type: 2, // DNS
51+
value: 'localhost'
52+
}
53+
]
54+
}
55+
]
56+
});
57+
return x509;
58+
}
59+
60+
// ----- TLS configs
61+
const x509 = generateCertificate();
62+
const validServerTLS = {
63+
key: x509.private,
64+
cert: x509.cert,
65+
ca: [x509.cert],
66+
rejectUnauthorized: true,
67+
requestCert: true
68+
};
69+
70+
const validClientTLS = {
71+
key: x509.clientprivate,
72+
cert: x509.clientcert,
73+
ca: [x509.cert],
74+
rejectUnauthorized: true
75+
};
76+
77+
const untrustedClientTLS = {
78+
...validClientTLS,
79+
ca: null
80+
};
81+
82+
const missingClientTLS = {
83+
ca: null,
84+
key: null,
85+
cert: null,
86+
rejectUnauthorized: false
87+
};
88+
89+
// ----- TLS Server
90+
const serverEvents = {
91+
data: 'data',
92+
tlsClientError: 'tlsClientError',
93+
error: 'error',
94+
listening: 'listening',
95+
secureConnection: 'secureConnection'
96+
};
97+
98+
async function startServer({ host = HOST, port = PORT, tlsOpts } = {}) {
99+
const server = tls.createServer({ ...tlsOpts });
100+
let clients = [];
101+
server.on('secureConnection', (client) => {
102+
clients.push(client);
103+
client.on('close', () => {
104+
clients = clients.filter((c) => c !== client);
105+
});
106+
client.on('data', (data) => {
107+
server.emit(serverEvents.data, data.toString());
108+
});
109+
});
110+
server.forceClose = () => {
111+
clients.forEach((client) => client.destroy());
112+
server.close();
113+
};
114+
server.listen({ host, port });
115+
await wrapToPromise(server, serverEvents.listening);
116+
return server;
117+
}
118+
119+
// ----- Init Logger
120+
function initLogger({ host = HOST, port = PORT, tlsOpts } = {}) {
121+
const syslogOptions = {
122+
host,
123+
port,
124+
protocol: 'tls4',
125+
protocolOptions: { ...tlsOpts }
126+
};
127+
const logger = winston.createLogger({
128+
transports: [new winston.transports.Syslog(syslogOptions)]
129+
});
130+
return logger;
131+
}
132+
133+
// ----- Test Cases
134+
const TEST_MESSAGE = 'Test Message';
135+
const SYSLOG_FORMAT = `"message":"${TEST_MESSAGE}"`;
136+
137+
let serverInstance;
138+
139+
vows
140+
.describe('tls-connect')
141+
.addBatch({
142+
'Trying to connect to a TLS server with mutual TLS': {
143+
'topic': function () {
144+
startServer({ tlsOpts: validServerTLS }).then((server) => {
145+
serverInstance = server;
146+
const promise = wrapToPromise(server, serverEvents.data);
147+
initLogger({ tlsOpts: validClientTLS }).info(TEST_MESSAGE);
148+
promise.then(msg => this.callback(null, msg));
149+
});
150+
},
151+
'TLS server should receive log message': function (msg) {
152+
assert.include(msg, SYSLOG_FORMAT);
153+
},
154+
'teardown': function () {
155+
serverInstance.forceClose();
156+
}
157+
}
158+
})
159+
.addBatch({
160+
'Trying to connect to a TLS server with untrusted certificate': {
161+
'topic': function () {
162+
startServer({ tlsOpts: validServerTLS }).then((server) => {
163+
serverInstance = server;
164+
const logger = initLogger({ tlsOpts: untrustedClientTLS });
165+
logger.on('error', (loggerError) => {
166+
this.callback(null, loggerError);
167+
});
168+
logger.info(TEST_MESSAGE);
169+
});
170+
},
171+
'Client should refuse connection': function (e, loggerError) {
172+
assert.strictEqual(loggerError.code, 'DEPTH_ZERO_SELF_SIGNED_CERT');
173+
assert.include(loggerError.message, 'self signed certificate');
174+
},
175+
'teardown': function () {
176+
serverInstance.forceClose();
177+
}
178+
}
179+
})
180+
.addBatch({
181+
'Trying to connect to a TLS server without client certificate': {
182+
'topic': function () {
183+
startServer({ tlsOpts: validServerTLS }).then((server) => {
184+
serverInstance = server;
185+
const promise = wrapToPromise(server, serverEvents.tlsClientError);
186+
const logger = initLogger({ tlsOpts: missingClientTLS });
187+
logger.on('error', (loggerError) => {
188+
promise
189+
.then((serverError) => this.callback(null, loggerError, serverError))
190+
.catch((error) => this.callback(error, loggerError, null));
191+
});
192+
logger.info(TEST_MESSAGE);
193+
});
194+
},
195+
'Server should refuse connection': function (e, loggerError, serverError) {
196+
// Client and Server error type changes between Node versions
197+
if (nodeMajorVersion() >= 12) {
198+
assert.strictEqual(loggerError.code, 'ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED');
199+
assert.include(loggerError.message, 'alert number 116');
200+
assert.strictEqual(serverError.code, 'ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE');
201+
assert.include(serverError.message, 'peer did not return a certificate');
202+
} else {
203+
assert.strictEqual(loggerError.code, 'EPROTO');
204+
assert.include(loggerError.message, 'alert number 40');
205+
assert.include(serverError.message, 'peer did not return a certificate');
206+
}
207+
},
208+
'teardown': function () {
209+
serverInstance.forceClose();
210+
}
211+
}
212+
})
213+
.export(module);

0 commit comments

Comments
 (0)