From 1156a9e5f849f32a3664587f9cf37cd876fe0dd1 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Nov 2019 12:47:00 -0800 Subject: [PATCH] crypto: fix assertion caused by unsupported ext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `X509V3_EXT_print` can return value different from `1` if the X509 extension does not support printing to a buffer. Instead of failing with an unrecoverable assertion - replace the relevant value in the hashmap with a JS null value. Fixes: https://hackerone.com/reports/746733 PR-URL: https://github.com/nodejs-private/node-private/pull/175 Reviewed-By: Sam Roberts  Reviewed-By: Matteo Collina  Reviewed-By: Ben Noordhuis  Reviewed-By: Rich Trott  Reviewed-By: Anna Henningsen  Reviewed-By: Shelley Vohr  Reviewed-By: Tobias Nießen  Reviewed-By: Gireesh Punathil  Reviewed-By: Beth Griggs  Reviewed-By: Michael Dawson  --- src/node_crypto.cc | 7 +- test/parallel/test-tls-cert-ext-encoding.js | 79 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-tls-cert-ext-encoding.js diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9165e9171e6e6f..afec3a5fc86552 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2011,8 +2011,11 @@ static Local X509ToObject(Environment* env, X509* cert) { X509_EXTENSION* ext = X509_get_ext(cert, index); CHECK_NOT_NULL(ext); - if (!SafeX509ExtPrint(bio.get(), ext)) { - CHECK_EQ(1, X509V3_EXT_print(bio.get(), ext, 0, 0)); + if (!SafeX509ExtPrint(bio.get(), ext) && + X509V3_EXT_print(bio.get(), ext, 0, 0) != 1) { + info->Set(context, keys[i], Null(env->isolate())).Check(); + USE(BIO_reset(bio.get())); + continue; } BIO_get_mem_ptr(bio.get(), &mem); diff --git a/test/parallel/test-tls-cert-ext-encoding.js b/test/parallel/test-tls-cert-ext-encoding.js new file mode 100644 index 00000000000000..d4b89473dc126d --- /dev/null +++ b/test/parallel/test-tls-cert-ext-encoding.js @@ -0,0 +1,79 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// NOTE: This certificate is hand-generated, hence it is not located in +// `test/fixtures/keys` to avoid confusion. +// +// The key property of this cert is that subjectAltName contains a string with +// a type `23` which cannot be encoded into string by `X509V3_EXT_print`. +const pem = ` +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAzrmfPz5M3wTq2/CwMeSQr/N+R1FCJ+O5n+SMleKvBqaK63eJ +kL4BnySMc+ZLKCt4UQSsPFIBK63QFq8n6/vjuTDMJiBTsvzytw8zJt1Zr2HA71N3 +VIPt6NdJ/w5lgddTYxR7XudJZJ5lk3PkG8ZgrhuenPYP80UJYVzAC2YZ9KYe3r2B +rVbut1j+8h0TwVcx2Zg5PorsC/EVxHwo4dCmIHceodikr3UVqHneRcrDBytdG6Mo +IqHhZJwBeii/EES9tpWwWbzYYh+38aGGLIF2h5UlVpr0bdBVVUg+uVX3y/Qmu2Qv +4CrAO2IPV6JER9Niwl3ktzNjOMAUQG6BCRSqRQIDAQABAoIBAAmB0+cOsG5ZRYvT +5+aDgnv1EMuq2wYGnRTTZ/vErxP5OM5XcwYrFtwAzEzQPIieZywisOEdTFx74+QH +LijWLsTnj5v5RKAorejpVArnhyZfsoXPKt/CKYDZ1ddbDCQKiRU3be0RafisqDM9 +0zHLz8pyDrtdPaKMfD/0Cgj8KxlrLTmfD4otPXds8fZpQe1hR1y12XKVp47l1siW +qFGTaUPDJpQ67xybR08x5DOqmyo4cNMOuReRWrc/qRbWint9U1882eOH09gVfpJZ +Gp6FZVPSgz10MZdLSPLhXqZkY4IxIvNltjBDqkmivd12CD+GVr0qUmTJHzTpk+kG +/CWuRQkCgYEA4EFf8SJHEl0fLDJnOQFyUPY3MalMuopUkQ5CBUe3QXjQhHXsRDfj +Ci/lyzShJkHPbMDHb/rx3lYZB0xNhwnMWKS1gCFVgOCOTZLfD0K1Anxc1hOSgVxI +y5FdO9VW7oQNlsMH/WuDHps0HhJW/00lcrmdyoUM1+fE/3yPQndhUmMCgYEA6/z6 +8Gq4PHHNql+gwunAH2cZKNdmcP4Co8MvXCZwIJsLenUuLIZQ/YBKZoM/y5X/cFAG +WFJJuUe6KFetPaDm6NgZgpOmawyUwd5czDjJ6wWgsRywiTISInfJlgWLBVMOuba7 +iBL9Xuy0hmcbj0ByoRW9l3gCiBX3yJw3I6wqXTcCgYBnjei22eRF15iIeTHvQfq+ +5iNwnEQhM7V/Uj0sYQR/iEGJmUaj7ca6somDf2cW2nblOlQeIpxD1jAyjYqTW/Pv +zwc9BqeMHqW3rqWwT1Z0smbQODOD5tB6qEKMWaSN+Y6o2qC65kWjAXpclI110PME ++i+iEDRxEsaGT8d7otLfDwKBgQCs+xBaQG/x5p2SAGzP0xYALstzc4jk1FzM+5rw +mkBgtiXQyqpg+sfNOkfPIvAVZEsMYax0+0SNKrWbMsGLRjFchmMUovQ+zccQ4NT2 +4b2op8Rlbxk8R9ahK1s5u7Bu47YMjZSjJwBQn4OobVX3SI994njJ2a9JX4j0pQWK +AX5AOwKBgAfOsr8HSHTcxSW4F9gegj+hXsRYbdA+eUkFhEGrYyRJgIlQrk/HbuZC +mKd/bQ5R/vwd1cxgV6A0APzpZtbwdhvP0RWji+WnPPovgGcfK0AHFstHnga67/uu +h2LHnKQZ1qWHn+BXWo5d7hBRwWVaK66g3GDN0blZpSz1kKcpy1Pl +-----END RSA PRIVATE KEY----- +-----BEGIN CERTIFICATE----- +MIICwjCCAaqgAwIBAgIDAQABMA0GCSqGSIb3DQEBDQUAMBUxEzARBgNVBAMWCmxv +Y2FsLmhvc3QwHhcNMTkxMjA1MDQyODMzWhcNNDQxMTI5MDQyODMzWjAVMRMwEQYD +VQQDFgpsb2NhbC5ob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +zrmfPz5M3wTq2/CwMeSQr/N+R1FCJ+O5n+SMleKvBqaK63eJkL4BnySMc+ZLKCt4 +UQSsPFIBK63QFq8n6/vjuTDMJiBTsvzytw8zJt1Zr2HA71N3VIPt6NdJ/w5lgddT +YxR7XudJZJ5lk3PkG8ZgrhuenPYP80UJYVzAC2YZ9KYe3r2BrVbut1j+8h0TwVcx +2Zg5PorsC/EVxHwo4dCmIHceodikr3UVqHneRcrDBytdG6MoIqHhZJwBeii/EES9 +tpWwWbzYYh+38aGGLIF2h5UlVpr0bdBVVUg+uVX3y/Qmu2Qv4CrAO2IPV6JER9Ni +wl3ktzNjOMAUQG6BCRSqRQIDAQABoxswGTAXBgNVHREEEDAOlwwqLmxvY2FsLmhv +c3QwDQYJKoZIhvcNAQENBQADggEBAH5ThRLDLwOGuhKsifyiq7k8gbx1FqRegO7H +SIiIYYB35v5Pk0ZPN8QBJwNQzJEjUMjCpHXNdBxknBXRaA8vkbnryMfJm37gPTwA +m6r0uEG78WgcEAe8bgf9iKtQGP/iydKXpSSpDgKoHbswIxD5qtzT+o6VNnkRTSfK +/OGwakluFSoJ/Q9rLpR8lKjA01BhetXMmHbETiY8LSkxOymMldXSzUTD1WdrVn8U +L3dobxT//R/0GraKXG02mf3gZNlb0MMTvW0pVwVy39YmcPEGh8L0hWh1rpAA/VXC +f79uOowv3lLTzQ9na5EThA0tp8d837hdYrrIHh5cfTqBDxG0Tu8= +-----END CERTIFICATE----- +`; + +const tls = require('tls'); + +const options = { + key: pem, + cert: pem, +}; + +const server = tls.createServer(options, (socket) => { + socket.end(); +}); +server.listen(0, common.mustCall(function() { + const client = tls.connect({ + port: this.address().port, + rejectUnauthorized: false + }, common.mustCall(() => { + // This should not crash process: + client.getPeerCertificate(); + + server.close(); + client.end(); + })); +}));