From 2961e7424bb0e4ce43f849150b67ca09151b7767 Mon Sep 17 00:00:00 2001 From: Dave Golombek Date: Fri, 16 Feb 2018 09:10:10 -0800 Subject: [PATCH] Don't use padding for streaming cipher modes OFB, CFB[8], CTR, and GCM cipher modes don't require padding, since they act in a streaming manner, working byte-by-byte. Adding padding to them makes the output incompatible with MRI, and unable to be decrypted with it (and OpenSSL, underneath it). GCM is added to NO_PADDING_BLOCK_MODES despite not being in KNOWN_BLOCK_MODES to keep backward compatibility in getPaddingType. I'm happy removing it if others agree, since there shouldn't be any way for it to be supported currently. Fixes #13 --- src/main/java/org/jruby/ext/openssl/Cipher.java | 8 +++++--- .../java/org/jruby/ext/openssl/CipherTest.java | 17 ++++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/Cipher.java b/src/main/java/org/jruby/ext/openssl/Cipher.java index 621e64a7..795ddd7d 100644 --- a/src/main/java/org/jruby/ext/openssl/Cipher.java +++ b/src/main/java/org/jruby/ext/openssl/Cipher.java @@ -261,6 +261,10 @@ public static final class Algorithm { KNOWN_BLOCK_MODES.add("NONE"); // valid to pass into JCE } + // Subset of KNOWN_BLOCK_MODES that do not require padding (and shouldn't have it by default). + private static final List NO_PADDING_BLOCK_MODES = Arrays.asList( + "CFB", "CFB8", "OFB", "CTR", "GCM"); + // Ruby to Java name String (or FALSE) static final HashMap supportedCiphers = new LinkedHashMap(120, 1); // we're _marking_ unsupported keys with a Boolean.FALSE mapping @@ -557,12 +561,10 @@ 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 ) { - if ( "GCM".equalsIgnoreCase(cryptoMode) ) { + if ( NO_PADDING_BLOCK_MODES.contains(cryptoMode) ) { return "NoPadding"; } return defaultPadding; diff --git a/src/test/java/org/jruby/ext/openssl/CipherTest.java b/src/test/java/org/jruby/ext/openssl/CipherTest.java index d739a7c0..51d7f81b 100644 --- a/src/test/java/org/jruby/ext/openssl/CipherTest.java +++ b/src/test/java/org/jruby/ext/openssl/CipherTest.java @@ -1,4 +1,3 @@ - package org.jruby.ext.openssl; import org.junit.*; @@ -97,13 +96,13 @@ private void doTestOsslToJsse() { assertEquals("DES", alg.base); assertEquals("EDE3", alg.version); assertEquals("CFB", alg.mode); - assertEquals("DESede/CFB/PKCS5Padding", alg.getRealName()); + assertEquals("DESede/CFB/NoPadding", alg.getRealName()); alg = Cipher.Algorithm.osslToJava("DES-CFB"); assertEquals("DES", alg.base); assertEquals(null, alg.version); assertEquals("CFB", alg.mode); - assertEquals("DES/CFB/PKCS5Padding", alg.getRealName()); + assertEquals("DES/CFB/NoPadding", alg.getRealName()); alg = Cipher.Algorithm.osslToJava("DES3"); assertEquals("DES", alg.base); @@ -129,6 +128,18 @@ private void doTestOsslToJsse() { assertEquals("PKCS5Padding", alg.getPadding()); assertEquals("AES/CBC/PKCS5Padding", alg.getRealName()); + alg = Cipher.Algorithm.osslToJava("AES-256-OFB"); + assertEquals("AES", alg.base); + assertEquals("256", alg.version); + assertEquals("OFB", alg.mode); + assertEquals("AES/OFB/NoPadding", alg.getRealName()); + + alg = Cipher.Algorithm.osslToJava("AES-256-CTR"); + assertEquals("AES", alg.base); + assertEquals("256", alg.version); + assertEquals("CTR", alg.mode); + assertEquals("AES/CTR/NoPadding", alg.getRealName()); + alg = Cipher.Algorithm.osslToJava("AES-256-CBC-HMAC-SHA1"); assertEquals("AES", alg.base); assertEquals("CBC", alg.mode);