From c69531ca1a82c840f186fd0b16c588b6de88043f Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sat, 19 Oct 2024 16:37:25 +0100 Subject: [PATCH 1/4] store and storecontext: check whether object is frozen before executing state change functions this makes them mostly useless, considering how they're used standalone --- ext/openssl/ossl_x509store.c | 15 +++++++++++++++ test/openssl/test_x509store.rb | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 670519feb..9548e5d68 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -246,6 +246,7 @@ static VALUE ossl_x509store_set_flags(VALUE self, VALUE flags) { X509_STORE *store; + rb_check_frozen(self); long f = NUM2LONG(flags); GetX509Store(self, store); @@ -281,6 +282,7 @@ static VALUE ossl_x509store_set_purpose(VALUE self, VALUE purpose) { X509_STORE *store; + rb_check_frozen(self); int p = NUM2INT(purpose); GetX509Store(self, store); @@ -305,6 +307,7 @@ static VALUE ossl_x509store_set_trust(VALUE self, VALUE trust) { X509_STORE *store; + rb_check_frozen(self); int t = NUM2INT(trust); GetX509Store(self, store); @@ -331,6 +334,7 @@ ossl_x509store_set_time(VALUE self, VALUE time) X509_STORE *store; X509_VERIFY_PARAM *param; + rb_check_frozen(self); GetX509Store(self, store); #ifdef HAVE_X509_STORE_GET0_PARAM param = X509_STORE_get0_param(store); @@ -358,6 +362,7 @@ ossl_x509store_add_file(VALUE self, VALUE file) X509_LOOKUP *lookup; const char *path; + rb_check_frozen(self); GetX509Store(self, store); path = StringValueCStr(file); lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()); @@ -393,6 +398,7 @@ ossl_x509store_add_path(VALUE self, VALUE dir) X509_LOOKUP *lookup; const char *path; + rb_check_frozen(self); GetX509Store(self, store); path = StringValueCStr(dir); lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir()); @@ -422,6 +428,7 @@ ossl_x509store_set_default_paths(VALUE self) { X509_STORE *store; + rb_check_frozen(self); GetX509Store(self, store); if (X509_STORE_set_default_paths(store) != 1) ossl_raise(eX509StoreError, "X509_STORE_set_default_paths"); @@ -443,6 +450,7 @@ ossl_x509store_add_cert(VALUE self, VALUE arg) X509_STORE *store; X509 *cert; + rb_check_frozen(self); cert = GetX509CertPtr(arg); /* NO NEED TO DUP */ GetX509Store(self, store); if (X509_STORE_add_cert(store, cert) != 1) @@ -465,6 +473,7 @@ ossl_x509store_add_crl(VALUE self, VALUE arg) X509_STORE *store; X509_CRL *crl; + rb_check_frozen(self); crl = GetX509CRLPtr(arg); /* NO NEED TO DUP */ GetX509Store(self, store); if (X509_STORE_add_crl(store, crl) != 1) @@ -498,6 +507,7 @@ ossl_x509store_verify(int argc, VALUE *argv, VALUE self) VALUE cert, chain; VALUE ctx, proc, result; + rb_check_frozen(self); rb_scan_args(argc, argv, "11", &cert, &chain); ctx = rb_funcall(cX509StoreContext, rb_intern("new"), 3, self, cert, chain); proc = rb_block_given_p() ? rb_block_proc() : @@ -695,6 +705,7 @@ ossl_x509stctx_set_error(VALUE self, VALUE err) { X509_STORE_CTX *ctx; + rb_check_frozen(self); GetX509StCtx(self, ctx); X509_STORE_CTX_set_error(ctx, NUM2INT(err)); @@ -793,6 +804,7 @@ static VALUE ossl_x509stctx_set_flags(VALUE self, VALUE flags) { X509_STORE_CTX *store; + rb_check_frozen(self); long f = NUM2LONG(flags); GetX509StCtx(self, store); @@ -814,6 +826,7 @@ static VALUE ossl_x509stctx_set_purpose(VALUE self, VALUE purpose) { X509_STORE_CTX *store; + rb_check_frozen(self); int p = NUM2INT(purpose); GetX509StCtx(self, store); @@ -835,6 +848,7 @@ static VALUE ossl_x509stctx_set_trust(VALUE self, VALUE trust) { X509_STORE_CTX *store; + rb_check_frozen(self); int t = NUM2INT(trust); GetX509StCtx(self, store); @@ -857,6 +871,7 @@ ossl_x509stctx_set_time(VALUE self, VALUE time) X509_STORE_CTX *store; long t; + rb_check_frozen(self); t = NUM2LONG(rb_Integer(time)); GetX509StCtx(self, store); X509_STORE_CTX_set_time(store, 0, t); diff --git a/test/openssl/test_x509store.rb b/test/openssl/test_x509store.rb index d6c0e707a..06761de91 100644 --- a/test/openssl/test_x509store.rb +++ b/test/openssl/test_x509store.rb @@ -91,6 +91,13 @@ def test_verify_simple assert_match(/ok/i, store.error_string) assert_equal(OpenSSL::X509::V_OK, store.error) assert_equal([ee1_cert, ca2_cert, ca1_cert], store.chain) + + # frozen, operation invalid + store = OpenSSL::X509::Store.new + store.freeze + assert_raise(FrozenError) do + store.verify(ee1_cert, [ca2_cert, ca1_cert]) + end end def test_verify_callback From d89f28c9bfbc708352ffe57e29d001e8014aee8a Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sat, 19 Oct 2024 16:39:19 +0100 Subject: [PATCH 2/4] store and store context: shareable when frozen --- ext/openssl/ossl.h | 1 - ext/openssl/ossl_x509store.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index dde2c54f0..36a6eef36 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -22,7 +22,6 @@ #else #define RUBY_TYPED_FROZEN_SHAREABLE 0 #endif - #include #include diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 9548e5d68..bc19cc53c 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -133,7 +133,7 @@ static const rb_data_type_t ossl_x509store_type = { { ossl_x509store_mark, ossl_x509store_free, }, - 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED, + 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_FROZEN_SHAREABLE, }; /* From b1c44db76a304fc172e53254b7b88ce9fe9e1512 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sat, 19 Oct 2024 16:55:19 +0100 Subject: [PATCH 3/4] make DEFAULT_CERT_STORE frozen and shareable --- lib/openssl/ssl.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index 2186f5f43..1e1080408 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -90,9 +90,11 @@ class SSLContext ) end - DEFAULT_CERT_STORE = OpenSSL::X509::Store.new # :nodoc: - DEFAULT_CERT_STORE.set_default_paths - DEFAULT_CERT_STORE.flags = OpenSSL::X509::V_FLAG_CRL_CHECK_ALL + DEFAULT_CERT_STORE = OpenSSL::X509::Store.new.tap do |store| # :nodoc: + store.set_default_paths + store.flags = OpenSSL::X509::V_FLAG_CRL_CHECK_ALL + store.freeze + end # A callback invoked when DH parameters are required for ephemeral DH key # exchange. From 9bcc63062be506e4a587c74118cb94ac58d4ff57 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 31 Oct 2024 18:19:55 +0000 Subject: [PATCH 4/4] POC for certificates/CRLs --- ext/openssl/ossl_x509store.c | 18 ++++++++++++++++++ lib/openssl/x509.rb | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index bc19cc53c..b3c517a44 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -224,6 +224,10 @@ ossl_x509store_initialize(int argc, VALUE *argv, VALUE self) rb_iv_set(self, "@error_string", Qnil); rb_iv_set(self, "@chain", Qnil); + /* added certificate/CRL references */ + rb_iv_set(self, "@certificates", rb_ary_new()); + rb_iv_set(self, "@crls", rb_ary_new()); + return self; } @@ -449,13 +453,20 @@ ossl_x509store_add_cert(VALUE self, VALUE arg) { X509_STORE *store; X509 *cert; + VALUE certificates; rb_check_frozen(self); + cert = GetX509CertPtr(arg); /* NO NEED TO DUP */ GetX509Store(self, store); if (X509_STORE_add_cert(store, cert) != 1) ossl_raise(eX509StoreError, "X509_STORE_add_cert"); + certificates = rb_iv_get(self, "@certificates"); + + if(!RTEST(rb_funcall(certificates, rb_intern("include?"), 1, arg))) + rb_ary_push(certificates, arg); + return self; } @@ -472,13 +483,20 @@ ossl_x509store_add_crl(VALUE self, VALUE arg) { X509_STORE *store; X509_CRL *crl; + VALUE crls; rb_check_frozen(self); + crl = GetX509CRLPtr(arg); /* NO NEED TO DUP */ GetX509Store(self, store); if (X509_STORE_add_crl(store, crl) != 1) ossl_raise(eX509StoreError, "X509_STORE_add_crl"); + crls = rb_iv_get(self, "@crls"); + + if(!RTEST(rb_funcall(crls, rb_intern("include?"), 1, arg))) + rb_ary_push(crls, arg); + return self; } diff --git a/lib/openssl/x509.rb b/lib/openssl/x509.rb index b66727420..9ade2726f 100644 --- a/lib/openssl/x509.rb +++ b/lib/openssl/x509.rb @@ -333,6 +333,14 @@ def ==(other) end end + class Store + def freeze + super + @certificates.each(&:freeze) + @crls.each(&:freeze) + end + end + class StoreContext def cleanup warn "(#{caller.first}) OpenSSL::X509::StoreContext#cleanup is deprecated with no replacement" if $VERBOSE