Feature #2022 [ruby-core:25210]

Patch for openssl-1.0

Added by Jeroen van Meeuwen 195 days ago. Updated 14 days ago.

Status :Closed Start :08/31/2009
Priority :Normal Due date :
Assigned to :Evan Phoenix % Done :

100%

Category :ext
Target version :1.9.2

Description

Attached is a patch for ruby-1.8.6 to enable it to compile with and use openssl-1.0

ruby-openssl-1.0.patch - Patch for openssl-1.0 (5.3 KB) Jeroen van Meeuwen, 08/31/2009 08:31 AM

0001-Apply-Jeroen-van-Meeuwen-s-and-Nobu-Nakada-s-OpenSSL.patch (5.6 KB) Hongli Lai, 01/05/2010 07:05 PM

0002-Fix-some-various-OpenSSL-compilation-and-runtime-war.patch (7.6 KB) Hongli Lai, 01/05/2010 07:05 PM

0003-Fix-some-OpenSSL-extension-unit-tests-for-OpenSSL-1..patch (6 KB) Hongli Lai, 01/05/2010 07:05 PM

openssl-build-fix-v2.patch (6 KB) Motohiro KOSAKI, 02/05/2010 10:56 PM

0002-openssl-verify-don-t-assume-false.patch (6.1 KB) Motohiro KOSAKI, 02/05/2010 10:56 PM

0003-test-openssl-drop-the-test-of-assuming-too-cool-guess.patch (1.9 KB) Motohiro KOSAKI, 02/05/2010 10:56 PM


Related issues

related to Bug #2604 test_dsa_sign_verify() in test/openssl/test_ec.rb failes Open 01/14/2010
duplicates Bug #2536 openssl fails to compile Closed 12/28/2009

Associated revisions

Revision 26781
Added by naruse 14 days ago

  • openssl/ossl.c (OSSL_IMPL_SK2ARY): for OpenSSL 1.0. patched by Jeroen van Meeuwen at [ruby-core:25210] fixed by Nobuyoshi Nakada [ruby-core:25238], Hongli Lai [ruby-core:27417], and Motohiro KOSAKI [ruby-core:28063]
  • ext/openssl/ossl_ssl.c (ossl_ssl_method_tab), (ossl_ssl_cipher_to_ary): constified.
  • ext/openssl/ossl_pkcs7.c (pkcs7_get_certs, pkcs7_get_crls): split pkcs7_get_certs_or_crls.

History

08/31/2009 08:55 AM - Kirk Haines

  • Assigned to set to Kirk Haines
Thanks. I will take a look at it very soon.

08/31/2009 11:42 AM - Nobuyoshi Nakada

Hi,

At Mon, 31 Aug 2009 08:32:19 +0900,
Jeroen van Meeuwen wrote in [ruby-core:25210]:
> Attached is a patch for ruby-1.8.6 to enable it to compile
> with and use openssl-1.0

Other versions have no problem?
And it can compile with earlier openssl?

-- 
Nobu Nakada

09/01/2009 03:46 PM - Nobuyoshi Nakada

Hi,

At Mon, 31 Aug 2009 08:32:19 +0900,
Jeroen van Meeuwen wrote in [ruby-core:25210]:
> Attached is a patch for ruby-1.8.6 to enable it to compile
> with and use openssl-1.0

It could compile with openssl 0.9.8k, though one hunk was
rejected in trunk.

This patch is against the trunk and can be applied to 1.9.1,
1.8 and 1.8.7.


* ext/openssl/ossl.c (OSSL_IMPL_SK2ARY): for OpenSSL 1.0.  based
  on a patch from Jeroen van Meeuwen at [ruby-core:25210]

* ext/openssl/ossl_ssl.c (ossl_ssl_method_tab),
  (ossl_ssl_cipher_to_ary): constified.

* ext/openssl/ossl_pkcs7.c (pkcs7_get_certs, pkcs7_get_crls):
  split pkcs7_get_certs_or_crls.


Index: ext/openssl/ossl.c
===================================================================
--- ext/openssl/ossl.c	(revision 24721)
+++ ext/openssl/ossl.c	(working copy)
@@ -93,5 +93,5 @@ ossl_x509_ary2sk(VALUE ary)
 #define OSSL_IMPL_SK2ARY(name, type)	        \
 VALUE						\
-ossl_##name##_sk2ary(STACK *sk)			\
+ossl_##name##_sk2ary(STACK_OF(type) *sk)	\
 {						\
     type *t;					\
@@ -103,5 +103,5 @@ ossl_##name##_sk2ary(STACK *sk)			\
 	return Qnil;				\
     }						\
-    num = sk_num(sk);				\
+    num = sk_##type##_num(sk);			\
     if (num < 0) {				\
 	OSSL_Debug("items in sk < -1???");	\
@@ -111,5 +111,5 @@ ossl_##name##_sk2ary(STACK *sk)			\
 						\
     for (i=0; i<num; i++) {			\
-	t = (type *)sk_value(sk, i);		\
+	t = sk_##type##_value(sk, i);		\
 	rb_ary_push(ary, ossl_##name##_new(t));	\
     }						\
Index: ext/openssl/ossl_pkcs7.c
===================================================================
--- ext/openssl/ossl_pkcs7.c	(revision 24721)
+++ ext/openssl/ossl_pkcs7.c	(working copy)
@@ -573,10 +573,9 @@ ossl_pkcs7_add_certificate(VALUE self, V
 }
 
-static STACK *
-pkcs7_get_certs_or_crls(VALUE self, int want_certs)
+static STACK_OF(X509) *
+pkcs7_get_certs(VALUE self)
 {
     PKCS7 *pkcs7;
     STACK_OF(X509) *certs;
-    STACK_OF(X509_CRL) *crls;
     int i;
 
@@ -586,15 +585,36 @@ pkcs7_get_certs_or_crls(VALUE self, int 
     case NID_pkcs7_signed:
         certs = pkcs7->d.sign->cert;
-        crls = pkcs7->d.sign->crl;
         break;
     case NID_pkcs7_signedAndEnveloped:
         certs = pkcs7->d.signed_and_enveloped->cert;
+        break;
+    default:
+        certs = NULL;
+    }
+
+    return certs;
+}
+
+static STACK_OF(X509_CRL) *
+pkcs7_get_crls(VALUE self)
+{
+    PKCS7 *pkcs7;
+    STACK_OF(X509_CRL) *crls;
+    int i;
+
+    GetPKCS7(self, pkcs7);
+    i = OBJ_obj2nid(pkcs7->type);
+    switch(i){
+    case NID_pkcs7_signed:
+        crls = pkcs7->d.sign->crl;
+        break;
+    case NID_pkcs7_signedAndEnveloped:
         crls = pkcs7->d.signed_and_enveloped->crl;
         break;
     default:
-        certs = crls = NULL;
+        crls = NULL;
     }
 
-    return want_certs ? certs : crls;
+    return crls;
 }
 
@@ -611,5 +631,5 @@ ossl_pkcs7_set_certificates(VALUE self, 
     X509 *cert;
 
-    certs = pkcs7_get_certs_or_crls(self, 1);
+    certs = pkcs7_get_certs(self);
     while((cert = sk_X509_pop(certs))) X509_free(cert);
     rb_block_call(ary, rb_intern("each"), 0, 0, ossl_pkcs7_set_certs_i, self);
@@ -621,5 +641,5 @@ static VALUE
 ossl_pkcs7_get_certificates(VALUE self)
 {
-    return ossl_x509_sk2ary(pkcs7_get_certs_or_crls(self, 1));
+    return ossl_x509_sk2ary(pkcs7_get_certs(self));
 }
 
@@ -651,5 +671,5 @@ ossl_pkcs7_set_crls(VALUE self, VALUE ar
     X509_CRL *crl;
 
-    crls = pkcs7_get_certs_or_crls(self, 0);
+    crls = pkcs7_get_crls(self);
     while((crl = sk_X509_CRL_pop(crls))) X509_CRL_free(crl);
     rb_block_call(ary, rb_intern("each"), 0, 0, ossl_pkcs7_set_crls_i, self);
@@ -661,5 +681,5 @@ static VALUE
 ossl_pkcs7_get_crls(VALUE self)
 {
-    return ossl_x509crl_sk2ary(pkcs7_get_certs_or_crls(self, 0));
+    return ossl_x509crl_sk2ary(pkcs7_get_crls(self));
 }
 
Index: ext/openssl/ossl_ssl.c
===================================================================
--- ext/openssl/ossl_ssl.c	(revision 24721)
+++ ext/openssl/ossl_ssl.c	(working copy)
@@ -97,4 +97,10 @@ static const char *ossl_ssl_attrs[] = {
 ID ID_callback_state;
 
+#if OPENSSL_VERSION_NUMBER >= 0x10000000L
+#define OSSL_MORE_CONST const
+#define STACK _STACK
+#else
+#define OSSL_MORE_CONST
+#endif
 /*
  * SSLContext class
@@ -102,5 +108,5 @@ ID ID_callback_state;
 struct {
     const char *name;
-    SSL_METHOD *(*func)(void);
+    OSSL_MORE_CONST SSL_METHOD *(*func)(void);
 } ossl_ssl_method_tab[] = {
 #define OSSL_SSL_METHOD_ENTRY(name) { #name, name##_method }
@@ -151,5 +157,5 @@ static VALUE
 ossl_sslctx_set_ssl_version(VALUE self, VALUE ssl_method)
 {
-    SSL_METHOD *method = NULL;
+    OSSL_MORE_CONST SSL_METHOD *method = NULL;
     const char *s;
     int i;
@@ -663,5 +669,5 @@ ossl_sslctx_setup(VALUE self)
 
 static VALUE
-ossl_ssl_cipher_to_ary(SSL_CIPHER *cipher)
+ossl_ssl_cipher_to_ary(OSSL_MORE_CONST SSL_CIPHER *cipher)
 {
     VALUE ary;
@@ -1419,5 +1425,5 @@ ossl_ssl_get_cipher(VALUE self)
 {
     SSL *ssl;
-    SSL_CIPHER *cipher;
+    OSSL_MORE_CONST SSL_CIPHER *cipher;
 
     Data_Get_Struct(self, SSL, ssl);


-- 
Nobu Nakada

09/29/2009 11:37 PM - Nikolai Lugovoi

Though patch applies well for compilation, it results in some random segfaults with openssl-1.0beta3, from test/openssl/test_ssl.rb, when SSLContext is garbage collected before referencing SSLSocket - in all ruby versions - 1.8.6, 1.8.7 and 1.9.2dev

As workaround, I tried to check references count in ossl_ssl.c:

 static void
 ossl_sslctx_free(SSL_CTX *ctx)
 {
+    if(ctx && ctx->references > 1) return;
     if(ctx && SSL_CTX_get_ex_data(ctx, ossl_ssl_ex_store_p)== (void*)1)
        ctx->cert_store = NULL;
     SSL_CTX_free(ctx)

that seemed to eliminate segfaults, but not sure if it does not introduce memory leaks.

10/28/2009 02:28 AM - Yui NARUSE

  • Subject changed from Patch for ruby-1.8.6 and openssl-1.0 to Patch for openssl-1.0

11/26/2009 10:52 AM - Yui NARUSE

  • Status changed from Open to Assigned
What's current status?
Fedora 12 uses openssl 1.0 Beta 3; So Fedora people can't build ruby now.

01/05/2010 07:05 PM - Hongli Lai

I verify that Nobuyoshi Nakada's and Jeroen van Meeuwen's patch works on both OpenSSL 0.9 and 1.0. I've tested on the following systems:

- OS X Snow Leopard
  All Ruby OpenSSL tests pass, and all RubySpec OpenSSL tests pass.

- Fedora 12, x86_64
  There are some compilation warnings. All RubySpec OpenSSL tests pass, but some Ruby OpenSSL tests fail. It seems that OpenSSL changed somewhat in behavior; for example some #verify methods now raise an error instead of returning false.
  The attached patches fixes some compilation warnings and fixes some unit tests.

Following tests still fail on Fedora 12:

    1) Failure:
  test_sign_and_verify(OpenSSL::TestX509Certificate) [./openssl/test_x509cert.rb:168]:
  <OpenSSL::X509::CertificateError> exception expected but none was thrown.
  
    2) Failure:
  test_sign_and_verify(OpenSSL::TestX509Request) [./openssl/test_x509req.rb:133]:
  <OpenSSL::X509::RequestError> exception expected but none was thrown.


I am not able to reproduce Nikolai Lugovoi's segfaults. According to the man page for ssl_cx_free (http://linux.die.net/man/3/ssl_ctx_free) that function only frees the context if the reference count drops to 0. There should be no need to check ctx->references.

01/05/2010 07:06 PM - Hongli Lai

Those patches are against 1.8.7-p248.

01/05/2010 07:33 PM - Yui NARUSE

  • Assigned to changed from Kirk Haines to Evan Phoenix

01/08/2010 06:36 PM - Nobuyoshi Nakada

Hi,

At Tue, 5 Jan 2010 19:05:54 +0900,
Hongli Lai wrote in [ruby-core:27417]:
>   There are some compilation warnings. All RubySpec OpenSSL
>   tests pass, but some Ruby OpenSSL tests fail. It seems that
>   OpenSSL changed somewhat in behavior; for example some
>   #verify methods now raise an error instead of returning
>   false.
>   The attached patches fixes some compilation warnings and
>   fixes some unit tests.

-    rconf = rb_iv_get(self, "@config");
+    i_config = rb_intern("@config");
+    if (rb_ivar_defined(self, i_config))
+	rconf = rb_ivar_get(self, i_config);
+    else
+	rconf = Qnil;
     conf = NIL_P(rconf) ? NULL : GetConfigPtr(rconf);
     ext = X509V3_EXT_nconf_nid(conf, ctx, nid, RSTRING_PTR(valstr));
 #else

rb_attr_get() doesn't yield "not initialized" warning.

-- 
Nobu Nakada

02/05/2010 10:56 PM - Motohiro KOSAKI

This issue still occur.
Now, Trunk + Fedora12 on x86_64 makes following error.

Then, I did forwardport Lai's patch to current trunk.

openssl-build-fix-v2.patch mean
    0001-Apply-Jeroen-van-Meeuwen-s-and-Nobu-Nakada-s-OpenSSL.patch
  + 0002-Fix-some-various-OpenSSL-compilation-and-runtime-war.patch
  - ugly OSSL_MORE_CONST macro
  - unnecessary hunk of comment#12

 This patch fixes the build error.

0002-openssl-verify-don-t-assume-false.patch mean
    simply forward port of 0003-Fix-some-OpenSSL-extension-unit-tests-for-OpenSSL-1..patch

 This patch fixes following false positive test failure.

  4) Error:
test_sign_and_verify(OpenSSL::TestX509CRL):
OpenSSL::X509::CRLError: wrong public key type
    /home/kosaki/linux/ruby/test/openssl/test_x509crl.rb:200:in `verify'
    /home/kosaki/linux/ruby/test/openssl/test_x509crl.rb:200:in `test_sign_and_verify'

  5) Error:
test_sign_and_verify(OpenSSL::TestX509Certificate):
OpenSSL::X509::CertificateError: wrong public key type
    /home/kosaki/linux/ruby/test/openssl/test_x509cert.rb:137:in `verify'
    /home/kosaki/linux/ruby/test/openssl/test_x509cert.rb:137:in `test_sign_and_verify'

  6) Error:
test_sign_and_verify(OpenSSL::TestX509Request):
OpenSSL::X509::RequestError: wrong public key type
    /home/kosaki/linux/ruby/test/openssl/test_x509req.rb:110:in `verify'
    /home/kosaki/linux/ruby/test/openssl/test_x509req.rb:110:in `test_sign_and_verify'

0003-test-openssl-drop-the-test-of-assuming-too-cool-guess.patch is new patch. It remove two following false positive warnings.

  4) Failure:
test_sign_and_verify(OpenSSL::TestX509Certificate) [/home/kosaki/linux/ruby/test/openssl/test_x509cert.rb:169]:
OpenSSL::X509::CertificateError expected but nothing was raised.

  5) Failure:
test_sign_and_verify(OpenSSL::TestX509Request) [/home/kosaki/linux/ruby/test/openssl/test_x509req.rb:133]:
OpenSSL::X509::RequestError expected but nothing was raised.

02/09/2010 04:03 AM - Yui NARUSE

  • Category set to ext
  • Target version set to 1.9.2
KOSAKI's patch looks good.
We may be able to commit fixes for tests.

02/28/2010 11:57 AM - Yui NARUSE

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100
This issue was solved with changeset r26781.
Jeroen, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

03/05/2010 11:40 PM - Hiroshi NAKAMURA

On Tue, Feb 9, 2010 at 04:03, Yui NARUSE <redmine@ruby-lang.org> wrote:
> Issue #2022 has been updated by Yui NARUSE.
>
> Category set to ext
> Target version set to 1.9.2
>
> KOSAKI's patch looks good.
> We may be able to commit fixes for tests.

Here's a topic branch for ruby_1_8(1.8.8dev).
http://github.com/nahi/ruby/tree/openssl_19_bump

Guys, would you please try the branch and let me know how it works for
your env?  It should support 1.0.0b5, 0.9.8m and earlier versions.

I'll cleanup commits and merge it to ruby_1_8 in a few days. Trunk
should follow it.

Regards,
// NaHi

03/07/2010 07:19 AM - Hiroshi NAKAMURA

On Fri, Mar 5, 2010 at 23:40, NAKAMURA, Hiroshi <nakahiro@gmail.com> wrote:
> I'll cleanup commits and merge it to ruby_1_8 in a few days. Trunk
> should follow it.

Merged to ruby_1_8. Following commits should be applied to the trunk.
http://redmine.ruby-lang.org/repositories/revision/ruby-18?rev=26837
http://redmine.ruby-lang.org/repositories/revision/ruby-18?rev=26839
http://redmine.ruby-lang.org/repositories/revision/ruby-18?rev=26840

Regards,
// NaHi

Also available in: Atom PDF