You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2020/05/04 16:41:59 UTC

[mina-sshd] branch master updated (160703e -> a3a1b83)

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.


    from 160703e  [SSHD-978] Updated source formatter plugin configuration
     new e2a67dd  [SSHD-974] Clean up un-necessary sensitive data a.s.a.p.
     new a3a1b83  [SSHD-978] Update import statements sort plugin configuration

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 pom.xml                                            | 19 ++++--
 .../keys/loader/AESPrivateKeyObfuscator.java       |  3 +-
 .../keys/loader/AbstractPrivateKeyObfuscator.java  |  2 +-
 .../openssh/OpenSSHKeyPairResourceWriter.java      | 76 ++++++++++++----------
 .../openssh/OpenSSHKeyPairResourceWriterTest.java  | 19 ++++--
 .../apache/sshd/util/test/JUnitTestSupport.java    |  2 +-
 6 files changed, 71 insertions(+), 50 deletions(-)


[mina-sshd] 01/02: [SSHD-974] Clean up un-necessary sensitive data a.s.a.p.

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit e2a67dd7e97429bcbe09da6d43ee826647e4df08
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon May 4 19:33:23 2020 +0300

    [SSHD-974] Clean up un-necessary sensitive data a.s.a.p.
---
 .../keys/loader/AESPrivateKeyObfuscator.java       |  3 +-
 .../keys/loader/AbstractPrivateKeyObfuscator.java  |  2 +-
 .../openssh/OpenSSHKeyPairResourceWriter.java      | 76 ++++++++++++----------
 .../openssh/OpenSSHKeyPairResourceWriterTest.java  | 19 ++++--
 .../apache/sshd/util/test/JUnitTestSupport.java    |  2 +-
 5 files changed, 59 insertions(+), 43 deletions(-)

diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AESPrivateKeyObfuscator.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AESPrivateKeyObfuscator.java
index 8ba47ce..08a24bb 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AESPrivateKeyObfuscator.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AESPrivateKeyObfuscator.java
@@ -61,7 +61,8 @@ public class AESPrivateKeyObfuscator extends AbstractPrivateKeyObfuscator {
     }
 
     @Override
-    protected int resolveInitializationVectorLength(PrivateKeyEncryptionContext encContext) throws GeneralSecurityException {
+    protected int resolveInitializationVectorLength(PrivateKeyEncryptionContext encContext)
+            throws GeneralSecurityException {
         int keyLength = resolveKeyLength(encContext);
         CipherInformation ci = resolveCipherInformation(keyLength, encContext.getCipherMode());
         if (ci == null) {
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AbstractPrivateKeyObfuscator.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AbstractPrivateKeyObfuscator.java
index 57ff3e3..2bfc970 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AbstractPrivateKeyObfuscator.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/AbstractPrivateKeyObfuscator.java
@@ -93,7 +93,7 @@ public abstract class AbstractPrivateKeyObfuscator implements PrivateKeyObfuscat
     // see http://www.ict.griffith.edu.au/anthony/info/crypto/openssl.hints (Password to Encryption Key section)
     // see http://openssl.6102.n7.nabble.com/DES-EDE3-CBC-technical-details-td24883.html
     protected byte[] deriveEncryptionKey(PrivateKeyEncryptionContext encContext, int outputKeyLength)
-            throws GeneralSecurityException {
+            throws IOException, GeneralSecurityException {
         Objects.requireNonNull(encContext, "No encryption context");
         ValidateUtils.checkNotNullAndNotEmpty(encContext.getCipherName(), "No cipher name");
         ValidateUtils.checkNotNullAndNotEmpty(encContext.getCipherType(), "No cipher type");
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriter.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriter.java
index 6251778..dd91e85 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriter.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriter.java
@@ -51,7 +51,6 @@ import org.apache.sshd.common.config.keys.loader.openssh.kdf.BCrypt;
 import org.apache.sshd.common.config.keys.loader.openssh.kdf.BCryptKdfOptions;
 import org.apache.sshd.common.config.keys.writer.KeyPairResourceWriter;
 import org.apache.sshd.common.util.GenericUtils;
-import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.common.util.io.SecureByteArrayOutputStream;
 
 /**
@@ -75,9 +74,9 @@ public class OpenSSHKeyPairResourceWriter implements KeyPairResourceWriter<OpenS
     @Override
     public void writePrivateKey(KeyPair key, String comment, OpenSSHKeyEncryptionContext options, OutputStream out)
             throws IOException, GeneralSecurityException {
-        ValidateUtils.checkNotNull(key, "Cannot write null key");
+        Objects.requireNonNull(key, "Cannot write null key");
         String keyType = KeyUtils.getKeyType(key);
-        if (keyType == null) {
+        if (GenericUtils.isEmpty(keyType)) {
             throw new GeneralSecurityException("Unsupported key: " + key.getClass().getName());
         }
         OpenSSHKeyEncryptionContext opt = determineEncryption(options);
@@ -188,16 +187,19 @@ public class OpenSSHKeyPairResourceWriter implements KeyPairResourceWriter<OpenS
     public static void write(OutputStream out, byte[] bytes, int lineLength) throws IOException {
         byte[] encoded = Base64.getEncoder().encode(bytes);
         Arrays.fill(bytes, (byte) 0);
-        int last = encoded.length;
-        for (int i = 0; i < last; i += lineLength) {
-            if (i + lineLength <= last) {
-                out.write(encoded, i, lineLength);
-            } else {
-                out.write(encoded, i, last - i);
+        try {
+            int last = encoded.length;
+            for (int i = 0; i < last; i += lineLength) {
+                if ((i + lineLength) <= last) {
+                    out.write(encoded, i, lineLength);
+                } else {
+                    out.write(encoded, i, last - i);
+                }
+                out.write('\n');
             }
-            out.write('\n');
+        } finally {
+            Arrays.fill(encoded, (byte) 0);
         }
-        Arrays.fill(encoded, (byte) 0);
     }
 
     /**
@@ -209,23 +211,24 @@ public class OpenSSHKeyPairResourceWriter implements KeyPairResourceWriter<OpenS
     @Override
     public void writePublicKey(PublicKey key, String comment, OutputStream out)
             throws IOException, GeneralSecurityException {
-        StringBuilder b = new StringBuilder();
+        StringBuilder b = new StringBuilder(82);
         PublicKeyEntry.appendPublicKeyEntry(b, key);
-        // Append first line of comment
-        if (comment != null) {
-            String line = firstLine(comment);
-            if (line != null && !line.isEmpty()) {
-                b.append(' ').append(line);
-            }
+        // Append first line of comment - if available
+        String line = firstLine(comment);
+        if (GenericUtils.isNotEmpty(line)) {
+            b.append(' ').append(line);
         }
         write(out, b.toString());
     }
 
     public static String firstLine(String text) {
-        Matcher m = VERTICALSPACE.matcher(text);
-        if (m.find()) {
-            return text.substring(0, m.start());
+        if (GenericUtils.isNotEmpty(text)) {
+            Matcher m = VERTICALSPACE.matcher(text);
+            if (m.find()) {
+                return text.substring(0, m.start()).trim();
+            }
         }
+
         return text;
     }
 
@@ -268,37 +271,38 @@ public class OpenSSHKeyPairResourceWriter implements KeyPairResourceWriter<OpenS
          */
         @Override
         protected byte[] deriveEncryptionKey(PrivateKeyEncryptionContext context, int keyLength)
-                throws GeneralSecurityException {
+                throws IOException, GeneralSecurityException {
             byte[] iv = context.getInitVector();
             if (iv == null) {
                 iv = generateInitializationVector(context);
             }
-            byte[] kdfOutput = new byte[keyLength + iv.length];
+
             byte[] salt = new byte[BCRYPT_SALT_LENGTH];
-            BCrypt bcrypt = new BCrypt();
             SecureRandom random = new SecureRandom();
             random.nextBytes(salt);
-            int rounds = options.getKdfRounds();
-            byte[] pwd = null;
-            byte[] result = null;
+
+            byte[] kdfOutput = new byte[keyLength + iv.length];
+            BCrypt bcrypt = new BCrypt();
             // "kdf" collects the salt and number of rounds; not sensitive data.
             try (ByteArrayOutputStream kdf = new ByteArrayOutputStream()) {
-                pwd = convert(options.getPassword());
-                bcrypt.pbkdf(pwd, salt, rounds, kdfOutput);
+                int rounds = options.getKdfRounds();
+                byte[] pwd = convert(options.getPassword());
+                try {
+                    bcrypt.pbkdf(pwd, salt, rounds, kdfOutput);
+                } finally {
+                    if (pwd != null) {
+                        Arrays.fill(pwd, (byte) 0);
+                    }
+                }
+
                 KeyEntryResolver.writeRLEBytes(kdf, salt);
                 KeyEntryResolver.encodeInt(kdf, rounds);
                 kdfOptions = kdf.toByteArray();
                 context.setInitVector(Arrays.copyOfRange(kdfOutput, keyLength, kdfOutput.length));
-                result = Arrays.copyOf(kdfOutput, keyLength);
-            } catch (IOException impossible) {
-                // Never occurs with a ByteArrayOutputStream
+                return Arrays.copyOf(kdfOutput, keyLength);
             } finally {
                 Arrays.fill(kdfOutput, (byte) 0); // Contains the IV at the end
-                if (pwd != null) {
-                    Arrays.fill(pwd, (byte) 0);
-                }
             }
-            return result;
         }
 
         protected byte[] convert(String password) {
diff --git a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
index 1cce6d6..050615a 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
@@ -118,9 +118,9 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
         if (data.provider == null) {
             generator = KeyPairGenerator.getInstance(data.algorithm);
         } else {
-            generator = KeyPairGenerator.getInstance(data.algorithm,
-                    data.provider);
+            generator = KeyPairGenerator.getInstance(data.algorithm, data.provider);
         }
+
         if (data.spec != null) {
             generator.initialize(data.spec);
         } else {
@@ -331,8 +331,19 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     }
 
     private Path getTemporaryOutputFile() throws IOException {
-        Path dir = assertHierarchyTargetFolderExists(getTempTargetFolder());
-        return dir.resolve(getCurrentTestName());
+        Path dir = createTempClassFolder();
+        String testName = getCurrentTestName();
+        int pos = testName.indexOf('[');
+        Path file;
+        if (pos > 0) {
+            String baseName = testName.substring(0, pos);
+            String paramName = testName.substring(pos + 1, testName.length() - 1);
+            file = dir.resolve(baseName + "-" + paramName.replace('(', '-').replace(")", "").trim());
+        } else {
+            file = dir.resolve(testName);
+        }
+        Files.deleteIfExists(file);
+        return file;
     }
 
     @SuppressWarnings("checkstyle:VisibilityModifier")
diff --git a/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java b/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
index 55537e0..6db9657 100644
--- a/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
+++ b/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
@@ -189,7 +189,7 @@ public abstract class JUnitTestSupport extends Assert {
      * @see                #assertHierarchyTargetFolderExists(Path, LinkOption...) assertHierarchyTargetFolderExists
      */
     protected Path createTempClassFolder() throws IOException {
-        Path tmpDir = detectTargetFolder();
+        Path tmpDir = getTempTargetFolder();
         return assertHierarchyTargetFolderExists(tmpDir.resolve(getClass().getSimpleName()));
     }
 


[mina-sshd] 02/02: [SSHD-978] Update import statements sort plugin configuration

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit a3a1b83728866e35fe7bff1ff8d5f3180ba39e90
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon May 4 19:35:42 2020 +0300

    [SSHD-978] Update import statements sort plugin configuration
---
 pom.xml | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/pom.xml b/pom.xml
index 5ae7540..9f4bb97 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1039,6 +1039,18 @@
                         <lineEnding>LF</lineEnding>
                     </configuration>
                 </plugin>
+                <plugin>
+                    <groupId>net.revelc.code</groupId>
+                    <artifactId>impsort-maven-plugin</artifactId>
+                    <version>1.4.1</version>
+                    <configuration>
+                        <lineEnding>LF</lineEnding>
+                        <groups>java.,javax.,org.w3c.,org.xml.,junit.</groups>
+                        <removeUnused>true</removeUnused>
+                        <staticAfter>true</staticAfter>
+                        <staticGroups>java.,javax.,org.w3c.,org.xml.,junit.</staticGroups>
+                    </configuration>
+                </plugin>
             </plugins>
         </pluginManagement>
 
@@ -1094,13 +1106,6 @@
             <plugin>
                 <groupId>net.revelc.code</groupId>
                 <artifactId>impsort-maven-plugin</artifactId>
-                <version>1.3.2</version>
-                <configuration>
-                    <groups>java.,javax.,org.w3c.,org.xml.,junit.</groups>
-                    <removeUnused>true</removeUnused>
-                    <staticAfter>true</staticAfter>
-                    <staticGroups>java.,javax.,org.w3c.,org.xml.,junit.</staticGroups>
-                </configuration>
                 <executions>
                     <execution>
                         <id>sort-imports</id>