[SSHD-984] Fixed some minor coding issues to make OpenSSHKeyPairResourceWriter conform to SSHD style
diff --git a/CHANGES.md b/CHANGES.md
index 7022e45..0fbc103 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -33,4 +33,6 @@
* [SSHD-977](https://issues.apache.org/jira/browse/SSHD-977) - Apply consistent logging policy to caught exceptions
-* [SSHD-660](https://issues.apache.org/jira/browse/SSHD-660) - Added support for server-side signed certificate keys
\ No newline at end of file
+* [SSHD-660](https://issues.apache.org/jira/browse/SSHD-660) - Added support for server-side signed certificate keys
+
+* [SSHD-984](https://issues.apache.org/jira/browse/SSHD-984) - Utility method to export KeyPair in OpenSSH format
\ No newline at end of file
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PrivateKeyEntryDecoder.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PrivateKeyEntryDecoder.java
index 4deccc8..0e61acc 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PrivateKeyEntryDecoder.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PrivateKeyEntryDecoder.java
@@ -22,7 +22,6 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.OutputStream;
import java.io.StreamCorruptedException;
import java.security.GeneralSecurityException;
import java.security.PrivateKey;
@@ -122,16 +121,15 @@
throws IOException, GeneralSecurityException;
/**
- * Encodes the {@link PrivateKey} using the {@code OpenSSH} format - same one
- * used by the {@code decodePublicKey} method(s)
+ * Encodes the {@link PrivateKey} using the {@code OpenSSH} format - same one used by the {@code decodePublicKey}
+ * method(s)
*
- * @param s The {@link SecureByteArrayOutputStream} to write the data to.
- * @param key The {@link PrivateKey} - may not be {@code null}
- * @param pubKey The {@link PublicKey} belonging to the private key - must be
- * non-{@code null} if {@link #isPublicKeyRecoverySupported()
- * public key recovery} is not supported
- * @return The key type value - one of the {@link #getSupportedKeyTypes()} or
- * {@code null} if encoding not supported
+ * @param s The {@link SecureByteArrayOutputStream} to write the data to.
+ * @param key The {@link PrivateKey} - may not be {@code null}
+ * @param pubKey The {@link PublicKey} belonging to the private key - must be non-{@code null} if
+ * {@link #isPublicKeyRecoverySupported() public key recovery} is not supported
+ * @return The key type value - one of the {@link #getSupportedKeyTypes()} or {@code null} if encoding
+ * not supported
* @throws IOException If failed to generate the encoding
*/
default String encodePrivateKey(SecureByteArrayOutputStream s, PRV key, PUB pubKey) throws IOException {
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/KeyPairResourceWriter.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/KeyPairResourceWriter.java
index c1be690..9201aea 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/KeyPairResourceWriter.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/KeyPairResourceWriter.java
@@ -26,59 +26,59 @@
import java.security.PublicKey;
import org.apache.sshd.common.config.keys.loader.PrivateKeyEncryptionContext;
-import org.apache.sshd.common.util.io.SecureByteArrayOutputStream;
/**
- * A {@code KeyPairResourceWriter} can serialize keys to an external
- * representation.
+ * A {@code KeyPairResourceWriter} can serialize keys to an external representation.
*
- * @param <OPTIONS> The type of {@link PrivateKeyEncryptionContext} to use with
- * this {@code KeyPairResourceWriter}.
+ * @param <OPTIONS> The type of {@link PrivateKeyEncryptionContext} to use with this {@code KeyPairResourceWriter}.
*/
public interface KeyPairResourceWriter<OPTIONS extends PrivateKeyEncryptionContext> {
-
/**
- * Writes a serialization of a private key from a given {@link KeyPair} to a
- * given {@link SecureByteArrayOutputStream}.
+ * Writes a serialization of a private key from a given {@link KeyPair} to a given {@link OutputStream}.
*
- * @param key to write the private key of
- * @param comment to write with the private key
- * @param options for writing the key; may be {@code null} if no encryption is
- * wanted. The caller is responsible for
- * {@link PrivateKeyEncryptionContext#clear() clearing} the
- * options when no longer needed. If the passphrase obtained from
- * the context is {@code null} or an empty array (length zero or
- * containing only whitespace), the key is written unencrypted.
- * @param out to write to
- * @return a byte array containing the serialized private key data
- * @throws GeneralSecurityException if the key is inconsistent or unknown, or
- * the encryption specified cannot be applied
+ * @param key to write the private key of
+ * @param comment to write with the private key
+ * @param options for writing the key; may be {@code null} if no encryption is wanted. The caller
+ * is responsible for clearing the options when no longer needed. If the passphrase
+ * obtained from the context is {@code null} or an empty/blank string (length zero
+ * or containing only whitespace), the key is written unencrypted.
+ * @param out The {@link OutputStream} to write to - recommend using a
+ * {@code SecureByteArrayOutputStream} in order to reduce sensitive data exposure
+ * in memory
+ * @throws GeneralSecurityException if the key is inconsistent or unknown, or the encryption specified cannot be
+ * applied
* @throws IOException if the key cannot be written
*/
- void writePrivateKey(KeyPair key, String comment, OPTIONS options, SecureByteArrayOutputStream out)
+ void writePrivateKey(KeyPair key, String comment, OPTIONS options, OutputStream out)
throws IOException, GeneralSecurityException;
/**
- * Writes a serialization of a public key from a given {@link KeyPair} to a
- * given {@link OutputStream}.
+ * Writes a serialization of a public key from a given {@link KeyPair} to a given {@link OutputStream}.
*
- * @param key to write the public key of
- * @param comment to write with the public key
- * @param out to write to
+ * @param key to write the public key of
+ * @param comment to write with the public key
+ * @param out The {@link OutputStream} to write to - recommend using a
+ * {@code SecureByteArrayOutputStream} in order to reduce sensitive data exposure
+ * in memory
* @throws GeneralSecurityException if the key is unknown
* @throws IOException if the key cannot be written
*/
- void writePublicKey(KeyPair key, String comment, OutputStream out) throws IOException, GeneralSecurityException;
+ default void writePublicKey(KeyPair key, String comment, OutputStream out)
+ throws IOException, GeneralSecurityException {
+ writePublicKey(key.getPublic(), comment, out);
+ }
/**
- * Writes a serialization of a {@link PublicKey} to a given
- * {@link OutputStream}.
+ * Writes a serialization of a {@link PublicKey} to a given {@link OutputStream}.
*
- * @param key to write
- * @param comment to write with the key
- * @param out to write to
+ * @param key to write
+ * @param comment to write with the key
+ * @param out The {@link OutputStream} to write to - recommend using a
+ * {@code SecureByteArrayOutputStream} in order to reduce sensitive data exposure
+ * in memory
* @throws GeneralSecurityException if the key is unknown
* @throws IOException if the key cannot be written
*/
- void writePublicKey(PublicKey key, String comment, OutputStream out) throws IOException, GeneralSecurityException;
+ void writePublicKey(PublicKey key, String comment, OutputStream out)
+ throws IOException, GeneralSecurityException;
}
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyEncryptionContext.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyEncryptionContext.java
index 78313a6..477211a 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyEncryptionContext.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyEncryptionContext.java
@@ -19,28 +19,22 @@
package org.apache.sshd.common.config.keys.writer.openssh;
-import java.util.Arrays;
-
import org.apache.sshd.common.config.keys.loader.PrivateKeyEncryptionContext;
import org.apache.sshd.common.util.ValidateUtils;
/**
- * A {@link PrivateKeyEncryptionContext} for use with a
- * {@link OpenSSHKeyPairResourceWriter}.
+ * A {@link PrivateKeyEncryptionContext} for use with a {@link OpenSSHKeyPairResourceWriter}.
*/
public class OpenSSHKeyEncryptionContext extends PrivateKeyEncryptionContext {
/** Default number of bcrypt KDF rounds to apply. */
public static final int DEFAULT_KDF_ROUNDS = 16;
- private static final String AES = "AES";
-
- private char[] passphrase;
+ public static final String AES = "AES";
private int kdfRounds = DEFAULT_KDF_ROUNDS;
public OpenSSHKeyEncryptionContext() {
- super();
setCipherMode("CTR"); // Set default to CTR, as in OpenSSH
}
@@ -51,58 +45,11 @@
@Override
public void setCipherName(String value) {
- ValidateUtils.checkTrue(value != null && value.equalsIgnoreCase(AES),
+ ValidateUtils.checkTrue((value != null) && value.equalsIgnoreCase(AES),
"OpenSSHKeyEncryptionContext works only with AES encryption");
}
/**
- * {@inheritDoc}
- *
- * @deprecated Use {@link #getPassphrase()} instead
- */
- @Deprecated
- @Override
- public String getPassword() {
- return passphrase == null ? null : new String(passphrase);
- }
-
- /**
- * {@inheritDoc}
- *
- * @deprecated Use {@link #setPassphrase()} instead
- */
- @Deprecated
- @Override
- public void setPassword(String value) {
- setPassphrase(value.toCharArray());
- }
-
- /**
- * Retrieves a <em>copy</em> of the internally stored passphrase. The caller is
- * responsible for clearing the returned array when it is no longer needed.
- *
- * @return the passphrase
- */
- public char[] getPassphrase() {
- return passphrase == null ? null : passphrase.clone();
- }
-
- /**
- * Stores a <em>copy</em> of the passphrase. The caller is responsible for
- * eventually clearing the array passed as an argument, and for {@link #clear()
- * clearing} this {@link OpenSSHKeyEncryptionContext} once it is no longer
- * needed.
- *
- * @param passphrase to store
- */
- public void setPassphrase(char[] passphrase) {
- if (this.passphrase != null) {
- Arrays.fill(this.passphrase, '\000');
- }
- this.passphrase = passphrase == null ? null : passphrase.clone();
- }
-
- /**
* Retrieves the number of KDF rounds to apply.
*
* @return the default number of KDF rounds, >= {@link #DEFAULT_KDF_ROUNDS}
@@ -112,8 +59,7 @@
}
/**
- * Sets the number of KDF rounds to apply. If smaller than the
- * {@link #DEFAULT_KDF_ROUNDS}, set that default.
+ * Sets the number of KDF rounds to apply. If smaller than the {@link #DEFAULT_KDF_ROUNDS}, set that default.
*
* @param rounds number of rounds to apply
*/
@@ -122,26 +68,9 @@
}
/**
- * Retrieves the cipher's factory name.
- *
- * @return the name
+ * @return the cipher's factory name.
*/
protected String getCipherFactoryName() {
return getCipherName().toLowerCase() + getCipherType() + '-' + getCipherMode().toLowerCase();
}
-
- /**
- * Clears internal sensitive data (the password and the init vector).
- */
- public void clear() {
- if (this.passphrase != null) {
- Arrays.fill(this.passphrase, '\000');
- this.passphrase = null;
- }
- byte[] iv = getInitVector();
- if (iv != null) {
- Arrays.fill(iv, (byte) 0);
- }
- }
-
}
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 bc8ae46..6251778 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
@@ -48,28 +48,32 @@
import org.apache.sshd.common.config.keys.loader.openssh.OpenSSHKeyPairResourceParser;
import org.apache.sshd.common.config.keys.loader.openssh.OpenSSHParserContext;
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;
/**
- * A {@link KeyPairResourceWriter} for writing keys in the modern OpenSSH format, using
- * the OpenBSD bcrypt KDF for passphrase-protected encrypted private keys.
+ * A {@link KeyPairResourceWriter} for writing keys in the modern OpenSSH format, using the OpenBSD bcrypt KDF for
+ * passphrase-protected encrypted private keys.
*/
public class OpenSSHKeyPairResourceWriter implements KeyPairResourceWriter<OpenSSHKeyEncryptionContext> {
+ public static final String DASHES = "-----"; //$NON-NLS-1$
+
+ public static final int LINE_LENGTH = 70;
+
+ public static final OpenSSHKeyPairResourceWriter INSTANCE = new OpenSSHKeyPairResourceWriter();
+
private static final Pattern VERTICALSPACE = Pattern.compile("\\v"); //$NON-NLS-1$
- private static final String DASHES = "-----"; //$NON-NLS-1$
-
- private static final int LINE_LENGTH = 70;
-
public OpenSSHKeyPairResourceWriter() {
- super();
+ super();
}
@Override
- public void writePrivateKey(KeyPair key, String comment, OpenSSHKeyEncryptionContext options, SecureByteArrayOutputStream out)
+ public void writePrivateKey(KeyPair key, String comment, OpenSSHKeyEncryptionContext options, OutputStream out)
throws IOException, GeneralSecurityException {
ValidateUtils.checkNotNull(key, "Cannot write null key");
String keyType = KeyUtils.getKeyType(key);
@@ -94,22 +98,19 @@
}
byte[] privateBytes = encodePrivateKey(key, keyType, blockSize, comment);
String kdfName = OpenSSHParserContext.NONE_CIPHER;
- byte[] kdfOptions = new byte[0];
+ byte[] kdfOptions = GenericUtils.EMPTY_BYTE_ARRAY;
try (SecureByteArrayOutputStream bytes = new SecureByteArrayOutputStream()) {
write(bytes, OpenSSHKeyPairResourceParser.AUTH_MAGIC);
bytes.write(0);
if (opt != null) {
KeyEncryptor encryptor = new KeyEncryptor(opt);
opt.setPrivateKeyObfuscator(encryptor);
- try {
- byte[] encodedBytes = encryptor.applyPrivateKeyCipher(privateBytes, opt, true);
- Arrays.fill(privateBytes, (byte) 0);
- privateBytes = encodedBytes;
- kdfName = "bcrypt"; //$NON-NLS-1$
- kdfOptions = encryptor.getKdfOptions();
- } finally {
- opt.clear();
- }
+
+ byte[] encodedBytes = encryptor.applyPrivateKeyCipher(privateBytes, opt, true);
+ Arrays.fill(privateBytes, (byte) 0);
+ privateBytes = encodedBytes;
+ kdfName = BCryptKdfOptions.NAME;
+ kdfOptions = encryptor.getKdfOptions();
}
KeyEntryResolver.encodeString(bytes, cipherName);
KeyEntryResolver.encodeString(bytes, kdfName);
@@ -125,27 +126,23 @@
out.write('\n');
}
- private static OpenSSHKeyEncryptionContext determineEncryption(OpenSSHKeyEncryptionContext options) {
- if (options == null) {
+ public static OpenSSHKeyEncryptionContext determineEncryption(OpenSSHKeyEncryptionContext options) {
+ CharSequence password = (options == null) ? null : options.getPassword();
+ if (GenericUtils.isEmpty(password)) {
return null;
}
- char[] passphrase = options.getPassphrase();
- if (passphrase == null) {
- return null;
- }
- try {
- for (char ch : passphrase) {
- if (!Character.isWhitespace(ch)) {
- return options;
- }
+
+ for (int pos = 0, len = password.length(); pos < len; pos++) {
+ char ch = password.charAt(pos);
+ if (!Character.isWhitespace(ch)) {
+ return options;
}
- } finally {
- Arrays.fill(passphrase, '\000');
}
+
return null;
}
- private static byte[] encodePrivateKey(KeyPair key, String keyType, int blockSize, String comment)
+ public static byte[] encodePrivateKey(KeyPair key, String keyType, int blockSize, String comment)
throws IOException, GeneralSecurityException {
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
int check = new SecureRandom().nextInt();
@@ -153,8 +150,9 @@
KeyEntryResolver.encodeInt(out, check);
KeyEntryResolver.encodeString(out, keyType);
@SuppressWarnings("unchecked") // Problem with generics
- PrivateKeyEntryDecoder<PublicKey, PrivateKey> encoder = (PrivateKeyEntryDecoder<PublicKey, PrivateKey>) OpenSSHKeyPairResourceParser
- .getPrivateKeyEntryDecoder(keyType);
+ PrivateKeyEntryDecoder<PublicKey, PrivateKey> encoder
+ = (PrivateKeyEntryDecoder<PublicKey, PrivateKey>) OpenSSHKeyPairResourceParser
+ .getPrivateKeyEntryDecoder(keyType);
if (encoder.encodePrivateKey(out, key.getPrivate(), key.getPublic()) == null) {
throw new GeneralSecurityException("Cannot encode key of type " + keyType);
}
@@ -173,10 +171,11 @@
}
}
- private static byte[] encodePublicKey(PublicKey key, String keyType) throws IOException, GeneralSecurityException {
+ public static byte[] encodePublicKey(PublicKey key, String keyType)
+ throws IOException, GeneralSecurityException {
@SuppressWarnings("unchecked") // Problem with generics.
- PublicKeyEntryDecoder<PublicKey, ?> decoder = (PublicKeyEntryDecoder<PublicKey, ?>) KeyUtils
- .getPublicKeyEntryDecoder(keyType);
+ PublicKeyEntryDecoder<PublicKey, ?> decoder
+ = (PublicKeyEntryDecoder<PublicKey, ?>) KeyUtils.getPublicKeyEntryDecoder(keyType);
if (decoder == null) {
throw new GeneralSecurityException("Unknown key type: " + keyType);
}
@@ -186,7 +185,7 @@
}
}
- private static void write(OutputStream out, byte[] bytes, int lineLength) throws IOException {
+ 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;
@@ -204,22 +203,8 @@
/**
* {@inheritDoc}
*
- * Writes the public key in the single-line OpenSSH format "key-type pub-key
- * comment" without terminating line ending. If the comment has multiple lines,
- * only the first line is written.
- */
- @Override
- public void writePublicKey(KeyPair key, String comment, OutputStream out)
- throws IOException, GeneralSecurityException {
- writePublicKey(key.getPublic(), comment, out);
- }
-
- /**
- * {@inheritDoc}
- *
- * Writes the public key in the single-line OpenSSH format "key-type pub-key
- * comment" without terminating line ending. If the comment has multiple lines,
- * only the first line is written.
+ * Writes the public key in the single-line OpenSSH format "key-type pub-key comment" without terminating line
+ * ending. If the comment has multiple lines, only the first line is written.
*/
@Override
public void writePublicKey(PublicKey key, String comment, OutputStream out)
@@ -236,7 +221,7 @@
write(out, b.toString());
}
- private static String firstLine(String text) {
+ public static String firstLine(String text) {
Matcher m = VERTICALSPACE.matcher(text);
if (m.find()) {
return text.substring(0, m.start());
@@ -244,18 +229,17 @@
return text;
}
- private static void write(OutputStream out, String s) throws IOException {
+ public static void write(OutputStream out, String s) throws IOException {
out.write(s.getBytes(StandardCharsets.UTF_8));
}
/**
* A key encryptor for modern-style OpenSSH private keys using the bcrypt KDF.
*/
- private static class KeyEncryptor extends AESPrivateKeyObfuscator {
+ public static class KeyEncryptor extends AESPrivateKeyObfuscator {
+ public static final int BCRYPT_SALT_LENGTH = 16;
- private static final int BCRYPT_SALT_LENGTH = 16;
-
- private final OpenSSHKeyEncryptionContext options;
+ protected final OpenSSHKeyEncryptionContext options;
private byte[] kdfOptions;
@@ -265,8 +249,7 @@
/**
* Retrieves the KDF options used. Valid only after
- * {@link #deriveEncryptionKey(PrivateKeyEncryptionContext, int)} has been
- * called.
+ * {@link #deriveEncryptionKey(PrivateKeyEncryptionContext, int)} has been called.
*
* @return the number of KDF rounds applied
*/
@@ -275,13 +258,13 @@
}
/**
- * Derives an encryption key and set the IV on the {@code context} from the
- * passphase provided by the context using the OpenBSD {@link BCrypt} KDF.
+ * Derives an encryption key and set the IV on the {@code context} from the passphase provided by the context
+ * using the OpenBSD {@link BCrypt} KDF.
*
- * @param context for the encryption, provides the passphrase and transports
- * other encryption-related information including the IV
- * @param keyLength number of key bytes to generate
- * @return {@code keyLength} bytes to use as encryption key
+ * @param context for the encryption, provides the passphrase and transports other encryption-related
+ * information including the IV
+ * @param keyLength number of key bytes to generate
+ * @return {@code keyLength} bytes to use as encryption key
*/
@Override
protected byte[] deriveEncryptionKey(PrivateKeyEncryptionContext context, int keyLength)
@@ -300,7 +283,7 @@
byte[] result = null;
// "kdf" collects the salt and number of rounds; not sensitive data.
try (ByteArrayOutputStream kdf = new ByteArrayOutputStream()) {
- pwd = convert(options.getPassphrase());
+ pwd = convert(options.getPassword());
bcrypt.pbkdf(pwd, salt, rounds, kdfOutput);
KeyEntryResolver.writeRLEBytes(kdf, salt);
KeyEntryResolver.encodeInt(kdf, rounds);
@@ -318,17 +301,24 @@
return result;
}
- private byte[] convert(char[] pass) {
- if (pass == null) {
- return new byte[0];
+ protected byte[] convert(String password) {
+ if (GenericUtils.isEmpty(password)) {
+ return GenericUtils.EMPTY_BYTE_ARRAY;
}
- ByteBuffer bytes = StandardCharsets.UTF_8.encode(CharBuffer.wrap(pass));
+
+ char[] pass = password.toCharArray();
+ ByteBuffer bytes;
+ try {
+ bytes = StandardCharsets.UTF_8.encode(CharBuffer.wrap(pass));
+ } finally {
+ Arrays.fill(pass, '\0');
+ }
+
byte[] pwd = new byte[bytes.remaining()];
bytes.get(pwd);
if (bytes.hasArray()) {
Arrays.fill(bytes.array(), (byte) 0);
}
- Arrays.fill(pass, '\000');
return pwd;
}
}
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/io/SecureByteArrayOutputStream.java b/sshd-common/src/main/java/org/apache/sshd/common/util/io/SecureByteArrayOutputStream.java
index a1e9d44..3cd073f 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/io/SecureByteArrayOutputStream.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/io/SecureByteArrayOutputStream.java
@@ -23,8 +23,8 @@
import java.util.Arrays;
/**
- * A {@link ByteArrayOutputStream} that clears its internal buffer after
- * resizing and when it is {@link #close() closed}.
+ * A {@link ByteArrayOutputStream} that clears its internal buffer after resizing and when it is {@link #close()
+ * closed}.
*/
public final class SecureByteArrayOutputStream extends ByteArrayOutputStream {
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
index e9becf4..5e72064 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
@@ -130,7 +130,8 @@
}
@Override
- public String encodePrivateKey(SecureByteArrayOutputStream s, EdDSAPrivateKey key, EdDSAPublicKey pubKey) throws IOException {
+ public String encodePrivateKey(SecureByteArrayOutputStream s, EdDSAPrivateKey key, EdDSAPublicKey pubKey)
+ throws IOException {
Objects.requireNonNull(key, "No private key provided");
// ed25519 bernstein naming: pk .. public key, sk .. secret key
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 f52f054..1cce6d6 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
@@ -19,14 +19,6 @@
package org.apache.sshd.common.config.keys.writer.openssh;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.io.File;
-import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -34,6 +26,7 @@
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
@@ -43,9 +36,12 @@
import java.security.spec.ECGenParameterSpec;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.List;
import java.util.Objects;
+import net.i2p.crypto.eddsa.EdDSAKey;
+import net.i2p.crypto.eddsa.spec.EdDSAGenParameterSpec;
import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.KeyUtils;
@@ -53,86 +49,71 @@
import org.apache.sshd.common.util.io.SecureByteArrayOutputStream;
import org.apache.sshd.common.util.io.resource.PathResource;
import org.apache.sshd.common.util.security.SecurityUtils;
+import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory;
+import org.apache.sshd.util.test.JUnitTestSupport;
+import org.apache.sshd.util.test.NoIoTestCase;
import org.junit.Before;
+import org.junit.FixMethodOrder;
import org.junit.Rule;
import org.junit.Test;
+import org.junit.experimental.categories.Category;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
+import org.junit.runners.MethodSorters;
import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
+import org.junit.runners.Parameterized.UseParametersRunnerFactory;
-import net.i2p.crypto.eddsa.EdDSAKey;
-import net.i2p.crypto.eddsa.spec.EdDSAGenParameterSpec;
-
-@RunWith(Parameterized.class)
-public class OpenSSHKeyPairResourceWriterTest {
-
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+@RunWith(Parameterized.class) // see https://github.com/junit-team/junit/wiki/Parameterized-tests
+@UseParametersRunnerFactory(JUnit4ClassRunnerWithParametersFactory.class)
+@Category({ NoIoTestCase.class })
+public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
@Rule
public TemporaryFolder folder = new TemporaryFolder();
- private static class TestData {
- public final String algorithm;
+ private final TestData data;
+ private KeyPair testKey;
- public final String provider;
-
- public final int keySize;
-
- public final AlgorithmParameterSpec spec;
-
- public TestData(String algorithm, int keySize,
- AlgorithmParameterSpec spec) {
- this(algorithm, null, keySize, spec);
- }
-
- public TestData(String algorithm, String provider, int keySize,
- AlgorithmParameterSpec spec) {
- this.algorithm = algorithm;
- this.provider = provider;
- this.keySize = keySize;
- this.spec = spec;
- }
-
- @Override
- public String toString() {
- return algorithm + '-' + keySize
- + (provider == null ? "" : '(' + provider + ')');
- }
+ public OpenSSHKeyPairResourceWriterTest(TestData data) {
+ this.data = data;
}
@Parameters(name = "{0}")
- public static Object[] getParams() {
+ public static Collection<Object[]> parameters() {
List<TestData> result = new ArrayList<>();
result.add(new TestData("RSA", 1024, null));
result.add(new TestData("RSA", 2048, null));
result.add(new TestData("DSA", 1024, null));
- result.add(
- new TestData("ECDSA", 256,
- new ECGenParameterSpec("secp256r1")));
- result.add(
- new TestData("ECDSA", 384,
- new ECGenParameterSpec("secp384r1")));
- result.add(
- new TestData("ECDSA", 521,
- new ECGenParameterSpec("secp521r1")));
- result.add(new TestData("EDDSA", "EdDSA", 25519,
- new EdDSAGenParameterSpec("Ed25519")));
- // Note: BC also has an EDDSA provider, but that one returns
- // "Ed25519" as algorithm from its keys, while the one in
- // net.i2p.crypto.eddsa gives keys with "EDDSA" as algorithm.
- // sshd handles only the latter.
- return result.toArray();
+ if (SecurityUtils.isECCSupported()) {
+ result.add(
+ new TestData(
+ "ECDSA", 256,
+ new ECGenParameterSpec("secp256r1")));
+ result.add(
+ new TestData(
+ "ECDSA", 384,
+ new ECGenParameterSpec("secp384r1")));
+ result.add(
+ new TestData(
+ "ECDSA", 521,
+ new ECGenParameterSpec("secp521r1")));
+ }
+ if (SecurityUtils.isEDDSACurveSupported()) {
+ // Note: BC also has an EDDSA provider, but that one returns
+ // "Ed25519" as algorithm from its keys, while the one in
+ // net.i2p.crypto.eddsa gives keys with "EDDSA" as algorithm.
+ // sshd handles only the latter.
+ result.add(new TestData(
+ "EDDSA", "EdDSA", 25519,
+ new EdDSAGenParameterSpec("Ed25519")));
+ }
+
+ return parameterize(result);
}
- @Parameter
- public TestData data;
-
- private KeyPair testKey;
-
- private OpenSSHKeyPairResourceWriter writer = new OpenSSHKeyPairResourceWriter();
-
@Before
- public void generateKey() throws Exception {
+ public void setUp() throws Exception {
KeyPairGenerator generator;
if (data.provider == null) {
generator = KeyPairGenerator.getInstance(data.algorithm);
@@ -161,9 +142,9 @@
return KeyUtils.compareKeyPairs(a, b);
}
- private static void writeToFile(File file, byte[] sensitiveData)
+ private static void writeToFile(Path file, byte[] sensitiveData)
throws IOException {
- try (ByteChannel out = Files.newByteChannel(file.toPath(),
+ try (ByteChannel out = Files.newByteChannel(file,
StandardOpenOption.CREATE, StandardOpenOption.WRITE)) {
ByteBuffer buf = ByteBuffer.wrap(sensitiveData);
while (buf.hasRemaining()) {
@@ -175,103 +156,99 @@
}
@Test
- public void privateNoEncryption() throws Exception {
- File tmp = folder.newFile();
+ public void testWritePrivateKeyNoEncryption() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- writer.writePrivateKey(testKey, "a comment", null, out);
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", null, out);
writeToFile(tmp, out.toByteArray());
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
- new PathResource(tmp.toPath()), in, null).iterator().next();
- assertNotNull(key);
+ new PathResource(tmp), in, null).iterator().next();
+ assertNotNull("No key pair parsed", key);
+ assertKeyPairEquals("Mismatched recovered keys", testKey, key);
assertTrue("Keys should be equal", compare(key, testKey));
}
}
@Test
- public void privateNoPassword() throws Exception {
- File tmp = folder.newFile();
- OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ public void testWritePrivateKeyNoPassword() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- writer.writePrivateKey(testKey, "a comment", options, out);
+ OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
writeToFile(tmp, out.toByteArray());
- } finally {
- options.clear();
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
- new PathResource(tmp.toPath()), in, null).iterator().next();
- assertNotNull(key);
+ new PathResource(tmp), in, null).iterator().next();
+ assertNotNull("No key pair parsed", key);
+ assertKeyPairEquals("Mismatched recovered keys", testKey, key);
assertTrue("Keys should be equal", compare(key, testKey));
}
}
@Test
- public void privateEncryptedAesCbc128() throws Exception {
- File tmp = folder.newFile();
- OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ public void testWritePrivateKeyEncryptedAesCbc128() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- options.setPassphrase("nonsense".toCharArray());
+ OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ options.setPassword("nonsense");
options.setCipherName("AES");
options.setCipherMode("CBC");
options.setCipherType("128");
- writer.writePrivateKey(testKey, "a comment", options, out);
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
writeToFile(tmp, out.toByteArray());
- } finally {
- options.clear();
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
KeyPair key = SecurityUtils
- .loadKeyPairIdentities(null, new PathResource(tmp.toPath()),
+ .loadKeyPairIdentities(null, new PathResource(tmp),
in, FilePasswordProvider.of("nonsense"))
.iterator().next();
- assertNotNull(key);
+ assertNotNull("No key pair parsed", key);
+ assertKeyPairEquals("Mismatched recovered keys", testKey, key);
assertTrue("Keys should be equal", compare(key, testKey));
}
}
@Test
- public void privateEncryptedAesCtr256() throws Exception {
- File tmp = folder.newFile();
- OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ public void testWritePrivateKeyEncryptedAesCtr256() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- options.setPassphrase("nonsense".toCharArray());
+ OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ options.setPassword("nonsense");
options.setCipherName("AES");
options.setCipherMode("CTR");
options.setCipherType("256");
- writer.writePrivateKey(testKey, "a comment", options, out);
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
writeToFile(tmp, out.toByteArray());
- } finally {
- options.clear();
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
KeyPair key = SecurityUtils
- .loadKeyPairIdentities(null, new PathResource(tmp.toPath()),
+ .loadKeyPairIdentities(null, new PathResource(tmp),
in, FilePasswordProvider.of("nonsense"))
.iterator().next();
- assertNotNull(key);
+ assertNotNull("No key pair parsed", key);
+ assertKeyPairEquals("Mismatched recovered keys", testKey, key);
assertTrue("Keys should be equal", compare(key, testKey));
}
}
@Test
- public void privateEncryptedWrongPassword() throws Exception {
- File tmp = folder.newFile();
- OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ public void testWritePrivateKeyEncryptedWrongPassword() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- options.setPassphrase("nonsense".toCharArray());
+ OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ options.setPassword("nonsense");
options.setCipherName("AES");
options.setCipherMode("CTR");
options.setCipherType("256");
- writer.writePrivateKey(testKey, "a comment", options, out);
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
writeToFile(tmp, out.toByteArray());
- } finally {
- options.clear();
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
SecurityUtils.loadKeyPairIdentities(null,
- new PathResource(tmp.toPath()), in,
+ new PathResource(tmp), in,
FilePasswordProvider.of("wrong"));
fail("Expected an exception");
} catch (StreamCorruptedException | GeneralSecurityException e) {
@@ -280,29 +257,26 @@
}
@Test
- public void privateEncryptedNoPassword() throws Exception {
- File tmp = folder.newFile();
- OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ public void testWritePrivateKeyEncryptedNoPassword() throws Exception {
+ Path tmp = getTemporaryOutputFile();
try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
- options.setPassphrase("nonsense".toCharArray());
+ OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+ options.setPassword("nonsense");
options.setCipherName("AES");
options.setCipherMode("CTR");
options.setCipherType("256");
- writer.writePrivateKey(testKey, "a comment", options, out);
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
writeToFile(tmp, out.toByteArray());
- } finally {
- options.clear();
}
- try (InputStream in = Files.newInputStream(tmp.toPath())) {
+ try (InputStream in = Files.newInputStream(tmp)) {
assertThrows(GeneralSecurityException.class,
() -> SecurityUtils.loadKeyPairIdentities(null,
- new PathResource(tmp.toPath()), in, null));
+ new PathResource(tmp), in, null));
}
}
- private void checkPublicKey(File tmp, String comment) throws Exception {
- List<AuthorizedKeyEntry> keysRead = AuthorizedKeyEntry
- .readAuthorizedKeys(tmp.toPath());
+ private void checkPublicKey(Path tmp, String comment) throws Exception {
+ List<AuthorizedKeyEntry> keysRead = AuthorizedKeyEntry.readAuthorizedKeys(tmp);
assertEquals("Unexpected list size", 1, keysRead.size());
AuthorizedKeyEntry entry = keysRead.get(0);
String readComment = entry.getComment();
@@ -318,41 +292,76 @@
}
@Test
- public void publicWithComment() throws Exception {
- File tmp = folder.newFile();
- try (OutputStream out = new FileOutputStream(tmp)) {
- writer.writePublicKey(testKey, "a comment", out);
+ public void testWritePublicKeyWithComment() throws Exception {
+ Path tmp = getTemporaryOutputFile();
+ try (OutputStream out = Files.newOutputStream(tmp)) {
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePublicKey(testKey, "a comment", out);
}
checkPublicKey(tmp, "a comment");
}
@Test
- public void publicWithMultilineComment() throws Exception {
- File tmp = folder.newFile();
- try (OutputStream out = new FileOutputStream(tmp)) {
- writer.writePublicKey(testKey,
+ public void testWritePublicKeyWithMultilineComment() throws Exception {
+ Path tmp = getTemporaryOutputFile();
+ try (OutputStream out = Files.newOutputStream(tmp)) {
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePublicKey(testKey,
"a comment" + System.lineSeparator() + "second line", out);
}
assertEquals("Unexpected number of lines", 1,
- Files.readAllLines(tmp.toPath()).size());
+ Files.readAllLines(tmp).size());
checkPublicKey(tmp, "a comment");
}
@Test
- public void publicNoComment() throws Exception {
- File tmp = folder.newFile();
- try (OutputStream out = new FileOutputStream(tmp)) {
- writer.writePublicKey(testKey, null, out);
+ public void testWritePublicKeyNoComment() throws Exception {
+ Path tmp = getTemporaryOutputFile();
+ try (OutputStream out = Files.newOutputStream(tmp)) {
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePublicKey(testKey, null, out);
}
checkPublicKey(tmp, null);
}
@Test
- public void publicEmptyComment() throws Exception {
- File tmp = folder.newFile();
- try (OutputStream out = new FileOutputStream(tmp)) {
- writer.writePublicKey(testKey, "", out);
+ public void testWritePublicKeyEmptyComment() throws Exception {
+ Path tmp = getTemporaryOutputFile();
+ try (OutputStream out = Files.newOutputStream(tmp)) {
+ OpenSSHKeyPairResourceWriter.INSTANCE.writePublicKey(testKey, "", out);
}
checkPublicKey(tmp, null);
}
+
+ private Path getTemporaryOutputFile() throws IOException {
+ Path dir = assertHierarchyTargetFolderExists(getTempTargetFolder());
+ return dir.resolve(getCurrentTestName());
+ }
+
+ @SuppressWarnings("checkstyle:VisibilityModifier")
+ private static class TestData {
+ public final String algorithm;
+
+ public final String provider;
+
+ public final int keySize;
+
+ public final AlgorithmParameterSpec spec;
+
+ TestData(String algorithm, int keySize,
+ AlgorithmParameterSpec spec) {
+ this(algorithm, null, keySize, spec);
+ }
+
+ TestData(String algorithm, String provider, int keySize,
+ AlgorithmParameterSpec spec) {
+ this.algorithm = algorithm;
+ this.provider = provider;
+ this.keySize = keySize;
+ this.spec = spec;
+ }
+
+ @Override
+ public String toString() {
+ return algorithm + '-' + keySize
+ + (provider == null ? "" : '(' + provider + ')');
+ }
+ }
}
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 e990c5b..55537e0 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
@@ -49,11 +49,14 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import java.util.function.BiPredicate;
+import org.apache.sshd.common.config.keys.BuiltinIdentities;
+import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.io.IoUtils;
@@ -404,12 +407,23 @@
assertKeyEquals(message + "[private]", expected.getPrivate(), actual.getPrivate());
}
+ public static void assertKeyEncodingEquals(String message, Key expected, Key actual) {
+ if (expected == actual) {
+ return;
+ }
+
+ assertEquals(message + "[format]", expected.getFormat(), actual.getFormat());
+ assertArrayEquals(message + "[encoded-data]", expected.getEncoded(), actual.getEncoded());
+ }
+
public static <T extends Key> void assertKeyEquals(String message, T expected, T actual) {
if (expected == actual) {
return;
}
- assertEquals(message + "[algorithm]", expected.getAlgorithm(), actual.getAlgorithm());
+ assertEquals(message + "[algorithm]",
+ resolveEffectiveAlgorithm(expected.getAlgorithm()),
+ resolveEffectiveAlgorithm(actual.getAlgorithm()));
if (expected instanceof RSAPublicKey) {
assertRSAPublicKeyEquals(message, RSAPublicKey.class.cast(expected), RSAPublicKey.class.cast(actual));
@@ -424,7 +438,16 @@
} else if (expected instanceof ECPrivateKey) {
assertECPrivateKeyEquals(message, ECPrivateKey.class.cast(expected), ECPrivateKey.class.cast(actual));
}
- assertArrayEquals(message + "[encoded-data]", expected.getEncoded(), actual.getEncoded());
+ }
+
+ public static String resolveEffectiveAlgorithm(String algorithm) {
+ if (GenericUtils.isEmpty(algorithm)) {
+ return algorithm;
+ } else if (BuiltinIdentities.Constants.ECDSA.equalsIgnoreCase(algorithm)) {
+ return KeyUtils.EC_ALGORITHM;
+ } else {
+ return algorithm.toUpperCase(Locale.ENGLISH);
+ }
}
public static void assertRSAPublicKeyEquals(String message, RSAPublicKey expected, RSAPublicKey actual) {
