hrr_rb_ssh
hrr_rb_ssh copied to clipboard
Support openssl3
As noted by https://github.com/hirura/hrr_rb_ssh/issues/32 - it looks like OpenSSL 3.0 has backwards incompatible changes
This is an initial test PR to document some of the changes that have been required to get tests passing for OpenSSL v3, but there's still more work to go
@adfoster-r7 Wow, Great work again ... I didn't follow the recent Ruby/OpenSSL updates, so need clarification. The change loose Ruby 2.6 or earlier support, right ? I'd like to clarify this because, originally, I tried to keep this available for older Ruby versions.
I didn't follow the recent Ruby/OpenSSL updates, so need clarification.
The main differences I'm seeing are:
- Old ciphers are not loaded by default, I still need a workaround for this - I believe
OPENSSL_CONF
could be used https://github.com/hirura/hrr_rb_ssh/issues/32
irb(main):003:0> OpenSSL::Cipher.new('BF-CBC')
(irb):2:in `initialize': unsupported (OpenSSL::Cipher::CipherError)
from (irb):2:in `new'
from (irb):2:in `<main>'
from /var/lib/gems/3.0.0/gems/irb-1.3.6/exe/irb:11:in `<top (required)>'
from /usr/local/bin/irb:25:in `load'
from /usr/local/bin/irb:25:in `<main>'
-
set_pqg
throws an exception at runtime:
[1] pry(HrrRbSsh::OpenSSLCompat)> dh = OpenSSL::PKey::DH.new
=> #<OpenSSL::PKey::DH:0x00007f8e1c6d0298 oid=dhKeyAgreement>
[2] pry(HrrRbSsh::OpenSSLCompat)> dh.set_pqg p, nil, g
OpenSSL::PKey::PKeyError: dh#set_pqg= is incompatible with OpenSSL 3.0
from (pry):2:in `set_pqg'
- openssl objects are immutable:
[4] pry(HrrRbSsh::OpenSSLCompat)> dh.generate_key!
OpenSSL::PKey::DHError: OpenSSL::PKey::DH is immutable on OpenSSL 3.0; use OpenSSL::PKey.generate_key instead
The change loose Ruby 2.6 or earlier support, right ? I'd like to clarify this because, originally, I tried to keep this available for older Ruby versions.
I'm hoping this change will be backwards compatible with Ruby 2.6 - although it is now end of life https://www.ruby-lang.org/en/downloads/branches/
For the proof of concept I've added an if statement to verify if the native openssl library is greater than version 3.0.0 or not.
Thank you ! That's quite informative. You're right, Ruby 2.6 and earlier are already EOL. This is just for my interest. Please go ahead without backward compatibility consideration.
Just two error groups left until the unit tests will pass green again with the current approach of skipping the disabled deprecated ciphers such as blowfish etc (for now)
I'm not sure why these are failing just yet, any guidance appreciated :)
- SshDss - This seems to fail on Ubuntu 20.04 too
Failures:
1) HrrRbSsh::Transport::ServerHostKeyAlgorithm::SshDss when secret_key is not specified #verify with correct sign returns true
Failure/Error: expect( server_host_key_algorithm.verify sign, data ).to be true
expected true
got false
# ./spec/hrr_rb_ssh/transport/server_host_key_algorithm/ssh_dss_spec.rb:121:in `block (5 levels) in <top (required)>'
- DiffieHellmanGroup17Sha512 - I verified that P matches the specification that I found here https://datatracker.ietf.org/doc/html/rfc3526#page-6, and I'm not sure why it would be failing on OpenSSL 3 🤔
1) HrrRbSsh::Transport::KexAlgorithms::DiffieHellmanGroup17Sha512#initialize can be instantiated
Failure/Error: OpenSSL::PKey.generate_key(dh)
OpenSSL::PKey::PKeyError:
EVP_PKEY_keygen: modulus too small
# ./lib/hrr_rb_ssh/open_ssl_compat.rb:18:in `generate_key'
# ./lib/hrr_rb_ssh/open_ssl_compat.rb:18:in `new_dh_pkey'
# ./lib/hrr_rb_ssh/transport/kex_algorithms/diffie_hellman.rb:11:in `initialize'
# ./spec/hrr_rb_ssh/transport/kex_algorithms/diffie_hellman_group17_sha512_spec.rb:3:in `new'
# ./spec/hrr_rb_ssh/transport/kex_algorithms/diffie_hellman_group17_sha512_spec.rb:3:in `block (2 levels) in <top (required)>'
# ./spec/hrr_rb_ssh/transport/kex_algorithms/diffie_hellman_group17_sha512_spec.rb:76:in `block (3 levels) in <top (required)>'
The DSS server host key verification failure reproduced on my Mac. It might be due to Ruby 3.1.x, not OpenSSL version.
# RUBY_VERSION => 3.1.2
# OpenSSL::OPENSSL_VERSION => OpenSSL 1.1.1f 31 Mar 2020
Failures:
1) HrrRbSsh::Transport::ServerHostKeyAlgorithm::SshDss when secret_key is not specified #verify with correct sign returns true
Failure/Error: expect( server_host_key_algorithm.verify sign, data ).to be true
expected true
got false
# ./spec/hrr_rb_ssh/transport/server_host_key_algorithm/ssh_dss_spec.rb:124:in `block (5 levels) in <top (required)>'
I was able to re-enable legacy openssl providers for the test suite by setting ENV['OPENSSL_CONF']
to the new config file that I've added in spec/fixtures/openssl.conf
. Users that want the legacy ciphers (blowfish-cbc, cast128-cbc, arcfour) will also have to opt-in. I think this information could be added to the hrr_rb_ssh README or a changelog somewhere - but let me know your thoughts :+1:
I'm now blocked on the following two issues
- SshDss verify - which looks like an existing issue:
- Fails on Ubuntu 20.04, Ruby 3.x,
OpenSSLRubyVersion=3.0.0
,OpenSSLVersionNumber=OpenSSL 1.1.1f 31 Mar 2020
,OpenSSLLibraryVersionOpenSSL 1.1.1f 31 Mar 2020
- Passes on Ubuntu 20.04, Ruby 2.7,
OpenSSLRubyVersion=2.1.3
,OpenSSLVersionNumber=OpenSSL 1.1.1f 31 Mar 2020
,OpenSSLLibraryVersionOpenSSL 1.1.1f 31 Mar 2020
- Test run: https://github.com/adfoster-r7/hrr_rb_ssh/runs/7410201720?check_suite_focus=true
- Fails on your local set up too, thanks for testing!
- Fails on Ubuntu 20.04, Ruby 3.x,
- DiffieHellmanGroup17Sha512 modulus errors
For the DiffieHellmanGroup17Sha512 modulus error, I found my wrong P definition. I will raise an issue and fix outside of this PR.
In diffie_hellman_group17_sha512.rb,
P = \
"FFFFFFFF" "FFFFFFFF" "C90FDAA2" "2168C234" "C4C6628B" "80DC1CD1 29024E08" \
should be
"FFFFFFFF" "FFFFFFFF" "C90FDAA2" "2168C234" "C4C6628B" "80DC1CD1" "29024E08" \
Awesome; nice catch! :+1:
When I compared hrr_rb_ssh's P definition to the RFC, I intentionally removed whitespace - so I unfortunately missed that 😄
DH group17's P is now fixed. Please see #36. Most probably I did the same thing...
Awesome, thanks! I've just rebased against that commit; now CI only has one unit test failing across the Ruby/OpenSSL versions - which seems like a separate issue to OpenSSL 3 support
1) HrrRbSsh::Transport::ServerHostKeyAlgorithm::SshDss when secret_key is not specified #verify with correct sign returns true
Failure/Error: expect( server_host_key_algorithm.verify sign, data ).to be true
expected true
got false
# ./spec/hrr_rb_ssh/transport/server_host_key_algorithm/ssh_dss_spec.rb:121:in `block (5 levels) in <top (required)>'
That's good to hear. Give me some moment so that I figure out the cause and fix the DSS issue.
I took some time to dig in the dss issue: see #37. I could successfully run the test suite with this PR on Ubuntu 22.04 with system Ruby (3.0) \o/.
Thanks for taking a look @smortex - that pointed me in a good direction to dig further 🕵️
Tests should be passing green now 🟢
For context on the final change to get the DSS tests working:
It looks like by default Ruby is generating q with different bit length between the OpenSSL versions:
# OpenSSL 1.1.1
2.7.2 :017 > OpenSSL::PKey::DSA.new(1024).q.to_s(2).bytes.count * 8
=> 160
# OpenSSL 3.x
irb(main):002:0> OpenSSL::PKey::DSA.new(1024).q.to_s(2).bytes.count
=> 224
RFC 4253 links to FIPS-186-2; Which has some requirements on the values of N and L. Since it looks like OpenSSL 3 now generated N as 224 by default, then L should be 2048:
https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf
4.2 Selection of Parameter Sizes and Hash Functions for DSA
This Standard specifies the following choices for the pair L and N (the bit lengths of p and q, respectively):
L = 1024, N = 160
L = 2048, N = 224
L = 2048, N = 256
L = 3072, N = 256
So we could most likely update the default secret key for OpenSSL 3 to be updated to match:
- SECRET_KEY = OpenSSL::PKey::DSA.new(1024).to_pem
+ SECRET_KEY = OpenSSL::PKey::DSA.new(2048).to_pem
However as noted by @smortex we'd also need to change these lines as well to handle the different byte size:
https://github.com/hirura/hrr_rb_ssh/pull/37/files#diff-9af7963c32cdeaf03421690df2a1bb0b89b0ba0b73751a6dd572139e3ba0a7cfL59-R60
To make things easier - I've just gone ahead and updated the key generation to use L=1024,N=160 for consistency - using the options from https://www.openssl.org/docs/man3.0/man1/openssl-genpkey.html
So the old OpenSSL::PKey::DSA.new(1024).to_pem
call has now been swapped for something more explicit for the OpenSSL 3 codepath:
OpenSSL::PKey.generate_key(
OpenSSL::PKey.generate_parameters('DSA', {
# The number of bits in the generated prime
'dsa_paramgen_bits' => 1024,
# The number of bits in the q parameter. Must be one of 160, 224 or 256. If not specified 224 is used.
# This must align with an allowed combination from Section 4.2 in
# https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf
'dsa_paramgen_q_bits' => 160
})
).to_pem
I think a separate PR could clean up the existing hard coded 20 byte size in the sign/verify command so that L could be 2048 and and N could be 224, but I don't think that should be a blocker.
Awesome :heart_eyes: !
@hirura Let me know if there's anything else I can do to help with landing this PR 💯
@hirura I've tidied up the PR to only include the openssl changes now. The unit tests are passing, and I've also tested it with your example test script from smortex's PR, as well as verifying these changes work with metasploit-framework on ubuntu 22.04.
I wasn't sure if there were any of the demo
scripts that I should also test out, but let me know and I can see if there's anything else I need to change 💯
OpenSSL 1.1.x is EOL in September, fwiw, so support for OpenSSL 3 by then would help out distros a lot if possible.
@hirura Let me know if there's anything else I can do to help here :+1:
@hirura Is there anything I can do to help here? 📈
@hirura I've updated this to include the tests passing on Ruby 3.3.0-preview1 and 3.3.0-preview2 :+1:
I think this would be good to release 🤞