Skip to content

Commit ac9d9cd

Browse files
author
Jamie Curnow
committed
Fix SSL custom certificates being saved, nginx errors being reported and leaking custom certs in api calls
1 parent 5ac0e3d commit ac9d9cd

File tree

7 files changed

+108
-21
lines changed

7 files changed

+108
-21
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "nginx-proxy-manager",
3-
"version": "2.0.1",
3+
"version": "2.0.2",
44
"description": "A beautiful interface for creating Nginx endpoints",
55
"main": "src/backend/index.js",
66
"devDependencies": {

‎src/backend/internal/certificate.js‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,7 @@ const internalCertificate = {
183183
});
184184
});
185185
} else {
186-
return internalCertificate.writeCustomCert(certificate)
187-
.then(() => {
188-
return certificate;
189-
});
186+
return certificate;
190187
}
191188
}).then(certificate => {
192189

@@ -409,6 +406,10 @@ const internalCertificate = {
409406
* @returns {Promise}
410407
*/
411408
writeCustomCert: certificate => {
409+
if (debug_mode) {
410+
logger.info('Writing Custom Certificate:', certificate);
411+
}
412+
412413
let dir = '/data/custom_ssl/npm-' + certificate.id;
413414

414415
return new Promise((resolve, reject) => {
@@ -549,8 +550,13 @@ const internalCertificate = {
549550
id: data.id,
550551
expires_on: certificateModel.raw('FROM_UNIXTIME(' + validations.certificate.dates.to + ')'),
551552
domain_names: [validations.certificate.cn],
552-
meta: row.meta
553-
});
553+
meta: _.clone(row.meta) // Prevent the update method from changing this value that we'll use later
554+
})
555+
.then(certificate => {
556+
console.log('ROWMETA:', row.meta);
557+
certificate.meta = row.meta;
558+
return internalCertificate.writeCustomCert(certificate);
559+
})
554560
})
555561
.then(() => {
556562
return _.pick(row.meta, internalCertificate.allowed_ssl_files);

‎src/backend/internal/dead-host.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ const internalDeadHost = {
189189
.then(row => {
190190
// Configure nginx
191191
return internalNginx.configure(deadHostModel, 'dead_host', row)
192-
.then(() => {
192+
.then(new_meta => {
193+
row.meta = new_meta;
194+
row = internalHost.cleanRowCertificateMeta(row);
193195
return _.omit(row, omissions());
194196
});
195197
});
@@ -235,6 +237,7 @@ const internalDeadHost = {
235237
})
236238
.then(row => {
237239
if (row) {
240+
row = internalHost.cleanRowCertificateMeta(row);
238241
return _.omit(row, omissions());
239242
} else {
240243
throw new error.ItemNotFoundError(data.id);
@@ -322,6 +325,13 @@ const internalDeadHost = {
322325
}
323326

324327
return query;
328+
})
329+
.then(rows => {
330+
if (typeof expand !== 'undefined' && expand !== null && expand.indexOf('certificate') !== -1) {
331+
return internalHost.cleanAllRowsCertificateMeta(rows);
332+
}
333+
334+
return rows;
325335
});
326336
},
327337

‎src/backend/internal/host.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,36 @@ const deadHostModel = require('../models/dead_host');
66

77
const internalHost = {
88

9+
/**
10+
* used by the getAll functions of hosts, this removes the certificate meta if present
11+
*
12+
* @param {Array} rows
13+
* @returns {Array}
14+
*/
15+
cleanAllRowsCertificateMeta: function (rows) {
16+
rows.map(function (row, idx) {
17+
if (typeof rows[idx].certificate !== 'undefined' && rows[idx].certificate) {
18+
rows[idx].certificate.meta = {};
19+
}
20+
});
21+
22+
return rows;
23+
},
24+
25+
/**
26+
* used by the get/update functions of hosts, this removes the certificate meta if present
27+
*
28+
* @param {Object} row
29+
* @returns {Object}
30+
*/
31+
cleanRowCertificateMeta: function (row) {
32+
if (typeof row.certificate !== 'undefined' && row.certificate) {
33+
row.certificate.meta = {};
34+
}
35+
36+
return row;
37+
},
38+
939
/**
1040
* This returns all the host types with any domain listed in the provided domain_names array.
1141
* This is used by the certificates to temporarily disable any host that is using the domain

‎src/backend/internal/nginx.js‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ const internalNginx = {
2525
* @returns {Promise}
2626
*/
2727
configure: (model, host_type, host) => {
28+
let combined_meta = {};
29+
2830
return internalNginx.test()
2931
.then(() => {
3032
// Nginx is OK
@@ -39,30 +41,46 @@ const internalNginx = {
3941
return internalNginx.test()
4042
.then(() => {
4143
// nginx is ok
44+
combined_meta = _.assign({}, host.meta, {
45+
nginx_online: true,
46+
nginx_err: null
47+
});
48+
4249
return model
4350
.query()
4451
.where('id', host.id)
4552
.patch({
46-
meta: _.assign({}, host.meta, {
47-
nginx_online: true,
48-
nginx_err: null
49-
})
53+
meta: combined_meta
5054
});
5155
})
5256
.catch(err => {
57+
// Remove the error_log line because it's a docker-ism false positive that doesn't need to be reported.
58+
// It will always look like this:
59+
// nginx: [alert] could not open error log file: open() "/var/log/nginx/error.log" failed (6: No such device or address)
60+
61+
let valid_lines = [];
62+
let err_lines = err.message.split("\n");
63+
err_lines.map(function (line) {
64+
if (line.indexOf('/var/log/nginx/error.log') === -1) {
65+
valid_lines.push(line);
66+
}
67+
});
68+
5369
if (debug_mode) {
54-
logger.error('Nginx test failed:', err.message);
70+
logger.error('Nginx test failed:', valid_lines.join("\n"));
5571
}
5672

5773
// config is bad, update meta and delete config
74+
combined_meta = _.assign({}, host.meta, {
75+
nginx_online: false,
76+
nginx_err: valid_lines.join("\n")
77+
});
78+
5879
return model
5980
.query()
6081
.where('id', host.id)
6182
.patch({
62-
meta: _.assign({}, host.meta, {
63-
nginx_online: false,
64-
nginx_err: err.message
65-
})
83+
meta: combined_meta
6684
})
6785
.then(() => {
6886
return internalNginx.deleteConfig(host_type, host, true);
@@ -71,7 +89,10 @@ const internalNginx = {
7189
})
7290
.then(() => {
7391
return internalNginx.reload();
74-
});
92+
})
93+
.then(() => {
94+
return combined_meta;
95+
})
7596
},
7697

7798
/**
@@ -82,7 +103,7 @@ const internalNginx = {
82103
logger.info('Testing Nginx configuration');
83104
}
84105

85-
return utils.exec('/usr/sbin/nginx -t');
106+
return utils.exec('/usr/sbin/nginx -t -g "error_log off;"');
86107
},
87108

88109
/**

‎src/backend/internal/proxy-host.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ const internalProxyHost = {
190190
.then(row => {
191191
// Configure nginx
192192
return internalNginx.configure(proxyHostModel, 'proxy_host', row)
193-
.then(() => {
193+
.then(new_meta => {
194+
row.meta = new_meta;
195+
row = internalHost.cleanRowCertificateMeta(row);
194196
return _.omit(row, omissions());
195197
});
196198
});
@@ -236,6 +238,7 @@ const internalProxyHost = {
236238
})
237239
.then(row => {
238240
if (row) {
241+
row = internalHost.cleanRowCertificateMeta(row);
239242
return _.omit(row, omissions());
240243
} else {
241244
throw new error.ItemNotFoundError(data.id);
@@ -323,6 +326,13 @@ const internalProxyHost = {
323326
}
324327

325328
return query;
329+
})
330+
.then(rows => {
331+
if (typeof expand !== 'undefined' && expand !== null && expand.indexOf('certificate') !== -1) {
332+
return internalHost.cleanAllRowsCertificateMeta(rows);
333+
}
334+
335+
return rows;
326336
});
327337
},
328338

‎src/backend/internal/redirection-host.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ const internalRedirectionHost = {
189189
.then(row => {
190190
// Configure nginx
191191
return internalNginx.configure(redirectionHostModel, 'redirection_host', row)
192-
.then(() => {
192+
.then(new_meta => {
193+
row.meta = new_meta;
194+
row = internalHost.cleanRowCertificateMeta(row);
193195
return _.omit(row, omissions());
194196
});
195197
});
@@ -235,6 +237,7 @@ const internalRedirectionHost = {
235237
})
236238
.then(row => {
237239
if (row) {
240+
row = internalHost.cleanRowCertificateMeta(row);
238241
return _.omit(row, omissions());
239242
} else {
240243
throw new error.ItemNotFoundError(data.id);
@@ -322,6 +325,13 @@ const internalRedirectionHost = {
322325
}
323326

324327
return query;
328+
})
329+
.then(rows => {
330+
if (typeof expand !== 'undefined' && expand !== null && expand.indexOf('certificate') !== -1) {
331+
return internalHost.cleanAllRowsCertificateMeta(rows);
332+
}
333+
334+
return rows;
325335
});
326336
},
327337

0 commit comments

Comments
 (0)