[SSHD-974] Clean up un-necessary sensitive data a.s.a.p.
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 @@
}
@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 @@
// 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.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 @@
@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 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 @@
@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 @@
*/
@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 @@
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 @@
}
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 @@
* @see #assertHierarchyTargetFolderExists(Path, LinkOption...) assertHierarchyTargetFolderExists
*/
protected Path createTempClassFolder() throws IOException {
- Path tmpDir = detectTargetFolder();
+ Path tmpDir = getTempTargetFolder();
return assertHierarchyTargetFolderExists(tmpDir.resolve(getClass().getSimpleName()));
}
