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 <lgoldst...@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()));
}
