From 786654646e75a3fc5014108032e2d105675ea88f Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Thu, 14 Oct 2021 20:40:24 -0700 Subject: [PATCH] tls_socket: more flexible pem file parsing (#2986) - order of key and cert blocks no longer matters * add EC pem test, fixes #2906 * EOL fix for cross platform (win) support --- Changes.md | 3 + tests/config/tls/ec.pem | 23 +++++ tests/loud/config/dhparams.pem | 0 tests/loud/config/tls.ini | 43 +++++++++ tests/{ => loud}/config/tls/goobered.pem | 0 tests/tls_socket.js | 108 ++++++++++++++++++++--- tls_socket.js | 40 ++++----- 7 files changed, 180 insertions(+), 37 deletions(-) create mode 100644 tests/config/tls/ec.pem create mode 100644 tests/loud/config/dhparams.pem create mode 100644 tests/loud/config/tls.ini rename tests/{ => loud}/config/tls/goobered.pem (100%) diff --git a/Changes.md b/Changes.md index 937e96c1e..384fa0cad 100644 --- a/Changes.md +++ b/Changes.md @@ -4,6 +4,8 @@ ### Changes - breaking: dkim.js has changed the constructor opts +- tls_socket: more flexible pem file parsing #2986 + - move bad certs into different directory, avoid test suite noise - added ability to define a default relay in relay_dest_domains - spamassassin: replace msg_too_big & should_check with should_skip #2972 - spamassassin: allow returning DENYSOFT on errors #2967 @@ -34,6 +36,7 @@ - use RFC-2045 Quoted-Printable in email message body - use RFC-2047 Q encoded-words in email headers + ## 2.8.27 - 2021-01-05 ### Changes diff --git a/tests/config/tls/ec.pem b/tests/config/tls/ec.pem new file mode 100644 index 000000000..b5ca205d7 --- /dev/null +++ b/tests/config/tls/ec.pem @@ -0,0 +1,23 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIIDhiI5q6l7txfMJ6kIEYjK12EFcHLvDIkfWIwzdZBsloAoGCCqGSM49 +AwEHoUQDQgAEZg2nHEFy9nquFPF3DQyQE28e/ytjXeb4nD/8U+L4KHKFtglaX3R4 +uZ+5JcwfcDghpL4Z8h4ouUD/xqe957e2+g== +-----END EC PRIVATE KEY----- +-----BEGIN CERTIFICATE----- +MIICaTCCAg+gAwIBAgIUEDa9VX16wCdo97WvIk7jyEBz1wQwCgYIKoZIzj0EAwIw +gYkxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApXYXNoaW5ndG9uMRAwDgYDVQQHDAdT +ZWF0dGxlMRQwEgYDVQQKDAtIYXJha2EgTWFpbDEXMBUGA1UEAwwObWFpbC5oYXJh +a2EuaW8xJDAiBgkqhkiG9w0BCQEWFWhhcmFrYS5tYWlsQGdtYWlsLmNvbTAeFw0y +MTEwMTQwNjQxMTlaFw0yMjEwMTQwNjQxMTlaMIGJMQswCQYDVQQGEwJVUzETMBEG +A1UECAwKV2FzaGluZ3RvbjEQMA4GA1UEBwwHU2VhdHRsZTEUMBIGA1UECgwLSGFy +YWthIE1haWwxFzAVBgNVBAMMDm1haWwuaGFyYWthLmlvMSQwIgYJKoZIhvcNAQkB +FhVoYXJha2EubWFpbEBnbWFpbC5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC +AARmDaccQXL2eq4U8XcNDJATbx7/K2Nd5vicP/xT4vgocoW2CVpfdHi5n7klzB9w +OCGkvhnyHii5QP/Gp73nt7b6o1MwUTAdBgNVHQ4EFgQU094ROMLHmLEspT4ZoCfX +Rz0mR/YwHwYDVR0jBBgwFoAU094ROMLHmLEspT4ZoCfXRz0mR/YwDwYDVR0TAQH/ +BAUwAwEB/zAKBggqhkjOPQQDAgNIADBFAiEAsmshzvMDjmYDHyGRrKdMmsnnESFd +GMtfRXYIv0AZe7ICIGD2Sta9LL0zZ44ARGXhh+sPjxd78I/+0FdIPsofr2I+ +-----END CERTIFICATE----- diff --git a/tests/loud/config/dhparams.pem b/tests/loud/config/dhparams.pem new file mode 100644 index 000000000..e69de29bb diff --git a/tests/loud/config/tls.ini b/tests/loud/config/tls.ini new file mode 100644 index 000000000..84bb4be6f --- /dev/null +++ b/tests/loud/config/tls.ini @@ -0,0 +1,43 @@ + +key = tls_key.pem +cert = tls_cert.pem +dhparam = dhparams.pem + +ciphers = ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA384 +minVersion = TLSv1 +rejectUnauthorized=false +requestCert=true +honorCipherOrder=true +requireAuthorized[]=2465 +requireAuthorized[]=2587 + +no_starttls_ports[]=2525 + +[redis] +; options in this block require redis to be available. + +; remember when a remote fails STARTTLS. The next time they connect, +; don't offer STARTTLS option (so message gets delivered). +; pro: increases mail reliability +; con: reduces security +; default: false +; disable_for_failed_hosts=true + + +; no_tls_hosts - disable TLS for servers with broken TLS. +[no_tls_hosts] +; 127.0.0.1 +; 192.168.1.1 +; 172.16.0.0/16 + +[outbound] +key = outbound_tls_key.pem +cert = outbound_tls_cert.pem +dhparam = dhparams.pem +ciphers = ECDHE-RSA-AES256-GCM-SHA384 +minVersion = TLSv1 +rejectUnauthorized=false +requestCert=false +honorCipherOrder=false +force_tls_hosts[]=first.example.com +force_tls_hosts[]=second.example.net diff --git a/tests/config/tls/goobered.pem b/tests/loud/config/tls/goobered.pem similarity index 100% rename from tests/config/tls/goobered.pem rename to tests/loud/config/tls/goobered.pem diff --git a/tests/tls_socket.js b/tests/tls_socket.js index 5df097336..ad7b38d2f 100644 --- a/tests/tls_socket.js +++ b/tests/tls_socket.js @@ -1,8 +1,8 @@ - -const path = require('path'); +const fs = require('fs') +const path = require('path') +const os = require('os') function _setup (done) { - this.socket = require('../tls_socket'); // use tests/config instead of ./config @@ -72,13 +72,26 @@ exports.load_tls_ini = { }, } +exports.get_loud_certs_dir = { + setUp: _setup, + 'loads certs from tests/loud/config/tls' (test) { + test.expect(2); + this.socket.config = this.socket.config.module_config(path.resolve('tests', 'loud')); + this.socket.get_certs_dir('tls', (err, certs) => { + test.ifError(err); + test.ok(certs); + test.done(); + }) + } +} + exports.get_certs_dir = { setUp: _setup, - 'loads certs from config/tls' (test) { + 'loads certs from tests/config/tls' (test) { test.expect(2); + this.socket.config = this.socket.config.module_config(path.resolve('tests')); this.socket.get_certs_dir('tls', (err, certs) => { test.ifError(err); - // console.error(certs); test.ok(certs); test.done(); }) @@ -189,24 +202,93 @@ exports.parse_x509 = { }, 'returns key from BEGIN PRIVATE KEY block' (test) { const res = this.socket.parse_x509('-BEGIN PRIVATE KEY-\nhello\n--END PRIVATE KEY--\n-its me-\n'); - res.key.toString(); test.deepEqual( res.key.toString(), - '-BEGIN PRIVATE KEY-\nhello\n--END PRIVATE KEY--\n' + '-BEGIN PRIVATE KEY-\nhello\n--END PRIVATE KEY--' ); - // everything after the private key is cert(s) - test.deepEqual(res.cert.toString(), '-its me-\n'); + test.deepEqual(res.cert, undefined); test.done(); }, 'returns key from BEGIN RSA PRIVATE KEY block' (test) { const res = this.socket.parse_x509('-BEGIN RSA PRIVATE KEY-\nhello\n--END RSA PRIVATE KEY--\n-its me-\n'); - res.key.toString(); test.deepEqual( res.key.toString(), - '-BEGIN RSA PRIVATE KEY-\nhello\n--END RSA PRIVATE KEY--\n' + '-BEGIN RSA PRIVATE KEY-\nhello\n--END RSA PRIVATE KEY--' ); - // everything after the private key is cert(s) - test.deepEqual(res.cert.toString(), '-its me-\n'); + test.deepEqual(res.cert, undefined); + test.done(); + }, + 'returns a key and certificate chain' (test) { + const str = `-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAoDGOlvw6lQptaNwqxYsW4aJCPIgvjYw3qA9Y0qykp8I8PapT +ercA8BsInrZg5+3wt2PT1+REprBvv6xfHyQ08o/udsSCBRf4Awadp0fxzUulENNi +3wWuuPy0WgaE4jam7tWItDBeEhXkEfcMTr9XkFxenuTcNw9O1+E8TtNP9KMmJDAe + +F+T5AoGAMRH1+JrjTpPYcs1hOyHMWnxkHv7fsJMJY/KN2NPoTlI4d4V1W5xyCZ0D +rl7RlVdVTQdZ9VjkWVjJcafNSmNyQEK4IQsaczwOU59IPhC/nUAyRgeoRbKWPQ4r +mj3g7uX9f07j34c01mH1zLgDa24LO9SW7B5ZbYYu4DORk7005B4= +-----END RSA PRIVATE KEY----- +-----BEGIN CERTIFICATE----- +MIIFVzCCBD+gAwIBAgISA/5ofbB6cUAp/PrYaBxTITF2MA0GCSqGSIb3DQEBCwUA +MDIxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MQswCQYDVQQD + +kOk4JdlpuBSPwx9wNAEYF15/4LDyev+tyAg7GxCZ9MW53leOxF+j2NQgc4kRIdQc +DYsruShsnwn4HErJKQAfE5Aq77UM32hfKzMb2PH6Ebw0TB2NCLVocOULAGTw4NPO +wBpsGsIFUxeDHZvhKohZyNqLrj7gR+XlKRKM +-----END CERTIFICATE----- + +-----BEGIN CERTIFICATE----- +MIIFFjCCAv6gAwIBAgIRAJErCErPDBinU/bWLiWnX1owDQYJKoZIhvcNAQELBQAw +TzELMAkGA1UEBhMCVVMxKTAnBgNVBAoTIEludGVybmV0IFNlY3VyaXR5IFJlc2Vh +cmNoIEdyb3VwMRUwEwYDVQQDEwxJU1JHIFJvb3QgWDEwHhcNMjAwOTA0MDAwMDAw + +HlUjr8gRsI3qfJOQFy/9rKIJR0Y/8Omwt/8oTWgy1mdeHmmjk7j1nYsvC9JSQ6Zv +MldlTTKB3zhThV1+XWYp6rjd5JW1zbVWEkLNxE7GJThEUG3szgBVGP7pSWTUTsqX +nLRbwHOoq7hHwg== +-----END CERTIFICATE----- + +-----BEGIN CERTIFICATE----- +MIIFYDCCBEigAwIBAgIQQAF3ITfU6UK47naqPGQKtzANBgkqhkiG9w0BAQsFADA/ +MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT +DkRTVCBSb290IENBIFgzMB4XDTIxMDEyMDE5MTQwM1oXDTI0MDkzMDE4MTQwM1ow + +WCLKTVXkcGdtwlfFRjlBz4pYg1htmf5X6DYO8A4jqv2Il9DjXA6USbW1FzXSLr9O +he8Y4IWS6wY7bCkjCWDcRQJMEhg76fsO3txE+FiYruq9RUWhiF1myv4Q6W+CyBFC +Dfvp7OOGAN6dEOM4+qR9sdjoSYKEBpsr6GtPAQw4dy753ec5 +-----END CERTIFICATE-----` + const res = this.socket.parse_x509(str); + test.deepEqual(res.key.length, 446); + test.deepEqual(res.cert.length, 1195); + test.done(); + }, + 'returns cert and key from EC pem' (test) { + const fp = fs.readFileSync(path.join('tests','config','tls','ec.pem')) + const res = this.socket.parse_x509(fp.toString()) + test.deepEqual( + res.key.toString().split(os.EOL).join('\n'), + `-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIIDhiI5q6l7txfMJ6kIEYjK12EFcHLvDIkfWIwzdZBsloAoGCCqGSM49 +AwEHoUQDQgAEZg2nHEFy9nquFPF3DQyQE28e/ytjXeb4nD/8U+L4KHKFtglaX3R4 +uZ+5JcwfcDghpL4Z8h4ouUD/xqe957e2+g== +-----END EC PRIVATE KEY-----` + ); + test.deepEqual( + res.cert.toString().split(os.EOL).join('\n'), + `-----BEGIN CERTIFICATE----- +MIICaTCCAg+gAwIBAgIUEDa9VX16wCdo97WvIk7jyEBz1wQwCgYIKoZIzj0EAwIw +gYkxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApXYXNoaW5ndG9uMRAwDgYDVQQHDAdT +ZWF0dGxlMRQwEgYDVQQKDAtIYXJha2EgTWFpbDEXMBUGA1UEAwwObWFpbC5oYXJh +a2EuaW8xJDAiBgkqhkiG9w0BCQEWFWhhcmFrYS5tYWlsQGdtYWlsLmNvbTAeFw0y +MTEwMTQwNjQxMTlaFw0yMjEwMTQwNjQxMTlaMIGJMQswCQYDVQQGEwJVUzETMBEG +A1UECAwKV2FzaGluZ3RvbjEQMA4GA1UEBwwHU2VhdHRsZTEUMBIGA1UECgwLSGFy +YWthIE1haWwxFzAVBgNVBAMMDm1haWwuaGFyYWthLmlvMSQwIgYJKoZIhvcNAQkB +FhVoYXJha2EubWFpbEBnbWFpbC5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC +AARmDaccQXL2eq4U8XcNDJATbx7/K2Nd5vicP/xT4vgocoW2CVpfdHi5n7klzB9w +OCGkvhnyHii5QP/Gp73nt7b6o1MwUTAdBgNVHQ4EFgQU094ROMLHmLEspT4ZoCfX +Rz0mR/YwHwYDVR0jBBgwFoAU094ROMLHmLEspT4ZoCfXRz0mR/YwDwYDVR0TAQH/ +BAUwAwEB/zAKBggqhkjOPQQDAgNIADBFAiEAsmshzvMDjmYDHyGRrKdMmsnnESFd +GMtfRXYIv0AZe7ICIGD2Sta9LL0zZ44ARGXhh+sPjxd78I/+0FdIPsofr2I+ +-----END CERTIFICATE-----`); test.done(); }, } diff --git a/tls_socket.js b/tls_socket.js index ce36d30d0..ec7c28082 100644 --- a/tls_socket.js +++ b/tls_socket.js @@ -210,19 +210,15 @@ exports.parse_x509_expire = (file, string) => { exports.parse_x509 = string => { const res = {}; + if (!string) return res - const match = /^([^-]*)?([-]+BEGIN (?:\w+\s)?PRIVATE KEY[-]+[^-]+[-]+END (?:\w+\s)?PRIVATE KEY[-]+\n)([^]*)$/.exec(string); - if (!match) return res; + const keyRe = new RegExp('([-]+BEGIN (?:\\w+ )?PRIVATE KEY[-]+[^-]*[-]+END (?:\\w+ )?PRIVATE KEY[-]+)', 'gm') + const keys = string.match(keyRe) + if (keys) res.key = Buffer.from(keys.join('\n')); - if (match[1] && match[1].length) { - log.logerror('leading garbage'); - log.logerror(match[1]); - } - if (!match[2] || !match[2].length) return res; - res.key = Buffer.from(match[2]); - - if (!match[3] || !match[3].length) return res; - res.cert = Buffer.from(match[3]); + const certRe = new RegExp('([-]+BEGIN CERTIFICATE[-]+[^-]*[-]+END CERTIFICATE[-]+)', 'gm') + const certs = string.match(certRe) + if (certs) res.cert = Buffer.from(certs.join('\n')); return res; } @@ -320,19 +316,19 @@ exports.applySocketOpts = name => { const allOpts = TLSSocketOptions.concat(createSecureContextOptions); - allOpts.forEach(opt => { + for (const opt of allOpts) { if (tlss.cfg[name] && tlss.cfg[name][opt] !== undefined) { // if the setting exists in tls.ini [name] tlss.saveOpt(name, opt, tlss.cfg[name][opt]); - return; + continue; } if (tlss.cfg.main[opt] !== undefined) { // if the setting exists in tls.ini [main] // then save it to the certsByHost options tlss.saveOpt(name, opt, tlss.cfg.main[opt]); - return; + continue; } // defaults @@ -356,7 +352,7 @@ exports.applySocketOpts = name => { tlss.saveOpt(name, opt, SNICallback); break; } - }) + } } exports.load_default_opts = () => { @@ -637,15 +633,11 @@ exports.ocsp = ocsp; exports.get_rejectUnauthorized = (rejectUnauthorized, port, port_list) => { // console.log(`rejectUnauthorized: ${rejectUnauthorized}, port ${port}, list: ${port_list}`) - if (rejectUnauthorized) { - // console.log('true for all ports'); - return true; - } - if (port_list.includes(port)) { - // console.log('port matched true'); - return true; - } - // console.log('returning default false'); + + if (rejectUnauthorized) return true; + + if (port_list.includes(port)) return true; + return false; }