CVE-2016-7798: Inadequate Encryption Strength (CWE-326)
value: The openssl gem for Ruby uses the same initialization vector (IV) in GCM Mode (aes-*-gcm) when the IV is set before the key, which makes it easier for context-dependent attackers to bypass the encryption protection mechanism.
CWE description: The product stores or transmits sensitive data using an encryption scheme that is theoretically sound, but is not strong enough for the level of protection required.
CVSS v3 base score: 7.5
Published: 2017-01-30T22:59Z
Repository: https://github.com/ruby/openssl
Language: Ruby
Files changed: ext/openssl/ossl_cipher.c, test/test_cipher.rb
Commit date: 2016-09-28 14:38:30 +0900
Commit message: cipher: don't set dummy encryption key in Cipher#initialize

Remove the encryption key initialization from Cipher#initialize. This
is effectively a revert of r32723 ("Avoid possible SEGV from AES
encryption/decryption", 2011-07-28).

r32723, which added the key initialization, was a workaround for
Ruby Bug #2768. For some certain ciphers, calling EVP_CipherUpdate()
before setting an encryption key caused segfault. It was not a problem
until OpenSSL implemented GCM mode - the encryption key could be
overridden by repeated calls of EVP_CipherInit_ex(). But, it is not the
case for AES-GCM ciphers. Setting a key, an IV, a key, in this order
causes the IV to be reset to an all-zero IV.

The problem of Bug #2768 persists on the current versions of OpenSSL.
So, make Cipher#update raise an exception if a key is not yet set by the
user. Since encrypting or decrypting without key does not make any
sense, this should not break existing applications.

Users can still call Cipher#key= and Cipher#iv= multiple times with
their own responsibility.

Reference: https://bugs.ruby-lang.org/issues/2768
Reference: https://bugs.ruby-lang.org/issues/8221
Reference: https://github.com/ruby/openssl/issues/49

Diff: diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c
index f1f3459..ffa080b 100644
--- a/ext/openssl/ossl_cipher.c
+++ b/ext/openssl/ossl_cipher.c
@@ -34,11 +34,11 @@
 /*
  * Classes
  */
 VALUE cCipher;
 VALUE eCipherError;
-static ID id_auth_tag_len;
+static ID id_auth_tag_len, id_key_set;
 
 static VALUE ossl_cipher_alloc(VALUE klass);
 static void ossl_cipher_free(void *ptr);
 
 static const rb_data_type_t ossl_cipher_type = {
@@ -116,31 +116,21 @@ static VALUE
 ossl_cipher_initialize(VALUE self, VALUE str)
 {
     EVP_CIPHER_CTX *ctx;
     const EVP_CIPHER *cipher;
     char *name;
-    unsigned char dummy_key[EVP_MAX_KEY_LENGTH] = { 0 };
 
     name = StringValueCStr(str);
     GetCipherInit(self, ctx);
     if (ctx) {
 	ossl_raise(rb_eRuntimeError, "Cipher already inititalized!");
     }
     AllocCipher(self, ctx);
     if (!(cipher = EVP_get_cipherbyname(name))) {
 	ossl_raise(rb_eRuntimeError, "unsupported cipher algorithm (%"PRIsVALUE")", str);
     }
-    /*
-     * EVP_CipherInit_ex() allows to specify NULL to key and IV, however some
-     * ciphers don't handle well (OpenSSL's bug). [Bug #2768]
-     *
-     * The EVP which has EVP_CIPH_RAND_KEY flag (such as DES3) allows
-     * uninitialized key, but other EVPs (such as AES) does not allow it.
-     * Calling EVP_CipherUpdate() without initializing key causes SEGV so we
-     * set the data filled with "\0" as the key by default.
-     */
-    if (EVP_CipherInit_ex(ctx, cipher, NULL, dummy_key, NULL, -1) != 1)
+    if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, -1) != 1)
 	ossl_raise(eCipherError, NULL);
 
     return self;
 }
 
@@ -249,10 +239,13 @@ ossl_cipher_init(int argc, VALUE *argv, VALUE self, int mode)
     }
     if (EVP_CipherInit_ex(ctx, NULL, NULL, p_key, p_iv, mode) != 1) {
 	ossl_raise(eCipherError, NULL);
     }
 
+    if (p_key)
+	rb_ivar_set(self, id_key_set, Qtrue);
+
     return self;
 }
 
 /*
  *  call-seq:
@@ -335,10 +328,12 @@ ossl_cipher_pkcs5_keyivgen(int argc, VALUE *argv, VALUE self)
     if (EVP_CipherInit_ex(ctx, NULL, NULL, key, iv, -1) != 1)
 	ossl_raise(eCipherError, NULL);
     OPENSSL_cleanse(key, sizeof key);
     OPENSSL_cleanse(iv, sizeof iv);
 
+    rb_ivar_set(self, id_key_set, Qtrue);
+
     return Qnil;
 }
 
 static int
 ossl_cipher_update_long(EVP_CIPHER_CTX *ctx, unsigned char *out, long *out_len_ptr,
@@ -385,10 +380,13 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self)
     long in_len, out_len;
     VALUE data, str;
 
     rb_scan_args(argc, argv, "11", &data, &str);
 
+    if (!RTEST(rb_attr_get(self, id_key_set)))
+	ossl_raise(eCipherError, "key not set");
+
     StringValue(data);
     in = (unsigned char *)RSTRING_PTR(data);
     if ((in_len = RSTRING_LEN(data)) == 0)
         ossl_raise(rb_eArgError, "data must not be empty");
     GetCipher(self, ctx);
@@ -486,10 +484,12 @@ ossl_cipher_set_key(VALUE self, VALUE key)
 	ossl_raise(rb_eArgError, "key must be %d bytes", key_len);
 
     if (EVP_CipherInit_ex(ctx, NULL, NULL, (unsigned char *)RSTRING_PTR(key), NULL, -1) != 1)
 	ossl_raise(eCipherError, NULL);
 
+    rb_ivar_set(self, id_key_set, Qtrue);
+
     return key;
 }
 
 /*
  *  call-seq:
@@ -1080,6 +1080,7 @@ Init_ossl_cipher(void)
     rb_define_method(cCipher, "iv_len", ossl_cipher_iv_length, 0);
     rb_define_method(cCipher, "block_size", ossl_cipher_block_size, 0);
     rb_define_method(cCipher, "padding=", ossl_cipher_set_padding, 1);
 
     id_auth_tag_len = rb_intern_const("auth_tag_len");
+    id_key_set = rb_intern_const("key_set");
 }
diff --git a/test/test_cipher.rb b/test/test_cipher.rb
index 74c5394..015bb56 100644
--- a/test/test_cipher.rb
+++ b/test/test_cipher.rb
@@ -88,10 +88,11 @@ class OpenSSL::TestCipher < OpenSSL::TestCase
     assert_raise(ArgumentError) { @c1.iv = "\x01" * 9 }
   end
 
   def test_empty_data
     @c1.encrypt
+    @c1.random_key
     assert_raise(ArgumentError){ @c1.update("") }
   end
 
   def test_initialize
     assert_raise(RuntimeError) {@c1.__send__(:initialize, "DES-EDE3-CBC")}
@@ -134,16 +135,14 @@ class OpenSSL::TestCipher < OpenSSL::TestCase
       c2.pkcs5_keyivgen("passwd")
       assert_equal(pt, c2.update(ct) + c2.final)
     }
   end
 
-  def test_AES_crush
-    500.times do
-      assert_nothing_raised("[Bug #2768]") do
-        # it caused OpenSSL SEGV by uninitialized key
-        OpenSSL::Cipher::AES128.new("ECB").update "." * 17
-      end
+  def test_update_raise_if_key_not_set
+    assert_raise(OpenSSL::Cipher::CipherError) do
+      # it caused OpenSSL SEGV by uninitialized key [Bug #2768]
+      OpenSSL::Cipher::AES128.new("ECB").update "." * 17
     end
   end
 
   if has_ciphers?(['aes-128-gcm', 'aes-192-gcm', 'aes-256-gcm'])
 
@@ -315,10 +314,28 @@ class OpenSSL::TestCipher < OpenSSL::TestCase
     assert_raise(OpenSSL::Cipher::CipherError) {
       decipher.update(ct) + decipher.final
     }
   end if has_cipher?("aes-128-ocb")
 
+  def test_aes_gcm_key_iv_order_issue
+    pt = "[ruby/openssl#49]"
+    cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
+    cipher.key = "x" * 16
+    cipher.iv = "a" * 12
+    ct1 = cipher.update(pt) << cipher.final
+    tag1 = cipher.auth_tag
+
+    cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
+    cipher.iv = "a" * 12
+    cipher.key = "x" * 16
+    ct2 = cipher.update(pt) << cipher.final
+    tag2 = cipher.auth_tag
+
+    assert_equal ct1, ct2
+    assert_equal tag1, tag2
+  end if has_cipher?("aes-128-gcm")
+
   private
 
   def new_encryptor(algo)
     cipher = OpenSSL::Cipher.new(algo)
     cipher.encrypt

Vulnerable code: static ID id_auth_tag_len;
    unsigned char dummy_key[EVP_MAX_KEY_LENGTH] = { 0 };
    /*
     * EVP_CipherInit_ex() allows to specify NULL to key and IV, however some
     * ciphers don't handle well (OpenSSL's bug). [Bug #2768]
     *
     * The EVP which has EVP_CIPH_RAND_KEY flag (such as DES3) allows
     * uninitialized key, but other EVPs (such as AES) does not allow it.
     * Calling EVP_CipherUpdate() without initializing key causes SEGV so we
     * set the data filled with "\0" as the key by default.
     */
    if (EVP_CipherInit_ex(ctx, cipher, NULL, dummy_key, NULL, -1) != 1)
  def test_AES_crush
    500.times do
      assert_nothing_raised("[Bug #2768]") do
        # it caused OpenSSL SEGV by uninitialized key
        OpenSSL::Cipher::AES128.new("ECB").update "." * 17
      end
Fixed code: static ID id_auth_tag_len, id_key_set;
    if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, -1) != 1)
    if (p_key)
	rb_ivar_set(self, id_key_set, Qtrue);

    rb_ivar_set(self, id_key_set, Qtrue);

    if (!RTEST(rb_attr_get(self, id_key_set)))
	ossl_raise(eCipherError, "key not set");

    rb_ivar_set(self, id_key_set, Qtrue);

    id_key_set = rb_intern_const("key_set");
    @c1.random_key
  def test_update_raise_if_key_not_set
    assert_raise(OpenSSL::Cipher::CipherError) do
      # it caused OpenSSL SEGV by uninitialized key [Bug #2768]
      OpenSSL::Cipher::AES128.new("ECB").update "." * 17
  def test_aes_gcm_key_iv_order_issue
    pt = "[ruby/openssl#49]"
    cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
    cipher.key = "x" * 16
    cipher.iv = "a" * 12
    ct1 = cipher.update(pt) << cipher.final
    tag1 = cipher.auth_tag

    cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
    cipher.iv = "a" * 12
    cipher.key = "x" * 16
    ct2 = cipher.update(pt) << cipher.final
    tag2 = cipher.auth_tag

    assert_equal ct1, ct2
    assert_equal tag1, tag2
  end if has_cipher?("aes-128-gcm")