From 1ae73170c8cc19a9bde5884a5cfe1e76bc0dc07c Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Tue, 31 Dec 2019 20:43:34 +0900 Subject: [PATCH 1/8] add pkey_path argument to ossl_sslctx_add_certificate_chain_file() --- ext/openssl/ossl_ssl.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 3b5ff5a1f..8a73decce 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1322,15 +1322,45 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) return self; } +/* + * call-seq: + * ctx.add_certificate_chain_file(certs_path, pkey_path) -> true | false + * + * Loads (chain) certificate(s) from _certs_path_ and private key from + * _pkey_path_. + * + * === Parameters + * _certs_path_:: + * A path to a (chain) certificate(s) file. A instance of String. + * _pkey_path_:: + * A path to a private key file. A instance of String. + * + * === Note + * The file format of certificate and private key must be PEM. + * + * The certificate file must be starting with the subject's certificate and + * followed by intermediate CA certificates (and root CA certificate). + */ static VALUE -ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE path) +ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_path) { - StringValue(path); - SSL_CTX *ctx = NULL; + SSL_CTX *ctx; GetSSLCTX(self, ctx); + if (NIL_P(certs_path)) + ossl_raise(rb_eArgError, "certs_path must be the path to certificates"); + + if (NIL_P(pkey_path)) + ossl_raise(rb_eArgError, "pkey_path must be the path to private key"); - return SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(path)) == 1 ? Qtrue : Qfalse; + /* SSL_CTX_use_certificate_chain_file() loads PEM format file. */ + if (SSL_CTX_use_certificate_chain_file(ctx, StringValueCStr(certs_path)) != 1) + return Qfalse; + + if (SSL_CTX_use_PrivateKey_file(ctx, StringValueCStr(pkey_path), SSL_FILETYPE_PEM) != 1) + return Qfalse; + + return Qtrue; } /* @@ -2784,7 +2814,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, 1); + rb_define_method(cSSLContext, "add_certificate_chain_file", ossl_sslctx_add_certificate_chain_file, 2); rb_define_method(cSSLContext, "setup", ossl_sslctx_setup, 0); rb_define_alias(cSSLContext, "freeze", "setup"); From 3d3847a4971b88d7c14c1f048d5f28dc4949350d Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Tue, 31 Dec 2019 21:12:34 +0900 Subject: [PATCH 2/8] modify test_add_certificate_chain_file to check ssl.peer_cert and ssl.peer_cert_chain --- ext/openssl/ossl_ssl.c | 9 +++++---- test/test_ssl.rb | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 8a73decce..4ee698cd8 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1326,17 +1326,18 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) * call-seq: * ctx.add_certificate_chain_file(certs_path, pkey_path) -> true | false * - * Loads (chain) certificate(s) from _certs_path_ and private key from + * Loads chain certificates from _certs_path_ and a private key from * _pkey_path_. * * === Parameters * _certs_path_:: - * A path to a (chain) certificate(s) file. A instance of String. + * 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. A instance of String. + * A path to a private key file. An instance of String. * * === Note - * The file format of certificate and private key must be PEM. + * 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 certificates (and root CA certificate). diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 5d4c09b87..074847699 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -186,8 +186,29 @@ def test_add_certificate_multiple_certs end def test_add_certificate_chain_file - ctx = OpenSSL::SSL::SSLContext.new - assert ctx.add_certificate_chain_file(Fixtures.file_path("chain", "server.crt")) + # Create chain certificates file + GC.disable # for tempfile + 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 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 end def test_sysread_and_syswrite From d742c888ea2b9b7fe373d3d99e1cd94bd5bbe182 Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Tue, 31 Dec 2019 21:48:52 +0900 Subject: [PATCH 3/8] modify ossl_sslctx_add_certificate_chain_file() to raise Error and to return self add test_add_certificate_chain_file_multiple_certs --- ext/openssl/ossl_ssl.c | 23 +++++++------ test/test_ssl.rb | 76 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 4ee698cd8..ee56edc04 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1324,7 +1324,7 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) /* * call-seq: - * ctx.add_certificate_chain_file(certs_path, pkey_path) -> true | false + * ctx.add_certificate_chain_file(certs_path, pkey_path) -> self * * Loads chain certificates from _certs_path_ and a private key from * _pkey_path_. @@ -1336,11 +1336,19 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) * _pkey_path_:: * A path to a private key file. An instance of String. * + * === Example + * ctx.add_certificate_chain(rsa_cert_path, rsa_key_path) + * + * ctx.add_certificate_chain(ecdsa_cert_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 certificates (and root CA certificate). + * 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) @@ -1348,20 +1356,15 @@ ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_ SSL_CTX *ctx; GetSSLCTX(self, ctx); - if (NIL_P(certs_path)) - ossl_raise(rb_eArgError, "certs_path must be the path to certificates"); - - if (NIL_P(pkey_path)) - ossl_raise(rb_eArgError, "pkey_path must be the path to private key"); /* SSL_CTX_use_certificate_chain_file() loads PEM format file. */ if (SSL_CTX_use_certificate_chain_file(ctx, StringValueCStr(certs_path)) != 1) - return Qfalse; + ossl_raise(eSSLError, "SSL_CTX_use_certificate_chain_file"); if (SSL_CTX_use_PrivateKey_file(ctx, StringValueCStr(pkey_path), SSL_FILETYPE_PEM) != 1) - return Qfalse; + ossl_raise(eSSLError, "SSL_CTX_use_PrivateKey_file"); - return Qtrue; + return self; } /* diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 074847699..cc89ab6ec 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -194,7 +194,7 @@ def test_add_certificate_chain_file ctx_proc = -> ctx { # Unset values set by start_server ctx.cert = ctx.key = ctx.extra_chain_cert = nil - assert ctx.add_certificate_chain_file(certs.path, pkey.path) + assert_nothing_raised { ctx.add_certificate_chain_file(certs.path, pkey.path) } } start_server(ctx_proc: ctx_proc) { |port| @@ -211,6 +211,80 @@ def test_add_certificate_chain_file pkey&.close 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 + GC.disable # for tempfile + 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 + certs2&.close + pkey2&.close + end + def test_sysread_and_syswrite start_server { |port| server_connect(port) { |ssl| From 03813b725b475194baf56c4bd71c0dcb7bfb4764 Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Wed, 1 Jan 2020 09:33:53 +0900 Subject: [PATCH 4/8] check with EVP_PKEY_cmp in advance --- ext/openssl/ossl_ssl.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index ee56edc04..d1eb977e4 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1354,14 +1354,45 @@ static VALUE ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_path) { SSL_CTX *ctx; + X509 *x509; + char *ccerts_path, *cpkey_path; + FILE *fp; + EVP_PKEY *pkey, *pub_pkey; 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) + rb_raise(rb_eArgError, "failed to open certs file"); + x509 = PEM_read_X509(fp, NULL, 0, NULL); + fclose(fp); + if (!x509) + rb_raise(rb_eArgError, "failed to open certs file"); + pub_pkey = X509_get_pubkey(x509); + /* The reference counter is bumped, and decremented immediately. */ + EVP_PKEY_free(pub_pkey); + if (!pub_pkey) + rb_raise(rb_eArgError, "certificate does not contain public key"); + + if (EVP_PKEY_cmp(pub_pkey, pkey) != 1) + rb_raise(rb_eArgError, "public key mismatch"); + /* SSL_CTX_use_certificate_chain_file() loads PEM format file. */ - if (SSL_CTX_use_certificate_chain_file(ctx, StringValueCStr(certs_path)) != 1) + 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, StringValueCStr(pkey_path), SSL_FILETYPE_PEM) != 1) + if (SSL_CTX_use_PrivateKey_file(ctx, cpkey_path, SSL_FILETYPE_PEM) != 1) ossl_raise(eSSLError, "SSL_CTX_use_PrivateKey_file"); return self; From 04f61fa06b1267b3e2a4de455d84ff3c6b969fc4 Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Wed, 1 Jan 2020 10:07:00 +0900 Subject: [PATCH 5/8] add X509_free and EVP_PKEY_free --- ext/openssl/ossl_ssl.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index d1eb977e4..d1147ab42 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1370,23 +1370,35 @@ ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_ 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) + 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) + if (!x509) { + EVP_PKEY_free(pkey); rb_raise(rb_eArgError, "failed to open certs file"); + } pub_pkey = X509_get_pubkey(x509); - /* The reference counter is bumped, and decremented immediately. */ - EVP_PKEY_free(pub_pkey); - if (!pub_pkey) + 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) + } + 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) From 417e8cffa50241c9f0c89a40e4d4d2febd4340e3 Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Sun, 5 Jan 2020 01:43:40 +0900 Subject: [PATCH 6/8] rm GC.disable && add {certs,pkey}.unlink --- test/test_ssl.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/test_ssl.rb b/test/test_ssl.rb index cc89ab6ec..46be4df93 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -187,7 +187,6 @@ def test_add_certificate_multiple_certs def test_add_certificate_chain_file # Create chain certificates file - GC.disable # for tempfile certs = Tempfile.open { |f| f << @svr_cert.to_pem << @ca_cert.to_pem; f } pkey = Tempfile.open { |f| f << @svr_key.to_pem; f } @@ -209,6 +208,8 @@ def test_add_certificate_chain_file ensure certs&.close pkey&.close + certs&.unlink + pkey&.unlink end def test_add_certificate_chain_file_multiple_certs @@ -239,7 +240,6 @@ def test_add_certificate_chain_file_multiple_certs ecdsa_cert = issue_cert(ecdsa_dn, ecdsa_key, 456, exts, ca2_cert, ca2_key) # Create chain certificates file - GC.disable # for tempfile 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 } @@ -281,8 +281,12 @@ def test_add_certificate_chain_file_multiple_certs ensure certs1&.close pkey1&.close + certs1&.unlink + pkey1&.unlink certs2&.close pkey2&.close + certs2&.unlink + pkey2&.unlink end def test_sysread_and_syswrite From dd1dd7574680f1346d194257da5199afff8db561 Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Fri, 24 Jan 2020 19:55:15 +0900 Subject: [PATCH 7/8] fix comment; Example --- ext/openssl/ossl_ssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index d1147ab42..c631dec1f 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1337,9 +1337,9 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) * A path to a private key file. An instance of String. * * === Example - * ctx.add_certificate_chain(rsa_cert_path, rsa_key_path) + * ctx.add_certificate_chain(rsa_and_intermediate_certs_path, rsa_key_path) * - * ctx.add_certificate_chain(ecdsa_cert_path, ecdsa_key_path) + * ctx.add_certificate_chain(ecdsa_and_intermediate_certs_path, ecdsa_key_path) * * === Note * The file format of the certificate and private key must be PEM. From 0ca81d51675e2f01ac77d8435c8db3cf6baa59af Mon Sep 17 00:00:00 2001 From: thekuwayama Date: Fri, 24 Jan 2020 20:11:13 +0900 Subject: [PATCH 8/8] fix an incorrect method name --- ext/openssl/ossl_ssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index c631dec1f..484057cd4 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1337,9 +1337,9 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self) * A path to a private key file. An instance of String. * * === Example - * ctx.add_certificate_chain(rsa_and_intermediate_certs_path, rsa_key_path) + * ctx.add_certificate_chain_file(rsa_and_intermediate_certs_path, rsa_key_path) * - * ctx.add_certificate_chain(ecdsa_and_intermediate_certs_path, ecdsa_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.