From 23b07043e7fde743ff920f8354b5a094fee19a03 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Sun, 16 Feb 2020 07:34:52 +0900 Subject: Revert add_certificate_chain_file changes (#320) Revert SSLContext#add_certificate_chain_file changes * 0da0dfaf09f549b2b2cd984627b321b7908d1186. * 8d12f0f6ca944212cb8000e689469d7aaa8190d7. * 49f42ad5f82f8b61f51a16e3a6df1ab0d5307d5f. * 5ee295ab8e37c8ffc6eb8c1b7b79ec024f3253e4. * 8b4fa5e336c7544ea677ccee160ec6d221559e10. * 443d13e9b2c127230fde2733959eaa4d41eb355d. * 5d866038920edf2729865653d6dc9309589f089a. * f18559acf97a6f6aaf3d253417eb0100b262cbc6. --- History.md | 2 - ext/openssl/ossl_ssl.c | 87 +++-------------------------------------- test/test_ssl.rb | 103 +------------------------------------------------ 3 files changed, 7 insertions(+), 185 deletions(-) diff --git a/History.md b/History.md index 42142d61..cdb44b12 100644 --- a/History.md +++ b/History.md @@ -30,8 +30,6 @@ Version 2.2.0 (not yet released) * Make `OpenSSL::PKey::RSA#{export,to_der}` correctly check `key`, `factors`, and `crt_params`. [[GitHub #258]](https://github.com/ruby/openssl/pull/258) -* Add `OpenSSL::SSL::SSLContext#add_certificate_chain_file` for - handling certificate chains. * Add `OpenSSL::SSL::{SSLSocket,SSLServer}#fileno`, returning the underlying socket file descriptor number. [[GitHub #247]](https://github.com/ruby/openssl/pull/247) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index b1f8fd58..dfbfbb22 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1322,92 +1322,15 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) return self; } -/* - * call-seq: - * ctx.add_certificate_chain_file(certs_path, pkey_path) -> self - * - * Loads chain certificates from _certs_path_ and a private key from - * _pkey_path_. - * - * === Parameters - * _certs_path_:: - * A path to a chain certificates file. It may be a single certificate. - * An instance of String. - * _pkey_path_:: - * A path to a private key file. An instance of String. - * - * === Example - * ctx.add_certificate_chain_file(rsa_and_intermediate_certs_path, rsa_key_path) - * - * ctx.add_certificate_chain_file(ecdsa_and_intermediate_certs_path, ecdsa_key_path) - * - * === Note - * The file format of the certificate and private key must be PEM. - * - * The certificate file must be starting with the subject's certificate and - * followed by intermediate CA certificate(s). - * - * OpenSSL before the version 1.0.2 could handle only one extra chain across - * all key types. Calling this method discards the chain set previously. - */ static VALUE -ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_path) +ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE path) { - SSL_CTX *ctx; - X509 *x509; - char *ccerts_path, *cpkey_path; - FILE *fp; - EVP_PKEY *pkey, *pub_pkey; + StringValue(path); + SSL_CTX *ctx = NULL; GetSSLCTX(self, ctx); - /* Retrieve private key */ - cpkey_path = StringValueCStr(pkey_path); - fp = fopen(cpkey_path, "r"); - if (!fp) - rb_raise(rb_eArgError, "failed to open pkey file"); - pkey = PEM_read_PrivateKey(fp, NULL, 0, NULL); - fclose(fp); - if (!pkey) - rb_raise(rb_eArgError, "failed to open pkey file"); - - /* Retrieve public key */ - ccerts_path = StringValueCStr(certs_path); - fp = fopen(ccerts_path, "r"); - if (!fp) { - EVP_PKEY_free(pkey); - rb_raise(rb_eArgError, "failed to open certs file"); - } - x509 = PEM_read_X509(fp, NULL, 0, NULL); - fclose(fp); - if (!x509) { - EVP_PKEY_free(pkey); - rb_raise(rb_eArgError, "failed to open certs file"); - } - pub_pkey = X509_get_pubkey(x509); - if (!pub_pkey) { - EVP_PKEY_free(pkey); - X509_free(x509); - rb_raise(rb_eArgError, "certificate does not contain public key"); - } - if (EVP_PKEY_cmp(pub_pkey, pkey) != 1) { - EVP_PKEY_free(pkey); - X509_free(x509); - EVP_PKEY_free(pub_pkey); - rb_raise(rb_eArgError, "public key mismatch"); - } - EVP_PKEY_free(pkey); - X509_free(x509); - EVP_PKEY_free(pub_pkey); - - /* SSL_CTX_use_certificate_chain_file() loads PEM format file. */ - if (SSL_CTX_use_certificate_chain_file(ctx, ccerts_path) != 1) - ossl_raise(eSSLError, "SSL_CTX_use_certificate_chain_file"); - - if (SSL_CTX_use_PrivateKey_file(ctx, cpkey_path, SSL_FILETYPE_PEM) != 1) - ossl_raise(eSSLError, "SSL_CTX_use_PrivateKey_file"); - - return self; + return SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(path)) == 1 ? Qtrue : Qfalse; } /* @@ -2861,7 +2784,7 @@ Init_ossl_ssl(void) rb_define_method(cSSLContext, "enable_fallback_scsv", ossl_sslctx_enable_fallback_scsv, 0); #endif rb_define_method(cSSLContext, "add_certificate", ossl_sslctx_add_certificate, -1); - rb_define_method(cSSLContext, "add_certificate_chain_file", ossl_sslctx_add_certificate_chain_file, 2); + rb_define_method(cSSLContext, "add_certificate_chain_file", ossl_sslctx_add_certificate_chain_file, 1); rb_define_method(cSSLContext, "setup", ossl_sslctx_setup, 0); rb_define_alias(cSSLContext, "freeze", "setup"); diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 6bfd3111..eb5b77be 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -186,107 +186,8 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end def test_add_certificate_chain_file - # Create chain certificates file - certs = Tempfile.open { |f| f << @svr_cert.to_pem << @ca_cert.to_pem; f } - pkey = Tempfile.open { |f| f << @svr_key.to_pem; f } - - ctx_proc = -> ctx { - # Unset values set by start_server - ctx.cert = ctx.key = ctx.extra_chain_cert = nil - assert_nothing_raised { ctx.add_certificate_chain_file(certs.path, pkey.path) } - } - - start_server(ctx_proc: ctx_proc) { |port| - server_connect(port) { |ssl| - assert_equal @svr_cert.subject, ssl.peer_cert.subject - assert_equal [@svr_cert.subject, @ca_cert.subject], - ssl.peer_cert_chain.map(&:subject) - - ssl.puts "abc"; assert_equal "abc\n", ssl.gets - } - } - ensure - certs&.close - pkey&.close - certs&.unlink - pkey&.unlink - end - - def test_add_certificate_chain_file_multiple_certs - pend "EC is not supported" unless defined?(OpenSSL::PKey::EC) - pend "TLS 1.2 is not supported" unless tls12_supported? - - # SSL_CTX_set0_chain() is needed for setting multiple certificate chains - add0_chain_supported = openssl?(1, 0, 2) - - if add0_chain_supported - ca2_key = Fixtures.pkey("rsa2048") - ca2_exts = [ - ["basicConstraints", "CA:TRUE", true], - ["keyUsage", "cRLSign, keyCertSign", true], - ] - ca2_dn = OpenSSL::X509::Name.parse_rfc2253("CN=CA2") - ca2_cert = issue_cert(ca2_dn, ca2_key, 123, ca2_exts, nil, nil) - else - # Use the same CA as @svr_cert - ca2_key = @ca_key; ca2_cert = @ca_cert - end - - ecdsa_key = Fixtures.pkey("p256") - exts = [ - ["keyUsage", "digitalSignature", false], - ] - ecdsa_dn = OpenSSL::X509::Name.parse_rfc2253("CN=localhost2") - ecdsa_cert = issue_cert(ecdsa_dn, ecdsa_key, 456, exts, ca2_cert, ca2_key) - - # Create chain certificates file - certs1 = Tempfile.open { |f| f << @svr_cert.to_pem << @ca_cert.to_pem; f } - pkey1 = Tempfile.open { |f| f << @svr_key.to_pem; f } - certs2 = Tempfile.open { |f| f << ecdsa_cert.to_pem << ca2_cert.to_pem; f } - pkey2 = Tempfile.open { |f| f << ecdsa_key.to_pem; f } - - ctx_proc = -> ctx { - # Unset values set by start_server - ctx.cert = ctx.key = ctx.extra_chain_cert = nil - ctx.ecdh_curves = "P-256" unless openssl?(1, 0, 2) - assert_nothing_raised { - ctx.add_certificate_chain_file(certs1.path, pkey1.path) # RSA - ctx.add_certificate_chain_file(certs2.path, pkey2.path) # ECDSA - } - } - - start_server(ctx_proc: ctx_proc) do |port| - ctx = OpenSSL::SSL::SSLContext.new - ctx.max_version = :TLS1_2 - ctx.ciphers = "aRSA" - server_connect(port, ctx) { |ssl| - assert_equal @svr_cert.subject, ssl.peer_cert.subject - assert_equal [@svr_cert.subject, @ca_cert.subject], - ssl.peer_cert_chain.map(&:subject) - - ssl.puts "abc"; assert_equal "abc\n", ssl.gets - } - - ctx = OpenSSL::SSL::SSLContext.new - ctx.max_version = :TLS1_2 - ctx.ciphers = "aECDSA" - server_connect(port, ctx) { |ssl| - assert_equal ecdsa_cert.subject, ssl.peer_cert.subject - assert_equal [ecdsa_cert.subject, ca2_cert.subject], - ssl.peer_cert_chain.map(&:subject) - - ssl.puts "123"; assert_equal "123\n", ssl.gets - } - end - ensure - certs1&.close - pkey1&.close - certs1&.unlink - pkey1&.unlink - certs2&.close - pkey2&.close - certs2&.unlink - pkey2&.unlink + ctx = OpenSSL::SSL::SSLContext.new + assert ctx.add_certificate_chain_file(Fixtures.file_path("chain", "server.crt")) end def test_sysread_and_syswrite -- cgit v1.2.3