You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by gn...@apache.org on 2015/03/30 15:10:54 UTC

[2/2] mina-sshd git commit: [SSHD-443] Improve supported cipher detection

[SSHD-443] Improve supported cipher detection

Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/82b0f82e
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/82b0f82e
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/82b0f82e

Branch: refs/heads/master
Commit: 82b0f82ece8deac40b48bbce69cab840a9e64f56
Parents: 1f1b88f
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Mar 30 15:00:04 2015 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Mon Mar 30 15:00:04 2015 +0200

----------------------------------------------------------------------
 .../apache/sshd/common/cipher/BaseCipher.java   |   7 +-
 .../sshd/common/cipher/BuiltinCiphers.java      | 143 +++++++++----------
 .../apache/sshd/common/cipher/CipherUtils.java  |  58 --------
 .../test/java/org/apache/sshd/CipherTest.java   |  28 ++--
 .../sshd/common/cipher/BuiltinCiphersTest.java  |  29 ++--
 5 files changed, 108 insertions(+), 157 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/82b0f82e/sshd-core/src/main/java/org/apache/sshd/common/cipher/BaseCipher.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/cipher/BaseCipher.java b/sshd-core/src/main/java/org/apache/sshd/common/cipher/BaseCipher.java
index 33c18b2..66a283c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/cipher/BaseCipher.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/cipher/BaseCipher.java
@@ -22,6 +22,7 @@ import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 
 import org.apache.sshd.common.Cipher;
+import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.util.SecurityUtils;
 
 /**
@@ -63,7 +64,7 @@ public class BaseCipher implements Cipher {
         }
         catch (Exception e) {
             cipher = null;
-            throw e;
+            throw new SshException("Unable to initialize cipher " + this, e);
         }
     }
 
@@ -80,4 +81,8 @@ public class BaseCipher implements Cipher {
         return data;
     }
 
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "[" + algorithm + "," + ivsize + "," + bsize + "," + transformation + "]";
+    }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/82b0f82e/sshd-core/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java b/sshd-core/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
index 78f1749..52cef83 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
@@ -22,7 +22,6 @@ package org.apache.sshd.common.cipher;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.sshd.common.Cipher;
 import org.apache.sshd.common.NamedFactory;
@@ -35,85 +34,52 @@ import org.apache.sshd.common.util.GenericUtils;
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public enum BuiltinCiphers implements NamedFactory<Cipher>, OptionalFeature {
-    none(Constants.NONE) {
+    none(Constants.NONE, 0, 0, "None", "None") {
         @Override
         public Cipher create() {
             return new CipherNone();
         }
     },
-    aes128cbc(Constants.AES128_CBC) {
+    aes128cbc(Constants.AES128_CBC, 16, 16, "AES", "AES/CBC/NoPadding"),
+    aes128ctr(Constants.AES128_CTR, 16, 16, "AES", "AES/CTR/NoPadding"),
+    aes192cbc(Constants.AES192_CBC, 16, 24, "AES", "AES/CBC/NoPadding"),
+    aes192ctr(Constants.AES192_CTR, 16, 24, "AES", "AES/CTR/NoPadding"),
+    aes256cbc(Constants.AES256_CBC, 16, 32, "AES", "AES/CBC/NoPadding"),
+    aes256ctr(Constants.AES256_CTR, 16, 32, "AES", "AES/CTR/NoPadding"),
+    arcfour128(Constants.ARCFOUR128, 8, 16, "ARCFOUR", "RC4") {
         @Override
         public Cipher create() {
-            return new BaseCipher(16, 16, "AES", "AES/CBC/NoPadding");
+            return new BaseRC4Cipher(getIVSize(), getBlockSize());
         }
     },
-    aes128ctr(Constants.AES128_CTR) {
+    arcfour256(Constants.ARCFOUR256, 8, 32, "ARCFOUR", "RC4") {
         @Override
         public Cipher create() {
-            return new BaseCipher(16, 16, "AES", "AES/CTR/NoPadding");
+            return new BaseRC4Cipher(getIVSize(), getBlockSize());
         }
     },
-    aes192cbc(Constants.AES192_CBC) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(16, 24, "AES", "AES/CBC/NoPadding");
-        }
-    },
-    aes192ctr(Constants.AES192_CTR) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(16, 24, "AES", "AES/CTR/NoPadding");
-        }
-    },
-    aes256cbc(Constants.AES256_CBC) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(16, 32, "AES", "AES/CBC/NoPadding");
-        }
-    },
-    aes256ctr(Constants.AES256_CTR) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(16, 32, "AES", "AES/CTR/NoPadding");
-        }
-    },
-    arcfour128(Constants.ARCFOUR128) {
-        @Override
-        public Cipher create() {
-            return new BaseRC4Cipher(8, 16);
-        }
-    },
-    arcfour256(Constants.ARCFOUR256) {
-        @Override
-        public Cipher create() {
-            return new BaseRC4Cipher(8, 32);
-        }
-    },
-    blowfishcbc(Constants.BLOWFISH_CBC) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(8, 16, "Blowfish", "Blowfish/CBC/NoPadding");
-        }
-    },
-    tripledescbc(Constants.DES_CBC) {
-        @Override
-        public Cipher create() {
-            return new BaseCipher(8, 24, "DESede", "DESede/CBC/NoPadding");
-        }
-    };
+    blowfishcbc(Constants.BLOWFISH_CBC, 8, 16, "Blowfish", "Blowfish/CBC/NoPadding"),
+    tripledescbc(Constants.TRIPLE_DES_CBC, 8, 24, "DESede", "DESede/CBC/NoPadding");
 
     private final String factoryName;
+    private final int ivsize;
+    private final int blocksize;
+    private final String algorithm;
+    private final String transformation;
 
     @Override
     public final String getName() {
         return factoryName;
     }
 
-    BuiltinCiphers(String facName) {
-        factoryName = facName;
-    }
+    BuiltinCiphers(String factoryName, int ivsize, int blocksize, String algorithm, String transformation) {
+        this.factoryName = factoryName;
+        this.ivsize = ivsize;
+        this.blocksize = blocksize;
+        this.algorithm = algorithm;
+        this.transformation = transformation;
 
-    private final AtomicReference<Boolean> _supported = new AtomicReference<>(null);
+    }
 
     /**
      * @return {@code true} if the current JVM configuration supports this
@@ -122,22 +88,53 @@ public enum BuiltinCiphers implements NamedFactory<Cipher>, OptionalFeature {
      */
     @Override
     public boolean isSupported() {
-        Boolean value;
-        synchronized (_supported) {
-            if ((value = _supported.get()) == null) {
-                // see BaseBuilder#fillWithDefaultValues
-                try {
-                    Exception t = CipherUtils.checkSupported(create());
-                    value = t == null;
-                } catch (Exception e) {
-                    value = Boolean.FALSE;
-                }
-
-                _supported.set(value);
-            }
+        try {
+            int maxKeyLength = javax.crypto.Cipher.getMaxAllowedKeyLength(getAlgorithm());
+            return maxKeyLength >= (1l << getBlockSize());
+        } catch (Exception e) {
+            return false;
         }
+    }
+
+    /**
+     * Retrieves the size of the initialization vector
+     *
+     * @return
+     */
+    public int getIVSize() {
+        return ivsize;
+    }
 
-        return value;
+    /**
+     * Retrieves the block size for this cipher
+     *
+     * @return
+     */
+    public int getBlockSize() {
+        return blocksize;
+    }
+
+    /**
+     * Retrieves the algorithm for this cipher
+     *
+     * @return
+     */
+    public String getAlgorithm() {
+        return algorithm;
+    }
+
+    /**
+     * Retrieves the algorithm for this cipher
+     *
+     * @return
+     */
+    public String getTransformation() {
+        return transformation;
+    }
+
+    @Override
+    public Cipher create() {
+        return new BaseCipher(getIVSize(), getBlockSize(), getAlgorithm(), getTransformation());
     }
 
     public static final Set<BuiltinCiphers> VALUES =
@@ -206,6 +203,6 @@ public enum BuiltinCiphers implements NamedFactory<Cipher>, OptionalFeature {
         public static final String ARCFOUR128 = "arcfour128";
         public static final String ARCFOUR256 = "arcfour256";
         public static final String BLOWFISH_CBC = "blowfish-cbc";
-        public static final String DES_CBC = "3des-cbc";
+        public static final String TRIPLE_DES_CBC = "3des-cbc";
     }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/82b0f82e/sshd-core/src/main/java/org/apache/sshd/common/cipher/CipherUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/cipher/CipherUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/cipher/CipherUtils.java
deleted file mode 100644
index 45070f6..0000000
--- a/sshd-core/src/main/java/org/apache/sshd/common/cipher/CipherUtils.java
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.apache.sshd.common.cipher;
-
-import org.apache.sshd.common.Cipher;
-import org.apache.sshd.common.Factory;
-
-/**
- * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
- */
-public class CipherUtils {
-    /**
-     * Checks if the provided cipher factory instance generates a cipher
-     * that is supported by the JCE
-     * @param f The cipher factory instance
-     * @return The {@link Exception} that the {@link Cipher#init(org.apache.sshd.common.Cipher.Mode, byte[], byte[])}
-     * call has thrown - {@code null} if successful (i.e., cipher supported)
-     * @see <A HREF="http://www.oracle.com/technetwork/java/javase/downloads/">Java Cryptography Extension (JCE)</A>
-     */
-    public static Exception checkSupported(Factory<? extends Cipher> f) {
-        return checkSupported(f.create());
-    }
-
-    /**
-     * Checks if the provided cipher is supported by the JCE
-     * @param c The {@link Cipher} to be checked
-     * @return The {@link Exception} that the {@link Cipher#init(org.apache.sshd.common.Cipher.Mode, byte[], byte[])}
-     * call has thrown - {@code null} if successful (i.e., cipher supported)
-     * @see <A HREF="http://www.oracle.com/technetwork/java/javase/downloads/">Java Cryptography Extension (JCE)</A>
-     */
-    public static Exception checkSupported(Cipher c) {
-        try {
-            byte[] key=new byte[c.getBlockSize()];
-            byte[] iv=new byte[c.getIVSize()];
-            c.init(Cipher.Mode.Encrypt, key, iv);
-            return null;
-        } catch(Exception e) {
-            return e;
-        }
-    }
-}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/82b0f82e/sshd-core/src/test/java/org/apache/sshd/CipherTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/CipherTest.java b/sshd-core/src/test/java/org/apache/sshd/CipherTest.java
index 9f9fa74..faa2c1f 100644
--- a/sshd-core/src/test/java/org/apache/sshd/CipherTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/CipherTest.java
@@ -28,7 +28,6 @@ import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.Random;
 import org.apache.sshd.common.cipher.BuiltinCiphers;
 import org.apache.sshd.common.random.BouncyCastleRandom;
-import org.apache.sshd.common.util.SecurityUtils;
 import org.apache.sshd.util.BaseTest;
 import org.apache.sshd.util.BogusPasswordAuthenticator;
 import org.apache.sshd.util.EchoShellFactory;
@@ -52,13 +51,17 @@ public class CipherTest extends BaseTest {
 
     @Test
     public void testAES128CBC() throws Exception {
-        setUp(BuiltinCiphers.aes128cbc);
-        runTest();
+        if (BuiltinCiphers.aes128cbc.isSupported()
+                && checkCipher(com.jcraft.jsch.jce.AES128CBC.class.getName())) {
+            setUp(BuiltinCiphers.aes128cbc);
+            runTest();
+        }
     }
 
     @Test
     public void testAES192CBC() throws Exception {
-        if (SecurityUtils.isBouncyCastleRegistered() && checkCipher(com.jcraft.jsch.jce.AES192CBC.class.getName())) {
+        if (BuiltinCiphers.aes192cbc.isSupported()
+                && checkCipher(com.jcraft.jsch.jce.AES192CBC.class.getName())) {
             setUp(BuiltinCiphers.aes192cbc);
             runTest();
         }
@@ -66,7 +69,8 @@ public class CipherTest extends BaseTest {
 
     @Test
     public void testAES256CBC() throws Exception {
-        if (SecurityUtils.isBouncyCastleRegistered() && checkCipher(com.jcraft.jsch.jce.AES256CBC.class.getName())) {
+        if (BuiltinCiphers.aes256cbc.isSupported()
+                && checkCipher(com.jcraft.jsch.jce.AES256CBC.class.getName())) {
             setUp(BuiltinCiphers.aes256cbc);
             runTest();
         }
@@ -74,14 +78,20 @@ public class CipherTest extends BaseTest {
 
     @Test
     public void testBlowfishCBC() throws Exception {
-        setUp(BuiltinCiphers.blowfishcbc);
-        runTest();
+        if (BuiltinCiphers.blowfishcbc.isSupported()
+                && checkCipher(com.jcraft.jsch.jce.BlowfishCBC.class.getName())) {
+            setUp(BuiltinCiphers.blowfishcbc);
+            runTest();
+        }
     }
 
     @Test
     public void testTripleDESCBC() throws Exception {
-        setUp(BuiltinCiphers.tripledescbc);
-        runTest();
+        if (BuiltinCiphers.tripledescbc.isSupported()
+                && checkCipher(com.jcraft.jsch.jce.TripleDESCBC.class.getName())) {
+            setUp(BuiltinCiphers.tripledescbc);
+            runTest();
+        }
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/82b0f82e/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java b/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
index 70d9d34..e90e64a 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
@@ -93,20 +93,17 @@ public class BuiltinCiphersTest extends BaseTest {
         Assert.assertEquals("Incomplete coverage", BuiltinCiphers.VALUES, avail);
     }
 
-//    @Test
-//    public void testFromCipher() {
-//        for (BuiltinCiphers expected : BuiltinCiphers.VALUES) {
-//            if (!expected.isSupported()) {
-//                System.out.append("Skip unsupported cipher: ").println(expected);
-//                continue;
-//            }
-//
-//            NamedFactory<Cipher>    factory=expected;
-//            Cipher                  cipher=factory.create();
-//            assertObjectInstanceOf(expected.name() + " - mismatched cipher type", expected.getCipherType(), cipher);
-//
-//            BuiltinCiphers  actual=BuiltinCiphers.fromCipher(cipher);
-//            Assert.assertSame(expected.getName() + " - mismatched enum values", expected, actual);
-//        }
-//    }
+    @Test
+    public void testSupportedCipher() throws Exception {
+        for (BuiltinCiphers expected : BuiltinCiphers.VALUES) {
+            if (!expected.isSupported()) {
+                System.out.append("Skip unsupported cipher: ").println(expected);
+                continue;
+            }
+            Cipher cipher = expected.create();
+            byte[] key = new byte[cipher.getBlockSize()];
+            byte[] iv = new byte[cipher.getIVSize()];
+            cipher.init(Cipher.Mode.Encrypt, key, iv);
+        }
+    }
 }