SmallLars / openssl-ccm

Ruby Gem for RFC 3610 - Counter with CBC-MAC (CCM)
rubygems.org/gems/openssl-ccm
MIT License
2 stars 6 forks source link

Unit tests no longer pass on OpenSSL 3.0 #8

Open adfoster-r7 opened 2 years ago

adfoster-r7 commented 2 years ago

Hi there @SmallLars - thanks for the openssl-ccm Gem! :+1:

It looks like OpenSSL 3.0 is out now, and the library/unit tests no longer work as expected with OpenSSL 3.0 https://github.com/ruby/openssl/blob/b31446464e1e9f8bd0c58e92f9e74a9c7663b0e0/History.md#version-300

With a fresh Ubuntu 22.04 machine, and the default Ruby 3.0 and OpenSSL 3.0 install the test suite fails:

Failure: test_aes_data(CCMTest)
/home/a/openssl-ccm/test/test_ccm.rb:294:in `block (5 levels) in test_aes_data'
     291:               output = o_file.read
     292:               ccm = OpenSSL::CCM.new(cipher, [key[j]].pack('H*'), mac_len[j])
     293:               c = ccm.encrypt(input, [nonce[j]].pack('H*'))
  => 294:               assert_equal(output.unpack('H*'), c.unpack('H*'),
     295:                            "Wrong ENCRYPT in Vector #{i + 1}")
     296:             end
     297:           end

With openssl 1.1.1n and libressl tests pass as expected:

6 tests, 203 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

I'm happy to help out where possible, let me know your thoughts 👍

adfoster-r7 commented 2 years ago

I just had a quick run through of openssl-cmac and it looks like the tests are failing too

Here's an example failure for OpenSSL 3.0:

Failure: test_cmac_digest(CMACTest)
/home/a/openssl-cmac/test/test_cmac.rb:142:in `block in test_cmac_digest'
     139: 
     140:       cmac.update(DATA[3].b[0...20])
     141:       m = cmac.update(DATA[3].b[20...64]).digest.unpack('H*')[0]
  => 142:       assert_equal(MAC[3], m, 'Digest after update')
     143: 
     144:       cmac.update(DATA[3])
     145:       m = cmac.update('').digest.unpack('H*')[0]
/home/a/openssl-cmac/test/test_cmac.rb:135:in `each'
/home/a/openssl-cmac/test/test_cmac.rb:135:in `test_cmac_digest'
Digest after update
<"51f0bebf7e3b9d92fc49741779363cfe">(UTF-8) expected but was
<"a3083f2b1c1f742d566cef5c4cc1dd3d">(US-ASCII)
...
5 tests, 32 assertions, 4 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
20% passed

Versus OpenSSL 1.1.1:

5 tests, 276 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

I thought it made sense grouping issues for openssl-ccm and openssl-cmac together, but I can raise a separate issue if that's preferred 💯

SmallLars commented 2 years ago

Thank you very much @adfoster-r7. I am very happy about your PRs. It's been a long time since I last worked with Ruby, so this helps alot.

Do you think it is possible for each gem, to make a small commit / PR first, where only ccm.rb / cmac.rb is changed? So we can check if this change is working in the old environment / CI and maybe publish a fast update. I would like to spend some time to understand your environment changes without blocking a fix.

Maybe the memory for the initialization vector is no longer initialized with zero bytes by OpenSSL and contains data garbage. So it can be seen as an security improvement to force user to set/consider an own initialization vector and avoid to work with a constant default.

adfoster-r7 commented 2 years ago

I've created two new PRs with just the code changes, and it looks like the old Travis setup no longer triggers:

Unfortunately Travis isn't really a viable option for running tests on Github for open source projects these days!

Let me know if there's anything else I can do to help :+1:

adfoster-r7 commented 2 years ago

@SmallLars Let me know if there's anything I can do to help here :+1:

As an additional data point - I've verified the fixes against our RubySMB Gem and all of our unit tests are passing now :+1: https://github.com/rapid7/ruby_smb/pull/234

adfoster-r7 commented 2 years ago

Thank you for landing + releasing the single line change PRs 🎉