Bug #8720: ECB mode seems to be broken

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.0.0p283 (2013-07-28) [x86_64-darwin12.0.0]

[netjunki (Ben Lau)]

The reporting on this is partially from someone else's work that I found in a gist on github:
https://gist.github.com/tarcieri/5550786

But there appears to be some sort of issue with ECB mode. It works correctly in JRuby, which I assume is due to the fact that JRuby is using Java's underlying crypto libs instead of OpenSSL.

I've attached the test code from the gist. I've also included the output in the description here from my run with latest svn and the gist for reference:

Testing output:

$ /usr/local/bin/ruby --version; /usr/local/bin/ruby ecb_test.rb
ruby 2.0.0p283 (2013-07-28) [x86_64-darwin12.0.0]
Testing encryption: FAILED! Got "\xCE\x9Dp\xDFL\xD0\x95\xC3\x13\x18+\xAC\x1D2\xE7\x15" instead of ":\xD7{\xB4\rz6`\xA8\x9E\xCA\xF3$f\xEF\x97"
Testing decryption: OK!

$ ruby --version; ruby ecb_test.rb
ruby 1.9.3p392 (2013-02-22 revision 39386) [x86_64-darwin12.3.0]
Testing encryption: FAILED! Got "\xCE\x9Dp\xDFL\xD0\x95\xC3\x13\x18+\xAC\x1D2\xE7\x15" instead of ":\xD7{\xB4\rz6`\xA8\x9E\xCA\xF3$f\xEF\x97"
Testing decryption: OK!

$ ruby --version; ruby ecb_test.rb
ruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-darwin12.3.0]
Testing encryption: FAILED! Got "\xCE\x9Dp\xDFL\xD0\x95\xC3\x13\x18+\xAC\x1D2\xE7\x15" instead of ":\xD7{\xB4\rz6`\xA8\x9E\xCA\xF3$f\xEF\x97"
Testing decryption: OK!

$ ruby --version; ruby ecb_test.rb
jruby 1.7.2 (1.9.3p327) 2013-01-04 302c706 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_21-b12 [darwin-x86_64]
Testing encryption: OK!
Testing decryption: OK!

[MartinBosslet (Martin Bosslet)]

• Status changed from Open to Rejected
• Assignee set to MartinBosslet (Martin Bosslet)

=begin
Hi Ben,

thanks for caring! But it's not really a bug of Ruby OpenSSL, the problem is that unfortunately the Cipher instance is stateful and it is important in which order you call #encrypt, #key= etc. In our defense, this is mentioned in the docs [1].

The example from the gist may be fixed like this:

require 'openssl'

AES-128 ECB mode test vectors

Taken from: http://www.inconteam.com/software-development/41-encryption/55-aes-test-vectors#aes-ecb-128

KEY = ["2b7e151628aed2a6abf7158809cf4f3c"].pack("H*")
PLAINTEXT = ["6bc1bee22e409f96e93d7e117393172a"].pack("H*")
CIPHERTEXT = ["3ad77bb40d7a3660a89ecaf32466ef97"].pack("H*")

cipher = OpenSSL::Cipher::Cipher.new("aes-128-ecb")
cipher.encrypt # call before assigning key
cipher.key = KEY
cipher.padding = 0 # Padding is enabled by default o_O

print "Testing encryption: "

ciphertext = cipher.update(PLAINTEXT) << cipher.final

if ciphertext == CIPHERTEXT
puts "OK!"
else
puts "FAILED! Got #{ciphertext.inspect} instead of #{CIPHERTEXT.inspect}"
end

print "Testing decryption: "

cipher.reset
cipher.decrypt # call before assigning key
cipher.key = KEY # needs to be set again

plaintext = cipher.update(CIPHERTEXT) << cipher.final

if plaintext == PLAINTEXT
puts "OK!"
else
puts "FAILED! Got #{plaintext.inspect} instead of #{PLAINTEXT.inspect}"
end

[1] http://www.ruby-doc.org/stdlib-2.0/libdoc/openssl/rdoc/OpenSSL/Cipher.html#method-i-encrypt
=end

[zzak (zzak _)]

• Status changed from Rejected to Open
• Assignee changed from MartinBosslet (Martin Bosslet) to zzak (zzak _)

I think we could do better to warn users when doing things out of order.

In this case we have a stateful thing, which should know whether key is set. In the case it's not yet, we should raise. This could be considered behavior change, however I think most users don't expect garbage output either.

[zzak (zzak _)]

• Assignee changed from zzak (zzak _) to 7150

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Closed
• Backport deleted (1.9.3: UNKNOWN, 2.0.0: UNKNOWN)

I agree that we should do better in this area. When calling Cipher#encrypt or Cipher#decrypt without arguments, the cipher is initialized with an empty key. This is normally not a problem, since you are supposed to call these methods directly after initializing the cipher according to the documentation. However, if you set the key first, then call one of the methods, the methods use a NULL key, but keep the flag that records whether the key was set. I submitted a pull request to unset the flag if the key is set to NULL. That way Cipher#update will raise an error that the key is not set, unless you set the key again after calling encrypt or decrypt. This pull request was just merged: https://github.com/ruby/openssl/pull/263.