Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests no longer pass on OpenSSL 3.0 #8

Open
adfoster-r7 opened this issue Jul 20, 2022 · 5 comments
Open

Unit tests no longer pass on OpenSSL 3.0 #8

adfoster-r7 opened this issue Jul 20, 2022 · 5 comments

Comments

@adfoster-r7
Copy link
Contributor

Hi there @SmallLars - thanks for the openssl-ccm Gem! 👍

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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 👍

@adfoster-r7
Copy link
Contributor Author

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

As an additional data point - I've verified the fixes against our RubySMB Gem and all of our unit tests are passing now 👍
rapid7/ruby_smb#234

@adfoster-r7
Copy link
Contributor Author

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

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

No branches or pull requests

2 participants