we've not been compatible with MRI's DES (EDE) - partly due DES(3) ECB mode default

fixing jruby/jruby#2617 as well as jruby/jruby#931

Author: kares

src/main/java/org/jruby/ext/openssl/Cipher.java

@@ -338,11 +338,11 @@ static Map<String, String[]> allSupportedCiphers() {
 
                 modes = cipherModes("DESede");
                 if ( modes != null ) {
-                    supportedCiphers.put( "DES-EDE", new String[] { "DES", "CBC", "EDE", "DESede/CBC" } );
-                    supportedCiphers.put( "DES-EDE-CBC", supportedCiphers.get("DES-EDE") );
+                    supportedCiphers.put( "DES-EDE", new String[] { "DES", "ECB", "EDE", "DESede/ECB" } );
+                    supportedCiphers.put( "DES-EDE-CBC", new String[] { "DES", "CBC", "EDE", "DESede/CBC" } );
                     supportedCiphers.put( "DES-EDE-CFB", new String[] { "DES", "CBC", "EDE", "DESede/CFB" } );
                     supportedCiphers.put( "DES-EDE-OFB", new String[] { "DES", "CBC", "EDE", "DESede/OFB" } );
-                    supportedCiphers.put( "DES-EDE3", new String[] { "DES", "CBC", "EDE3", "DESede/CBC" });
+                    supportedCiphers.put( "DES-EDE3", new String[] { "DES", "ECB", "EDE3", "DESede/ECB" });
                     for ( final String mode : modes ) {
                         supportedCiphers.put( "DES-EDE3-" + mode, new String[] { "DES", mode, "EDE3", "DESede/" + mode });
                     }
@@ -427,18 +427,21 @@ private static Algorithm osslToJava(final String osslName, final String padding)
                 alg.realName = algVals[3];
                 alg.realNameNeedsPadding = true;
                 alg.padding = getPaddingType(padding, cryptoMode);
+
+                System.out.println(osslName + " alg = " + alg);
+
                 return alg;
             }
 
-            String cryptoBase, cryptoVersion = null, cryptoMode = "CBC", realName;
-            String paddingType;
+            String cryptoBase, cryptoVersion = null, cryptoMode, realName;
+            String paddingType = null;
 
             // EXPERIMENTAL: if there's '/' assume it's a "real" JCE name :
             if ( osslName.indexOf('/') != -1 ) {
                 // e.g. "DESedeWrap/CBC/NOPADDING"
                 final List names = StringHelper.split((CharSequence) osslName, '/');
                 cryptoBase = (String) names.get(0);
-                if ( names.size() > 1 ) cryptoMode = (String) names.get(1);
+                cryptoMode = names.size() > 1 ? (String) names.get(1) : "CBC";
                 paddingType = getPaddingType(padding, cryptoMode);
                 if ( names.size() > 2 ) paddingType = (String) names.get(2);
                 Algorithm alg = new Algorithm(cryptoBase, null, cryptoMode);
@@ -449,7 +452,7 @@ private static Algorithm osslToJava(final String osslName, final String padding)
 
             int s = osslName.indexOf('-'); int i = 0;
             if (s == -1) {
-                cryptoBase = osslName;
+                cryptoBase = osslName; cryptoMode = null;
             }
             else {
                 cryptoBase = osslName.substring(i, s);
@@ -471,19 +474,19 @@ private static Algorithm osslToJava(final String osslName, final String padding)
             cryptoBase = cryptoBase.toUpperCase(); // allways upper e.g. "AES"
             if ( cryptoMode != null ) cryptoMode = cryptoMode.toUpperCase();
 
-            boolean realNameSet = false;
+            boolean realNameSet = false; boolean setDefaultCryptoMode = true;
 
             if ( "BF".equals(cryptoBase) ) realName = "Blowfish";
             else if ( "CAST".equals(cryptoBase) ) realName = "CAST5";
             else if ( cryptoBase.startsWith("DES") ) {
                 if ( "DES3".equals(cryptoBase) ) {
-                    cryptoBase = "DES"; realName = "DESede"; // cryptoVersion = cryptoMode; cryptoMode = "CBC";
+                    cryptoBase = "DES"; realName = "DESede"; cryptoVersion = "EDE3"; // cryptoMode = "CBC";
                 }
                 else if ( "EDE3".equalsIgnoreCase(cryptoVersion) || "EDE".equalsIgnoreCase(cryptoVersion) ) {
-                    realName = "DESede";
+                    realName = "DESede"; if ( cryptoMode == null ) cryptoMode = "ECB";
                 }
                 else if ( "EDE3".equalsIgnoreCase(cryptoMode) || "EDE".equalsIgnoreCase(cryptoMode) ) {
-                    realName = "DESede"; cryptoVersion = cryptoMode; cryptoMode = "CBC";
+                    realName = "DESede"; cryptoVersion = cryptoMode; cryptoMode = "ECB";
                 }
                 else realName = "DES";
             }
@@ -513,12 +516,14 @@ else if ( cryptoBase.length() > 8 && cryptoBase.startsWith("CAMELLIA") ) {
                         cryptoVersion = cryptoMode;
                     }
                     cryptoMode = null; // padding = null;
+                    setDefaultCryptoMode = false;
                     // cryptoMode = "NONE"; paddingType = "NoPadding";
                     realNameSet = true;
                 }
             }
 
-            paddingType = getPaddingType(padding, cryptoMode);
+            if ( cryptoMode == null && setDefaultCryptoMode ) cryptoMode = "CBC";
+            if ( paddingType == null ) paddingType = getPaddingType(padding, cryptoMode);
 
             if ( cryptoMode != null ) {
                 //if ( ! KNOWN_BLOCK_MODES.contains(cryptoMode) ) {
@@ -551,9 +556,8 @@ String getPadding() {
         }
 
         private static String getPaddingType(final String padding, final String cryptoMode) {
-
+            //if ( "ECB".equals(cryptoMode) ) return "NoPadding";
             // TODO check cryptoMode CFB/OFB
-
             final String defaultPadding = "PKCS5Padding";
 
             if ( padding == null ) return defaultPadding;
@@ -615,8 +619,11 @@ public int getIvLength() {
                 //else if ( "DES".equals(base) ) {
                 //    ivLength = 8;
                 //}
-                else if ( "RC4".equals(base) ) {
-                    ivLength = 8;
+                //else if ( "RC4".equals(base) ) {
+                //    ivLength = 8;
+                //}
+                else if ( "ECB".equals(mode) ) {
+                    ivLength = 0;
                 }
                 else {
                     ivLength = 8;
@@ -639,11 +646,9 @@ public int getKeyLength() {
             }
             if ( keyLen == -1 ) {
                 if ( "DES".equals(base) ) {
-                    if ( "EDE3".equalsIgnoreCase(version) ) {
-                        keyLen = 24;
-                    } else {
-                        keyLen = 8;
-                    }
+                    if ( "EDE".equalsIgnoreCase(version) ) keyLen = 16;
+                    else if ( "EDE3".equalsIgnoreCase(version) ) keyLen = 24;
+                    else keyLen = 8;
                 }
                 else if ( "RC4".equals(base) ) {
                     keyLen = 16;
@@ -715,22 +720,23 @@ public Cipher(Ruby runtime, RubyClass type) {
     private String padding;
 
     private void dumpVars(final PrintStream out, final String header) {
-        out.println(this.toString() + ' ' + header);
-        out.println(" name = " + name);
-        out.println(" cryptoBase = " + cryptoBase);
-        out.println(" cryptoVersion = " + cryptoVersion);
-        out.println(" cryptoMode = " + cryptoMode);
-        out.println(" padding_type = " + paddingType);
-        out.println(" realName = " + realName);
-        out.println(" keyLength = " + keyLength);
-        out.println(" ivLength = " + ivLength);
-        out.println(" cipher.alg = " + cipher == null ? null : cipher.getAlgorithm());
-        out.println(" cipher.blockSize = " + cipher == null ? null : cipher.getBlockSize());
-        out.println(" encryptMode = " + encryptMode);
-        out.println(" cipherInited = " + cipherInited);
-        out.println(" key.length = " + (key == null ? 0 : key.length));
-        out.println(" iv.length = " + (realIV == null ? 0 : realIV.length));
-        out.println(" padding = " + padding);
+        out.println(this.toString() + ' ' + header +
+                    "\n" +
+                    " name = " + name +
+                    " cryptoBase = " + cryptoBase +
+                    " cryptoVersion = " + cryptoVersion +
+                    " cryptoMode = " + cryptoMode +
+                    " padding_type = " + paddingType +
+                    " realName = " + realName +
+                    " keyLength = " + keyLength +
+                    " ivLength = " + ivLength +
+                    "\n" +
+                    " cipher.alg = " + (cipher == null ? null : cipher.getAlgorithm()) +
+                    " cipher.blockSize = " + (cipher == null ? null : cipher.getBlockSize()) +
+                    " encryptMode = " + encryptMode + " cipherInited = " + cipherInited +
+                    " key.length = " + (key == null ? 0 : key.length) +
+                    " iv.length = " + (realIV == null ? 0 : realIV.length) +
+                    " padding = " + padding);
     }
 
     @JRubyMethod(required = 1, visibility = Visibility.PRIVATE)

src/test/java/org/jruby/ext/openssl/CipherTest.java

@@ -63,12 +63,42 @@ private void doTestOsslToJsse() {
         assertEquals("CBC", alg.mode);
         assertEquals("DESede/CBC/PKCS5Padding", alg.getRealName());
 
+        alg = Cipher.Algorithm.osslToJava("DES3");
+        assertEquals("DES", alg.base);
+        assertEquals("EDE3", alg.version);
+        assertEquals("CBC", alg.mode);
+        assertEquals("DESede/CBC/PKCS5Padding", alg.getRealName());
+
         alg = Cipher.Algorithm.osslToJava("DES-EDE");
         assertEquals("DES", alg.base);
         assertEquals("EDE", alg.version);
+        assertEquals("ECB", alg.mode);
+        assertEquals("DESede/ECB/PKCS5Padding", alg.getRealName());
+
+        alg = Cipher.Algorithm.osslToJava("DES-EDE3");
+        assertEquals("DES", alg.base);
+        assertEquals("EDE3", alg.version);
+        assertEquals("ECB", alg.mode);
+        assertEquals("DESede/ECB/PKCS5Padding", alg.getRealName());
+
+        alg = Cipher.Algorithm.osslToJava("DES-EDE-CBC");
+        assertEquals("DES", alg.base);
+        assertEquals("EDE", alg.version);
         assertEquals("CBC", alg.mode);
         assertEquals("DESede/CBC/PKCS5Padding", alg.getRealName());
 
+        alg = Cipher.Algorithm.osslToJava("DES-EDE3-CBC");
+        assertEquals("DES", alg.base);
+        assertEquals("EDE3", alg.version);
+        assertEquals("CBC", alg.mode);
+        assertEquals("DESede/CBC/PKCS5Padding", alg.getRealName());
+
+        alg = Cipher.Algorithm.osslToJava("DES-EDE3-CFB");
+        assertEquals("DES", alg.base);
+        assertEquals("EDE3", alg.version);
+        assertEquals("CFB", alg.mode);
+        assertEquals("DESede/CFB/PKCS5Padding", alg.getRealName());
+
         alg = Cipher.Algorithm.osslToJava("DES-CFB");
         assertEquals("DES", alg.base);
         assertEquals(null, alg.version);
@@ -121,7 +151,7 @@ private void doTestOsslToJsse() {
         assertEquals("RC4", alg.getRealName());
 
         // keeps "invalid" modes :
-        
+
         alg = Cipher.Algorithm.osslToJava("DES-3X3");
         assertEquals("DES", alg.base);
         assertEquals(null, alg.version);

src/test/ruby/test_cipher.rb

@@ -75,7 +75,7 @@ def test_excludes_cfb1_ciphers # due no support in BC for CFB-1
     assert ! OpenSSL::Cipher.ciphers.find { |name| name =~ /CFB1/i }
   end if defined? JRUBY_VERSION
 
-  def test_encrypt_decrypt_des_ede3 # borrowed from OpenSSL suite
+  def test_encrypt_decrypt_des_ede3_cbc # borrowed from OpenSSL suite
     c1 = OpenSSL::Cipher::Cipher.new("DES-EDE3-CBC")
     c2 = OpenSSL::Cipher::DES.new(:EDE3, "CBC")
     key = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
@@ -95,6 +95,119 @@ def test_encrypt_decrypt_des_ede3 # borrowed from OpenSSL suite
     assert_equal(data, c2.update(s2) + c2.final, "decrypt")
   end
 
+  def test_des_key_len
+    cipher = OpenSSL::Cipher.new 'des'
+    assert_equal  8, cipher.key_len
+    cipher = OpenSSL::Cipher.new 'DES3'
+    assert_equal 24, cipher.key_len
+
+    cipher = OpenSSL::Cipher.new 'DES-CBC'
+    assert_equal  8, cipher.key_len
+    cipher = OpenSSL::Cipher.new 'des-ede3'
+    assert_equal 24, cipher.key_len
+
+    cipher = OpenSSL::Cipher.new 'des-ede'
+    assert_equal 16, cipher.key_len
+    cipher = OpenSSL::Cipher.new 'DES-EDE-CFB'
+    assert_equal 16, cipher.key_len
+  end
+
+  def test_des_iv_len
+    cipher = OpenSSL::Cipher.new 'des'
+    assert_equal 8, cipher.iv_len
+    cipher = OpenSSL::Cipher.new 'DES3'
+    assert_equal 8, cipher.iv_len
+
+    cipher = OpenSSL::Cipher.new 'DES-CBC'
+    assert_equal 8, cipher.iv_len
+    cipher = OpenSSL::Cipher.new 'des-ede3'
+    assert_equal 0, cipher.iv_len
+
+    cipher = OpenSSL::Cipher.new 'des-ede'
+    assert_equal 0, cipher.iv_len
+    cipher = OpenSSL::Cipher.new 'DES-EDE-CFB'
+    assert_equal 8, cipher.iv_len
+  end
+
+  @@test_encrypt_decrypt_des_variations = nil
+
+  def test_encrypt_decrypt_des_variations
+    key = "\0\0\0\0\0\0\0\0" * 3
+    iv =  "\0\0\0\0\0\0\0\0"
+    data = "JPMNT"
+
+    { # calculated on MRI
+      'des' => "b\x00<\xC0\x16\xAF\xDCd",
+      'des-cbc' => "b\x00<\xC0\x16\xAF\xDCd",
+      #'des-cfb' => "\xE0\x9ER\xCC\xD8",
+      #'des-ofb' => "\xE0\x9ER\xCC\xD8",
+      'des-ecb' => ".\x1E\xB3\x0E\xE0\xD2\x9DG",
+
+      'des-ede' => "@\x8B\x89}u\xB4\r\xA5",
+      'des-ede-cbc' => "\x99\x97\xBE(\xB9+f\xFA",
+      #'des-ede-cfb' => "l\x02?\x16\x1A",
+      #'des-ede-ofb' => "l\x02?\x16\x1A",
+      ##'des-ede-ecb' => RuntimeError: unsupported cipher algorithm (des-ede-ecb)
+
+      'des-ede3' => "\xDC\xD4\xF4\xBDmF\xC26", # actually ECB
+      'des-ede3-cbc' => "\x8D\xE6\x17\xD0\x97\rR\x8C",
+      #'des-ede3-cfb' => ",\x93^\xAD\x9C",
+      #'des-ede3-ofb' => ",\x93^\xAD\x9C",
+      ##'des-ede3-ecb' => unsupported cipher algorithm (des-ede3-ecb)
+      'des3' => "\x8D\xE6\x17\xD0\x97\rR\x8C"
+    }.each do |name, expected|
+        c = OpenSSL::Cipher.new name
+        c.encrypt
+        c.key = key
+        c.iv = iv
+        c.pkcs5_keyivgen(key, iv)
+
+        assert_equal expected, c.update(data) + c.final, "failed: #{name}"
+    end
+
+    cipher = OpenSSL::Cipher::Cipher.new("DES-EDE3")
+
+    cipher.encrypt.pkcs5_keyivgen(key, iv)
+    secret = cipher.update(data) + cipher.final
+    assert_equal "\xDC\xD4\xF4\xBDmF\xC26", secret
+
+    cipher.decrypt.pkcs5_keyivgen(key, iv)
+    assert_equal(data, cipher.update(secret) + cipher.final, "decrypt")
+
+    data = "sa jej lubim alebo moj bicykel"
+
+    cipher.encrypt.pkcs5_keyivgen(key, iv)
+    secret = cipher.update(data) + cipher.final
+    assert_equal "\xE9;\xDF\xEE/\x1D\xCB\xF9\xD1\xAF\xBC\xF0\x00\xA3\xDBsLxF2\xA4|\x11T\xD7&:\xD8\xF7\xA2\xD1b", secret
+
+    cipher.decrypt.pkcs5_keyivgen(key, iv)
+    assert_equal(data, cipher.update(secret) + cipher.final, "decrypt")
+
+    cipher.padding = 0
+    data = "hehehehemehehehe"
+
+    cipher.encrypt.pkcs5_keyivgen(key, iv)
+    secret = cipher.update(data) + cipher.final
+    assert_equal "v\r\xA4\xB3\x02\x18\xB5|A\x13\x87\xF1\xC0A\xC4U", secret
+
+    cipher.decrypt.pkcs5_keyivgen(key, iv)
+    assert_equal(data, cipher.update(secret) + cipher.final, "decrypt")
+
+    # assuming Cipher.ciphers not cached - re-run the tests with cache :
+    unless @@test_encrypt_decrypt_des_variations
+      @@test_encrypt_decrypt_des_variations = true
+      OpenSSL::Cipher.ciphers; test_encrypt_decrypt_des_variations
+    end
+  end
+
+  def test_another_encrypt_des_ede3
+    cipher = OpenSSL::Cipher.new('DES-EDE3')
+    cipher.encrypt # calculated on MRI :
+    cipher.key = "\x1F\xFF&\xA4k\x8F^\xC80\txq'S\x93\xD2\xE3A\xEDT\xDCs\xFD<=G\a\x8F=\x8FhE"
+    cipher.iv = "o\x15# \xD1\a\x90\xC7ZO\r[\xE2\x8F\v)# I6;\xE6\xB7h\xD3M\xDA\xA0\xD1\xDCy\xD2"
+    assert_equal "\xE1\x8DZ>MEq\xEF\x1A\xAC\xB1ab\x0Ea\x81", (cipher.update('sup3rs33kr3t') + cipher.final)
+  end
+
   def test_random
     cipher = OpenSSL::Cipher.new 'AES-128-OFB'