Skip to content

Add post-quantum cryptography (PQC) support to SSLConfig - #53

Open
junaruga wants to merge 1 commit into
ruby:masterfrom
junaruga:wip/support-pqc
Open

Add post-quantum cryptography (PQC) support to SSLConfig#53
junaruga wants to merge 1 commit into
ruby:masterfrom
junaruga:wip/support-pqc

Conversation

@junaruga

@junaruga junaruga commented Jul 29, 2026

Copy link
Copy Markdown
Member

This PR is related to #52.

Proof of concept

I prepared proof-of-concept scripts for this PR.

The document (drb/README.md) is here. A list of the sections in the drb/README.md is below. The sections without "(development)" test with the current master branch. These work. The sections with "(development)" test with this PR.

$ grep ^## drb/README.md
## druby (non-SSL)
## drbssl (SSL) auto-generated RSA cert
## drbssl (SSL) pre-generated RSA cert
## drbssl (SSL) pre-generated ML-DSA-65 cert
## drbssl (SSL) pre-generated RSA cert with client cert
## drbssl (SSL) pre-generated ML-DSA-65 cert with client cert
## drbssl (SSL) auto-generated ML-DSA-65 cert (development)
## drbssl (SSL) auto-generated ML-DSA-65/RSA multi cert (development)
## drbssl (SSL) pre-generated ML-DSA-65/RSA multi cert (development)
## drbssl (SSL) pre-generated ML-DSA-65/RSA multi cert with client cert (development)

The script is here. The CI result is here.

Commit message

DRb::DRbSSLSocket::SSLConfig already works
with pre-generated ML-DSA cert/key by
:SSLCertificate and :SSLPrivateKey config options.

This commit adds the following features in PQC use cases to #setup_certificate and #setup_ssl_context.

  • Add :SSLCertificates config option accepting an Array of [certificate, private_key] pairs for dual/multiple certificate support via OpenSSL::SSL::SSLContext#add_certificate changing from OpenSSL::SSL::SSLContext#cert and #key. Because an SSL server that accepts clients with either ML-DSA-NN or RSA certificates is useful in the use case of PQC migration from RSA (non-PQC) to ML-DSA (PQC). :SSLCertificate and :SSLPrivateKey are available for backward compatibility. But use OpenSSL::SSL::SSLContext#add_certificate internally. This is a behavior change.
  • Add :SSLPrivateKeyAlgorithms option accepting an Array of key algorithms (RSA, ML-DSA-44, ML-DSA-65, ML-DSA-87) for multi-certificate generation, defaulting to ["RSA"] for backward compatibility.
  • Add :SSLGroups option to control key exchange group such as ML-KEM in PQC.
  • Add :SSLSignatureAlgorithms, :SSLClientSignatureAlgorithms to control signature algorithms such as ML-DSA-NN in PQC, and RSA in non-PQC.
  • Add tool/create_certs.sh to generate test/drb/fixtures/*.{crt,key} to test with pre-generated certs/keys and add DRbTests::Fixtures module to read the certs/keys.
  • Add test/drb/test_ssl.rb to test lib/drb/ssl.rb as an unit test level. This approach aligns with test/drb/test_acl.rb to test lib/drb/acl.rb as an unit test level.
  • Add TestDRbSSLPQC, TestDRbSSLPQCMultiCertMLDSA65, TestDRbSSLPQCMultiCertRSA in test/drb/test_drbssl.rb. TestDRbSSLPQC is to test single PQC ML-KEM/ML-DSA server via test/drb/ut_drb_drbssl_pqc.rb. The testing class is inspired by TestDRbSSLCore. TestDRbSSLPQCMultiCertMLDSA65 and TestDRbSSLPQCMultiCertRSA are to test ML-DSA-65 (PQC) and RSA (non-PQC) dual (multiple) certificate server with client certificate via test/drb/ut_drb_drbssl_pqc_multi_cert.rb. The test is designed for a high-security use case with :SSLVerifyMode OpenSSL::SSL::VERIFY_PEER | OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT.

Assisted-by: Claude:claude-opus-4-6[1m]

Notes

CI cases supporting PQC

PQC (ML-KEM/ML-DSA) works in OpenSSL >= 3.5. OpenSSL 3.5 added the feature. And works in Ruby OpenSSL >= 4.0. Ruby OpenSSL 4.0.0 added some functions in PQC use cases, and fixed a bug.

The .github/workflows/test.yml uses runner ubuntu-latest, macos-latest, windows-latest.

According to https://github.com/actions/runner-images, ubuntu-latest is ubuntu-24.04, macos-latest is macOS-26-arm64, windows-latest is windows-2025-vs2026.

The ubuntu-24.04 uses OpenSSL 3.0.13 which doesn't support PQC. The windows-2025-vs2026 uses OpenSSL 3.6.3 which supports PQC. The macos-26 uses OpenSSL 3.6.2 which supports PQC. The new PQC tests should be executed on the macos-latest and windows-latest CI cases.

test/drb/test_ssl.rb testing lib/drb/ssl.rb as an unit test level

I checked all the existing test/drb/test_*.rb files.

  • test/drb/test_acl.rb tests drb/acl.rb as an unit test level.
  • test/drb/test_drbobject.rb tests DRbObject class as an unit test level.
  • test/drb/test_drb.rb tests druby protocol as an integration test level.
  • test/drb/test_drbssl.rb tests drbssl protocol as an integration test level.
  • test/drb/test_drbunix.rb is tests drbunix protocol as an integration test level.

So, I created test/drb/test_ssl.rb to test DRb::DRbSSLSocket::SSLConfig in lib/drb/ssl.rb as an unit test level.

tool/create_certs.sh design decisions

The tool/create_certs.sh is inspired by ruby/rubygems#9678. The directory test/drb/fixtures to manage SSL key/cert files is inspired by ruby/openssl managing SSL keys in test/openssl/fixtures/pkey directory. ruby/openssl manages only key files, not certificate files. ruby/openssl is generating testing certificates from the keys in tests.

Testing in RubyCI servers

I plan to test this PR with RHEL 9 server (OpenSSL version is >= 3.5, but OpenSSL config disables PQC by default), which is the case of support_pqc_handshake? is false ,and OpenBSD server as I saw the following logic in DRbSSLService, test/drb/test_drbssl.rb.

    if RUBY_PLATFORM.match?(/openbsd/)
      config[:SSLMinVersion] = OpenSSL::SSL::TLS1_2_VERSION
      config[:SSLMaxVersion] = OpenSSL::SSL::TLS1_2_VERSION
    end

Edited: I confirmed the tests passed in RubyCI RHEL 9 and OpenBSD servers.

drbssl PQC tests

Single cert tests ML-DSA-65

This test with TestDRbSSLPQC and DRbSSLPQCService is inspired by my proof-of-concept "## drbssl (SSL) auto-generated ML-DSA-65 cert". Also closed to the TestDRbSSLCore and DRbSSLService.

Multi cert tests ML-DSA-65/RSA

This test is inspired by my proof-of-concept "drbssl (SSL) pre-generated ML-DSA-65/RSA multi cert with client cert" case, and also inspired by ruby/openssl test/openssl/test_ssl.rb test_pqc_sigalg (https://github.com/ruby/openssl/blob/9796ee8f47003ec78fd8e052bc8dd6d1fcb0ae2b/test/openssl/test_ssl.rb#L2097).

PQC multi cert tests are executed via the following classes.

  • test/drb/test_drbssl.rb
    • TestDRbSSLPQCMultiCertMLDSA65, DRbSSLPQCMultiCertMLDSA65Service
    • TestDRbSSLPQCMultiCertRSA, DRbSSLPQCMultiCertRSAService
  • test/drb/ut_drb_drbssl_pqc_multi_cert.rb

It is hard to understand the testing framework in test/drb.

In DRbSSLPQCMultiCertMLDSA65Service, there are manager_config and client_config. manager_config is to manage the config used in manager to manage a child process ut_drb_drbssl_pqc_multi_cert.rb in this case. The main test workflow is between the server test/drb/ut_drb_drbssl_pqc_multi_cert.rb and client with client_config part. Client with client_config connects to the server ut_drb_drbssl_pqc_multi_cert.rb. However, it seems the ut_drb_drbssl_pqc_multi_cert.rb's config is also used as a client to connect to the server manager_config.

@junaruga

Copy link
Copy Markdown
Member Author

I see the following CI failure. Let me fix it.

https://github.com/ruby/drb/actions/runs/30444292244/job/90550763624?pr=53

@junaruga
junaruga marked this pull request as draft July 29, 2026 10:40
Comment thread lib/drb/ssl.rb Outdated
@junaruga

Copy link
Copy Markdown
Member Author

I see the following CI failure. Let me fix it.

https://github.com/ruby/drb/actions/runs/30444292244/job/90550763624?pr=53

The cause of the CI failure was that OpenSSL::PKey::PKey#public_to_der didn't exist in openssl gem included in Ruby 2.7.0. It seems the method was added in openssl gem 2.2.0. I am not sure what the openssl gem version is in Ruby 2.7.0, as I couldn't find constant variable OpenSSL::VERSION in the openssl gem.

https://github.com/ruby/openssl/blob/v2.2.0/History.md?plain=1#L72-L75

Now use OpenSSL::PKey::RSA#public_key as before in RSA case.

cert.public_key = pkey.public_key

And use the following logic in ML-DSA-NN case.

cert.public_key = OpenSSL::PKey.read(pkey.public_to_der)

@junaruga

junaruga commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

I plan to test this PR with RHEL 9 server (OpenSSL version is >= 3.5, but OpenSSL config disables PQC by default), which is the case of support_pqc_handshake? is false ,and OpenBSD server as I saw the following logic in DRbSSLService, test/drb/test_drbssl.rb.

I tested this PR in RubyCI's RHEL 9 and OpenBSD servers.

In RHEL 9 server, I confirmed the tests passed. The ML-DSA tests are successfully omitted by support_pqc_handshake? in test/drb/drbtest.rb.

In OpenBSD server, I confirmed the tests passed as well. The used OpenSSL version was LibreSSL 4.3.0.

In DRbSSLService#start, test/drb/test_drbssl.rb, there is the following logic as I mentioned.

    if RUBY_PLATFORM.match?(/openbsd/)
      config[:SSLMinVersion] = OpenSSL::SSL::TLS1_2_VERSION
      config[:SSLMaxVersion] = OpenSSL::SSL::TLS1_2_VERSION
    end

I was not sure if DRbSSLPQCService#start, DRbSSLPQCMultiCertMLDSA65Service#start and DRbSSLPQCMultiCertRSAService#start needs the openbsd guard logic as well.

However, TestDRbSSLPQC, TestDRbSSLPQCMultiCertMLDSA65 and TestDRbSSLPQCMultiCertRSA skip OpenBSD's LibreSSL case by omit_unless_support_pqc in #setup before executing DRbSSLPQCService#start, DRbSSLPQCMultiCertMLDSA65Service#start and DRbSSLPQCMultiCertRSAService#start. So, we don't need the openbsd guard to these new 3 classes, DRbSSLPQCService, DRbSSLPQCMultiCertMLDSA65Service and DRbSSLPQCMultiCertRSAService.

@junaruga
junaruga marked this pull request as ready for review July 30, 2026 19:41
@junaruga

junaruga commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

@seki or other maintainers, I updated notes section on the first comment. This PR is ready to review. I would appreciate your reviews.

@junaruga

Copy link
Copy Markdown
Member Author

@rhenium I would appreciate your review for this PR as Ruby OpenSSL (openssl gem) maintainer.

# drbssl server with pre-generated ML-DSA-65/RSA multi cert and client cert

require_relative 'ut_drb'
require_relative 'drbtest_utils'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, this file required require_relative 'drbtest' here. However, I got the following errors. The test result was 0 failures, 0 errors. But you see the RuntimeError tracktrace logs in test/drb/ut_drb_drbssl_pqc_multi_cert.rb. The cause was test/drb/drbtest.rb requiring test-unit via require 'test/unit'. It seems that the test-unit's at_exit calling Test::Unit::AutoRunner.run below was triggered when the child process exited, and caused errors when DRbTests::TestDRbSSLPQCMultiCertMLDSA65 and DRbTests::TestDRbSSLPQCMultiCertRSA were executed. So, now this file requires require_relative 'drbtest_utils' which doesn't require test-unit.

https://github.com/test-unit/test-unit/blob/3.7.8/lib/test/unit.rb#L516-L519

$ bundle exec ruby -I test/lib -rhelper test/drb/test_drbssl.rb -v
Loaded suite test/drb/test_drbssl
Started
DRbTests::TestDRbSSLAry:
  test_01:									.: (0.413258)
  test_02_collect:								.: (0.436087)
  test_03_redo:									.: (0.433541)
  test_05_break:								.: (0.435188)
  test_06_next:									.: (0.389877)
  test_07_break_18:								.: (0.408441)
DRbTests::TestDRbSSLCore:
  test_00_DRbObject:								.: (0.419140)
  test_01:									.: (0.509643)
  test_01_02_loop:								.: (0.453699)
  test_02_basic_object:								.: (0.380818)
  test_02_unknown:								.: (0.421713)
  test_03:									.: (0.417325)
  test_04:									.: (0.416480)
  test_05_eq:									.: (0.385149)
  test_06_timeout:								.: (2.705462)
  test_07_private_missing:							.: (0.422167)
  test_07_protected_missing:							.: (0.451488)
  test_07_public_missing:							.: (0.409648)
  test_07_send_missing:								.: (0.544292)
  test_08_here:									.: (0.382050)
  test_09_option:								.: (0.420762)
  test_10_yield:								.: (0.367299)
  test_10_yield_undumped:							.: (0.408900)
  test_11_remote_no_method_error:						.: (0.442798)
DRbTests::TestDRbSSLPQC:
  test_01_hello:								.: (0.326881)
  test_group_sigalg:								.: (0.336463)
DRbTests::TestDRbSSLPQCMultiCertMLDSA65:
  test_01_hello:								.: (0.460257)
  test_group_sigalg:								/home/jaruga/var/git/ruby/drb/test/drb/ut_drb_drbssl_pqc_multi_cert.rb:12:in 'shift': usage: /home/jaruga/var/git/ruby/drb/test/drb/ut_drb_drbssl_pqc_multi_cert.rb <manager-uri> <name> (RuntimeError)
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1735:in 'block in OptionParser#parse_in_order'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1734:in 'Kernel#catch'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1734:in 'OptionParser#parse_in_order'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1728:in 'OptionParser#order!'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit/autorunner.rb:201:in 'Test::Unit::AutoRunner#process_args'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit/autorunner.rb:68:in 'Test::Unit::AutoRunner.run'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit.rb:518:in 'block (2 levels) in <top (required)>'
.: (0.479292)
DRbTests::TestDRbSSLPQCMultiCertRSA:
  test_01_hello:								.: (0.447127)
  test_group_sigalg:								/home/jaruga/var/git/ruby/drb/test/drb/ut_drb_drbssl_pqc_multi_cert.rb:12:in 'shift': usage: /home/jaruga/var/git/ruby/drb/test/drb/ut_drb_drbssl_pqc_multi_cert.rb <manager-uri> <name> (RuntimeError)
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1735:in 'block in OptionParser#parse_in_order'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1734:in 'Kernel#catch'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1734:in 'OptionParser#parse_in_order'
	from /home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/optparse.rb:1728:in 'OptionParser#order!'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit/autorunner.rb:201:in 'Test::Unit::AutoRunner#process_args'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit/autorunner.rb:68:in 'Test::Unit::AutoRunner.run'
	from /home/jaruga/var/git/ruby/drb/vendor/bundle/ruby/4.1.0+1/gems/test-unit-3.7.8/lib/test/unit.rb:518:in 'block (2 levels) in <top (required)>'
.: (0.482582)

Finished in 15.021564814 seconds.
---------------------------------------------------------------------------------------------------
30 tests, 88 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
---------------------------------------------------------------------------------------------------
2.00 tests/s, 5.86 assertions/s

DRb::DRbSSLSocket::SSLConfig already works with pre-generated ML-DSA cert/key by
:SSLCertificate and :SSLPrivateKey config options.

This commit adds the following features in PQC use cases to #setup_certificate
and #setup_ssl_context.

* Add :SSLCertificates config option accepting an Array of
  [certificate, private_key] pairs for dual/multiple certificate
  support via OpenSSL::SSL::SSLContext#add_certificate
  changing from OpenSSL::SSL::SSLContext#cert and #key.
  Because an SSL server that accepts clients with either ML-DSA-NN
  or RSA certificates is useful in the use case of PQC migration from RSA
  (non-PQC) to ML-DSA (PQC).
  :SSLCertificate and :SSLPrivateKey are available for backward compatibility.
  But use OpenSSL::SSL::SSLContext#add_certificate internally.
  This is a behavior change.
* Add :SSLPrivateKeyAlgorithms option accepting an Array of key
  algorithms (RSA, ML-DSA-44, ML-DSA-65, ML-DSA-87) for
  multi-certificate generation, defaulting to ["RSA"] for backward
  compatibility.
* Add :SSLGroups option to control key exchange group such as ML-KEM in PQC.
* Add :SSLSignatureAlgorithms, :SSLClientSignatureAlgorithms to control
  signature algorithms such as ML-DSA-NN in PQC, and RSA in non-PQC.
* Add tool/create_certs.sh to generate test/drb/fixtures/*.{crt,key}
  to test with pre-generated certs/keys and add DRbTests::Fixtures module
  to read the certs/keys.
* Add test/drb/test_ssl.rb to test lib/drb/ssl.rb as an unit test level.
  This approach aligns with test/drb/test_acl.rb to test lib/drb/acl.rb
  as an unit test level.
* Add TestDRbSSLPQC, TestDRbSSLPQCMultiCertMLDSA65, TestDRbSSLPQCMultiCertRSA
  in test/drb/test_drbssl.rb.
  TestDRbSSLPQC is to test single PQC ML-KEM/ML-DSA server via
  test/drb/ut_drb_drbssl_pqc.rb. The testing class is inspired by
  TestDRbSSLCore.
  TestDRbSSLPQCMultiCertMLDSA65 and TestDRbSSLPQCMultiCertRSA are to test
  ML-DSA-65 (PQC) and RSA (non-PQC) dual (multiple) certificate server
  with client certificate via test/drb/ut_drb_drbssl_pqc_multi_cert.rb.
  The test is designed for a high-security use case with :SSLVerifyMode
  OpenSSL::SSL::VERIFY_PEER | OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT.

Assisted-by: Claude:claude-opus-4-6[1m]
@junaruga

Copy link
Copy Markdown
Member Author

I added and updated the code comments.

Comment thread test/drb/test_drbssl.rb
[true, nil]
end
end
end

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to simplify the above tests TestDRbSSLPQC, TestDRbSSLPQCMultiCertMLDSA65 and TestDRbSSLPQCMultiCertRSA. These class designs align with the existing TestDRbSSL. However, I am not sure how much important or practical the current two-ways server client tests between manger and ut_*.rb to achieve both client (manager) connection to server (ut_*.rb) and client (ut_*.rb) connection to server (manager) via TestDRbSSL are. It's hard to understand and maintain the code.

Is it better just to create test_* without child classes of DRbService and ut_*.rb with the simple one-way server/client connection like run_drbssl_server_multi_cert_mldsa65_rsa_client_cert_pre_generated.rb and run_drbssl_client_multi_cert_mldsa65_rsa_client_cert_pre_generated.rb?

Comment thread lib/drb/ssl.rb
case pkey_alg
when "RSA"
pkey = OpenSSL::PKey::RSA.new(2048)
cert.public_key = pkey.public_key

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cert.public_key = pkey.public_key
cert.public_key = pkey

I recognize this was taken from the original code, but the OpenSSL::PKey::RSA#public_key call was unnecessary. In OpenSSL's design, a private key can always act as a public key.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The examples of OpenSSL::PKey::RSA#public_key to set to OpenSSL::X509::Certificate#public_key are documented below. Maybe shall we improve the examples not to use OpenSSL::PKey::RSA#public_key in ruby/openssl?

https://github.com/ruby/openssl/blob/e324933034dffe45dce64b83c351d545c0fc93c4/ext/openssl/ossl_x509cert.c#L925

https://github.com/ruby/openssl/blob/e324933034dffe45dce64b83c351d545c0fc93c4/ext/openssl/ossl_x509cert.c#L946

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize it was in ruby/openssl's docs. Yes, it's unnecessary and we should clean it up.

The only valid use case for RSA#public_key was in method chains with #to_der/#to_pem (e.g., rsa.public_key.to_der), but it should be updated to use rsa.public_to_der nowadays.

Comment thread lib/drb/ssl.rb
cert.public_key = pkey.public_key
digest = "SHA256"
when "ML-DSA-44", "ML-DSA-65", "ML-DSA-87"
# openssl gem >= 4.0.0 and OpenSSL >= 3.5.0 support ML-DSA.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work with all ruby/openssl versions as long as it's linked with OpenSSL >= 3.5. Perhaps it's best to pass through the OpenSSL::PKey::PKeyError (for minimum amount of code in drb, and for allowing the user see the raw context information that may be added by OpenSSL)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you are talking about the following part.

          begin
            pkey = OpenSSL::PKey.generate_key(pkey_alg)
          rescue OpenSSL::PKey::PKeyError

For the topic to pass through OpenSSL::PKey::PKeyError, a similar topic to pass through NoMethodError is also suggested on another thread https://github.com/ruby/drb/pull/53/changes#r3729234016. Considering these 2 similar suggestions from you and kou, I would agree with you guys that we should pass through the OpenSSL::PKey::PKeyError and NoMethodError without rescuing these exception classes. I will fix it.

Comment thread lib/drb/ssl.rb
# OpenSSL::SSL::SSLContext#sigalgs=, #client_sigalgs=, #groups= were
# added in openssl gem 4.0.0.
# https://github.com/ruby/openssl/blob/v4.0.0/History.md?plain=1#L25-L31
# OpenSSL::SSL::SSLContext#sigalgs= is supported in OpenSSL >= 1.0.2.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenSSL 1.0.2 is the minimum version supported by ruby/openssl bundled with non-EOL Ruby versions. I think you can forget about even older versions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I will remove the "1.0.2" information.

@rhenium

rhenium commented Aug 1, 2026

Copy link
Copy Markdown
Member
  • Add :SSLPrivateKeyAlgorithms option accepting an Array of key algorithms (RSA, ML-DSA-44, ML-DSA-65, ML-DSA-87) for multi-certificate generation, defaulting to ["RSA"] for backward compatibility.

I won't be the one deciding whether to merge this, but I'm not really convinced there's much value in being able to set up a server with multiple self-signed certificates.

@hsbt

hsbt commented Aug 6, 2026

Copy link
Copy Markdown
Member

@kou Could you review this? @junaruga introduced this work at today's Dev meeting.

I would appreciate it if you could review this, especially regarding the API changes such as the multiple certificates feature that was added in this PR.

Comment thread lib/drb/ssl.rb
Comment on lines +168 to +172
@certs = nil
if config[:SSLCertificate] && config[:SSLPrivateKey]
@certs = [[config[:SSLCertificate], config[:SSLPrivateKey]]]
end
@certs = config[:SSLCertificates] if config.key?(:SSLCertificates)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simplify these conditions?

Suggested change
@certs = nil
if config[:SSLCertificate] && config[:SSLPrivateKey]
@certs = [[config[:SSLCertificate], config[:SSLPrivateKey]]]
end
@certs = config[:SSLCertificates] if config.key?(:SSLCertificates)
if config.key?(:SSLCertificates)
@certs = config[:SSLCertificates]
elsif config[:SSLCertificate] && config[:SSLPrivateKey]
@certs = [[config[:SSLCertificate], config[:SSLPrivateKey]]]
else
@certs = nil
end

Comment thread lib/drb/ssl.rb
Comment on lines +173 to +174
@pkey_algs = config[:SSLPrivateKeyAlgorithms] ||
self[:SSLPrivateKeyAlgorithms]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need config[:SSLPrivateKeyAlgorithms] || here?

Suggested change
@pkey_algs = config[:SSLPrivateKeyAlgorithms] ||
self[:SSLPrivateKeyAlgorithms]
@pkey_algs = self[:SSLPrivateKeyAlgorithms]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. The self[:SSLPrivateKeyAlgorithms] is good enough considering the value by argument config. I will fix it.

      def initialize(config)
        @config   = config
      # A convenience method to access the values like a Hash
      def [](key);
        @config[key] || DEFAULT[key]
      end

Comment thread lib/drb/ssl.rb

# Establish the OpenSSL::SSL::SSLContext with the configuration
# parameters provided.
def setup_ssl_context

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you want to make setup_ssl_context public? It's just for test? If so, we should not make it public. We can use __send__ in test.

@junaruga junaruga Aug 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current master branch 2b002d4, the setup_ssl_context is already public method. I didn't change the method from private to public.

def setup_ssl_context

Comment thread lib/drb/drb.rb
end

# Error raised when an error occurs on the bad configuration.
class DRbBadConfig < DRbError; end

@kou kou Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using ArgumentError instead of our custom exception class?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the current situation on the master branch.

The reason why I created DRbBadConfig for bad configuration was because I saw the following exceptions classes.

$ grep -r '^.*class.*DRbError' lib
lib/drb/drb.rb:# dRuby-specific errors, all of which are subclasses of DRb::DRbError.
lib/drb/drb.rb:  class DRbError < RuntimeError; end
lib/drb/drb.rb:  class DRbConnError < DRbError; end
lib/drb/drb.rb:  class DRbServerNotFound < DRbError; end
lib/drb/drb.rb:  class DRbBadURI < DRbError; end
lib/drb/drb.rb:  class DRbBadScheme < DRbError; end
lib/drb/drb.rb:  class DRbUnknownError < DRbError
lib/drb/drb.rb:  class DRbRemoteError < DRbError

However, now I notice Ruby standard exception classes ArgumentError, NoMethodError, and SecurityError are also used.

$ grep -r raise lib/ | grep -E 'ArgumentError|NoMethodError|SecurityError'
lib/drb/drb.rb:      raise(ArgumentError, "#{any_to_s(msg_id)} is not a symbol") unless Symbol == msg_id.class
lib/drb/drb.rb:      raise(SecurityError, "insecure method '#{msg_id}'") if insecure_method?(msg_id)
lib/drb/drb.rb:          raise NoMethodError, "private method '#{msg_id}' called for #{desc}"
lib/drb/drb.rb:          raise NoMethodError, "protected method '#{msg_id}' called for #{desc}"
lib/drb/drb.rb:          raise NoMethodError, "private method '#{msg_id}' called for #{desc}"
lib/drb/drb.rb:          raise NoMethodError, "protected method '#{msg_id}' called for #{desc}"

I also see the following code and past commit about raising DRb custom exception classes.

drb/lib/drb/drb.rb

Lines 511 to 513 in 2b002d4

rescue NameError, ArgumentError
DRbUnknown.new($!, s)
end

drb/lib/drb/drb.rb

Lines 619 to 623 in 2b002d4

begin
Marshal::load(str)
rescue NameError, ArgumentError
DRbUnknown.new($!, str)
end

The commit 0be5d9b is notable. This commit replaced the raised exception class from ArgumentError to DRbConnError. Though I don't know the context or reason of this change.

And I personally interpret the above code comment "dRuby-specific errors, all of which are subclasses of DRb::DRbError". We should use DRb specific (custom) exception classes for DRb specific things, that is anything that can happen in ruby/drb.

What is your justification or reason to use Ruby's standard ArgumentError class rather than DRb custom exception classes?

Comment thread tool/create_certs.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Ruby instead of shell?

BTW, where is this used?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tool/create_certs.sh is used in ruby/rubygems repository below. I copied and adjusted for ruby/drb. Sure. I can rewrite this create_certs.sh script using Ruby.

https://github.com/ruby/rubygems/blob/master/tool/create_certs.sh

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a reference, there is also tool/create_certs.rb in ruby/rubygems repository. While tool/create_certs.sh is for SSL/TLS server and client keys/certs. the tool/create_certs.rb is for various and error keys/certs to test signed gem feature. So, I thought the tool/create_certs.sh would fit more for drbssl's client/server keys/certs.

https://github.com/ruby/rubygems/blob/master/tool/create_certs.rb

I am trying to modify the tool/create_certs.rb to generate ML-DSA-65 keys/certs to use them for signed gems feature tests on ruby/rubygems#9697.

Comment thread lib/drb/ssl.rb
if config[:SSLCertificate] && config[:SSLPrivateKey]
@certs = [[config[:SSLCertificate], config[:SSLPrivateKey]]]
end
@certs = config[:SSLCertificates] if config.key?(:SSLCertificates)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you ignore :SSLCertificates when :SSLCertificate and :SSLPrivateKey are specified? (Why don't you mix them/)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. When I implemented and wrote the logic that :SSLCertificates overrides :SSLCertificate and :SSLPrivateKey. But this is my logic bug. This config option input behavior confuses users. Obviously we shouldn't mix the config option check between SSLCertificates and :SSLCertificate/:SSLPrivateKey even when we manage the internal value as one @certs. Maybe when I got an idea that managing internal value as one @certs for both :SSLCertificates and :SSLCertificate/:SSLPrivateKey, that made me blind, thinking such a mixed input check logic. I will fix it.

      # :SSLCertificates ::
      #   An Array of [certificate, private_key] pairs. Each element
      #   is an Array of an OpenSSL::X509::Certificate and its
      #   corresponding private key.  This option is prioritized over
      #   :SSLCertificate and :SSLPrivateKey.  See
      #   OpenSSL::SSL::SSLContext#add_certificate

Comment thread lib/drb/ssl.rb
Comment on lines +231 to +236
unless ctx.respond_to?(:sigalgs=)
raise(DRbBadConfig,
unsupported_message(
':SSLSignatureAlgorithms', '4.0.0', '1.0.2'
))
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this check? I think that NoMethodError is enough without this check.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intended the this check to notify users the cause of the errors, and how to fix, notifying the information they need to upgrade Ruby OpenSSL or/and OpenSSL. At the same time, I don't understand how the standard libraries like ruby/drb should handle in the missing method cases due to old dependent libraries. So, if you say so, I will remove this check.

@junaruga

Copy link
Copy Markdown
Member Author

Sorry for my late response. Thanks for your reviews. I will check the reviews and fix for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants