Skip to content

Commit 116190d

Browse files
committed
winstonjs#147 fix mTLS connection support, add tls tests
1 parent 4b74b36 commit 116190d

File tree

5 files changed

+233
-24
lines changed

5 files changed

+233
-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: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
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+
12+
// Certificate
13+
function generateCertificate() {
14+
// Generate server and client certificates
15+
const attributes = [{ name: 'commonName', value: 'localhost' }];
16+
const x509 = selfsigned.generate(attributes, {
17+
days: 1,
18+
clientCertificate: true,
19+
extensions: [
20+
{
21+
name: 'keyUsage',
22+
keyCertSign: true,
23+
digitalSignature: true,
24+
nonRepudiation: true,
25+
keyEncipherment: true,
26+
dataEncipherment: true
27+
},
28+
{
29+
name: 'subjectAltName',
30+
altNames: [
31+
{
32+
type: 2, // DNS
33+
value: 'localhost'
34+
}
35+
]
36+
}
37+
]
38+
});
39+
return x509;
40+
}
41+
42+
const x509 = generateCertificate();
43+
const validServerTLS = {
44+
key: x509.private,
45+
cert: x509.cert,
46+
ca: [x509.cert],
47+
rejectUnauthorized: true,
48+
requestCert: true
49+
};
50+
51+
const validClientTLS = {
52+
key: x509.clientprivate,
53+
cert: x509.clientcert,
54+
ca: [x509.cert],
55+
rejectUnauthorized: true
56+
};
57+
58+
const untrustedClientTLS = {
59+
...validClientTLS,
60+
ca: null
61+
};
62+
63+
const missingClientTLS = {
64+
ca: null,
65+
key: null,
66+
cert: null,
67+
rejectUnauthorized: false
68+
};
69+
70+
function wrapToPromise(target, event) {
71+
return new Promise((resolve) => {
72+
target.on(event, (...args) => resolve(...args));
73+
});
74+
}
75+
76+
// Start TLS Server
77+
const serverEvents = {
78+
data: 'data',
79+
tlsClientError: 'tlsClientError',
80+
error: 'error',
81+
listening: 'listening',
82+
secureConnection: 'secureConnection'
83+
};
84+
85+
async function startServer({ host = HOST, port = PORT, tlsOpts } = {}) {
86+
const server = tls.createServer({ ...tlsOpts });
87+
let clients = [];
88+
server.on('secureConnection', (client) => {
89+
clients.push(client);
90+
client.on('close', () => {
91+
clients = clients.filter((c) => c !== clients);
92+
});
93+
client.on('data', (data) => {
94+
server.emit(serverEvents.data, data.toString());
95+
});
96+
});
97+
server.forceClose = () => {
98+
clients.forEach((c) => c.destroy());
99+
server.close();
100+
};
101+
server.listen({ host, port });
102+
await wrapToPromise(server, serverEvents.listening);
103+
return server;
104+
}
105+
106+
function initLogger({ host = HOST, port = PORT, tlsOpts } = {}) {
107+
const syslogOptions = {
108+
host,
109+
port,
110+
protocol: 'tls4',
111+
protocolOptions: { ...tlsOpts }
112+
};
113+
const logger = winston.createLogger({
114+
transports: [new winston.transports.Syslog(syslogOptions)]
115+
});
116+
return logger;
117+
}
118+
119+
const TEST_MESSAGE = 'Test Message';
120+
const SYSLOG_FORMAT = `"message":"${TEST_MESSAGE}"`;
121+
122+
let serverInstance;
123+
124+
vows
125+
.describe('tls-connect')
126+
.addBatch({
127+
'Trying to connect to a TLS server with mutual TLS': {
128+
'topic': function () {
129+
startServer({ tlsOpts: validServerTLS }).then((server) => {
130+
serverInstance = server;
131+
const promise = wrapToPromise(server, serverEvents.data);
132+
initLogger({ tlsOpts: validClientTLS }).info(TEST_MESSAGE);
133+
promise.then(msg => this.callback(null, msg));
134+
});
135+
},
136+
'TLS server should receive log message': function (msg) {
137+
assert.include(msg, SYSLOG_FORMAT);
138+
},
139+
'teardown': function () {
140+
serverInstance.forceClose();
141+
}
142+
}
143+
})
144+
.addBatch({
145+
'Trying to connect to a TLS server with untrusted certificate': {
146+
'topic': function () {
147+
startServer({ tlsOpts: validServerTLS }).then((server) => {
148+
serverInstance = server;
149+
const promise = wrapToPromise(server, serverEvents.tlsClientError);
150+
const logger = initLogger({ tlsOpts: untrustedClientTLS });
151+
logger.on('error', (loggerError) => {
152+
promise.then((serverError) => this.callback(null, loggerError, serverError));
153+
});
154+
logger.info(TEST_MESSAGE);
155+
});
156+
},
157+
'Client should refuse connection': function (e, loggerError, serverError) {
158+
assert.equal(loggerError.code, 'DEPTH_ZERO_SELF_SIGNED_CERT');
159+
assert.include(loggerError.message, 'self signed certificate');
160+
assert.equal(serverError.code, 'ECONNRESET');
161+
assert.include(serverError.message, 'socket hang up');
162+
},
163+
'teardown': function () {
164+
serverInstance.forceClose();
165+
}
166+
}
167+
})
168+
.addBatch({
169+
'Trying to connect to a TLS server without client certificate': {
170+
'topic': function () {
171+
startServer({ tlsOpts: validServerTLS }).then((server) => {
172+
serverInstance = server;
173+
const promise = wrapToPromise(server, serverEvents.tlsClientError);
174+
const logger = initLogger({ tlsOpts: missingClientTLS });
175+
logger.on('error', (loggerError) => {
176+
promise.then((serverError) => this.callback(null, loggerError, serverError));
177+
});
178+
logger.info(TEST_MESSAGE);
179+
});
180+
},
181+
'Server should refuse connection': function (e, loggerError, serverError) {
182+
assert.equal(loggerError.code, 'ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED');
183+
assert.equal(serverError.code, 'ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE');
184+
},
185+
'teardown': function () {
186+
serverInstance.forceClose();
187+
}
188+
}
189+
})
190+
.export(module);

0 commit comments

Comments
 (0)