Better file handling for host keys
Store host keys in the OpenSSH format. This makes it possible to use
EdDSA host keys. Also set file permissions and read legacy files more
carefully.
diff --git a/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/AbstractGeneratorHostKeyProvider.java b/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/AbstractGeneratorHostKeyProvider.java
index f8cb533..29b6e3d 100644
--- a/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/AbstractGeneratorHostKeyProvider.java
+++ b/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/AbstractGeneratorHostKeyProvider.java
@@ -18,6 +18,7 @@
*/
package org.apache.sshd.server.keyprovider;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -25,6 +26,11 @@
import java.nio.file.LinkOption;
import java.nio.file.OpenOption;
import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.nio.file.attribute.AclEntry;
+import java.nio.file.attribute.AclEntryType;
+import java.nio.file.attribute.AclFileAttributeView;
+import java.nio.file.attribute.UserPrincipal;
import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.KeyPair;
@@ -46,6 +52,7 @@
import org.apache.sshd.common.keyprovider.KeySizeIndicator;
import org.apache.sshd.common.session.SessionContext;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.common.util.io.resource.PathResource;
import org.apache.sshd.common.util.security.SecurityUtils;
@@ -128,7 +135,7 @@
}
}
- @Override // co-variant return
+ @Override
public synchronized List<KeyPair> loadKeys(SessionContext session) {
Path keyPath = getPath();
Iterable<KeyPair> ids;
@@ -140,7 +147,7 @@
if (ids != null) {
keyPairHolder.set(ids);
}
- } catch (Throwable t) {
+ } catch (Exception t) {
warn("loadKeys({}) Failed ({}) to resolve: {}",
keyPath, t.getClass().getSimpleName(), t.getMessage(), t);
}
@@ -174,7 +181,7 @@
if (kp != null) {
return ids;
}
- } catch (Throwable e) {
+ } catch (Exception e) {
warn("resolveKeyPair({}) Failed ({}) to load: {}",
keyPath, e.getClass().getSimpleName(), e.getMessage(), e);
}
@@ -193,7 +200,7 @@
log.debug("resolveKeyPair({}) generated {} key={}-{}",
keyPath, alg, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
- } catch (Throwable e) {
+ } catch (Exception e) {
warn("resolveKeyPair({})[{}] Failed ({}) to generate {} key-pair: {}",
keyPath, alg, e.getClass().getSimpleName(), alg, e.getMessage(), e);
return null;
@@ -202,7 +209,7 @@
if (keyPath != null) {
try {
writeKeyPair(kp, keyPath);
- } catch (Throwable e) {
+ } catch (Exception e) {
warn("resolveKeyPair({})[{}] Failed ({}) to write {} key: {}",
alg, keyPath, e.getClass().getSimpleName(), alg, e.getMessage(), e);
}
@@ -263,22 +270,71 @@
return SecurityUtils.loadKeyPairIdentities(session, resourceKey, inputStream, null);
}
- protected void writeKeyPair(KeyPair kp, Path keyPath, OpenOption... options)
+ protected void writeKeyPair(KeyPair kp, Path keyPath)
throws IOException, GeneralSecurityException {
- if ((!Files.exists(keyPath)) || isOverwriteAllowed()) {
- PathResource location = new PathResource(keyPath); // The options are for write (!!)
- try (OutputStream os = Files.newOutputStream(keyPath, options)) {
- doWriteKeyPair(location, kp, os);
- } catch (Throwable e) {
- warn("writeKeyPair({}) failed ({}) to write key {}: {}",
- keyPath, e.getClass().getSimpleName(), kp, e.getMessage(), e);
+ Objects.requireNonNull(kp, "No host key");
+ if (!Files.exists(keyPath) || isOverwriteAllowed()) {
+ // Create an empty file or truncate an existing file
+ Files.newOutputStream(keyPath).close();
+ setFilePermissions(keyPath);
+ try (OutputStream os = Files.newOutputStream(keyPath, StandardOpenOption.WRITE,
+ StandardOpenOption.TRUNCATE_EXISTING)) {
+ doWriteKeyPair(new PathResource(keyPath), kp, os);
+ } catch (Exception e) {
+ error("writeKeyPair({}) failed ({}) to write {} host key : {}",
+ keyPath, e.getClass().getSimpleName(),
+ KeyUtils.getKeyType(kp), e.getMessage(), e);
}
} else {
- log.error("Overwriting key ({}) is disabled: using throwaway {}: {}",
- keyPath, KeyUtils.getKeyType(kp), KeyUtils.getFingerPrint((kp == null) ? null : kp.getPublic()));
+ log.warn("Overwriting host key ({}) is disabled: using throwaway {} key: {}",
+ keyPath, KeyUtils.getKeyType(kp), KeyUtils.getFingerPrint(kp.getPublic()));
}
}
+ private void setFilePermissions(Path path) throws IOException {
+ Throwable t = null;
+ if (OsUtils.isWin32()) {
+ AclFileAttributeView view = Files.getFileAttributeView(path, AclFileAttributeView.class);
+ UserPrincipal owner = Files.getOwner(path);
+ if (view != null && owner != null) {
+ try {
+ // Remove all access rights from non-owners.
+ List<AclEntry> restricted = new ArrayList<>();
+ for (AclEntry acl : view.getAcl()) {
+ if (owner.equals(acl.principal()) || AclEntryType.DENY.equals(acl.type())) {
+ restricted.add(acl);
+ } else {
+ // We can't use DENY access: if the owner is member of a group and we deny the group
+ // access, the owner won't be able to perform the access. Instead of denying permissions
+ // simply allow nothing.
+ restricted.add(AclEntry.newBuilder()
+ .setType(AclEntryType.ALLOW)
+ .setPrincipal(acl.principal())
+ .setPermissions(Collections.emptySet())
+ .build());
+ }
+ }
+ view.setAcl(restricted);
+ return;
+ } catch (IOException | SecurityException e) {
+ t = e;
+ }
+ }
+ } else {
+ File file = path.toFile();
+ if (!file.setExecutable(false)) {
+ log.debug("Host key file {}: cannot set non-executable", path);
+ }
+
+ boolean success = file.setWritable(false, false) && file.setWritable(true, true);
+ success = file.setReadable(false, false) && file.setReadable(true, true) && success;
+ if (success) {
+ return;
+ }
+ }
+ log.warn("Host key file {}: cannot set file permissions correctly (readable and writeable only by owner)", path, t);
+ }
+
protected abstract void doWriteKeyPair(
NamedResource resourceKey, KeyPair kp, OutputStream outputStream)
throws IOException, GeneralSecurityException;
@@ -305,6 +361,8 @@
} else if (keySize != 0) {
generator.initialize(keySize);
log.info("generateKeyPair({}) generating host key - size={}", algorithm, keySize);
+ } else {
+ log.info("generateKeyPair({}) generating host key", algorithm);
}
return generator.generateKeyPair();
diff --git a/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProvider.java b/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProvider.java
index 95c6bb0..38ba911 100644
--- a/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProvider.java
+++ b/sshd-common/src/main/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProvider.java
@@ -18,26 +18,36 @@
*/
package org.apache.sshd.server.keyprovider;
+import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
+import java.io.ObjectStreamClass;
+import java.io.ObjectStreamConstants;
import java.io.OutputStream;
+import java.io.StreamCorruptedException;
import java.nio.file.Path;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.spec.InvalidKeySpecException;
import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
import org.apache.sshd.common.NamedResource;
+import org.apache.sshd.common.config.keys.loader.openssh.OpenSSHKeyPairResourceParser;
+import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
import org.apache.sshd.common.session.SessionContext;
/**
- * TODO Add javadoc
+ * A simple implementation of an {@link AbstractGeneratorHostKeyProvider} that writes and reads host keys using the
+ * OpenSSH file format. Legacy keys written by earlier implementations used Java serialization. De-serializing is
+ * restricted to a small number of classes known to exist in serialized {@link KeyPair}s.
*
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class SimpleGeneratorHostKeyProvider extends AbstractGeneratorHostKeyProvider {
+
public SimpleGeneratorHostKeyProvider() {
super();
}
@@ -49,23 +59,100 @@
@Override
protected Iterable<KeyPair> doReadKeyPairs(SessionContext session, NamedResource resourceKey, InputStream inputStream)
throws IOException, GeneralSecurityException {
- KeyPair kp;
- try (ObjectInputStream r = new ObjectInputStream(inputStream)) {
- try {
- kp = (KeyPair) r.readObject();
- } catch (ClassNotFoundException e) {
- throw new InvalidKeySpecException("Missing classes: " + e.getMessage(), e);
+ try (BufferedInputStream in = new BufferedInputStream(inputStream)) {
+ if (isJavaSerialization(in, resourceKey)) {
+ try (ObjectInputStream r = new ValidatingObjectInputStream(in)) {
+ return Collections.singletonList((KeyPair) r.readObject());
+ } catch (ClassNotFoundException e) {
+ throw new InvalidKeySpecException(
+ "Cannot de-serialize " + resourceKey + ": missing classes: " + e.getMessage(), e);
+ }
+ } else {
+ OpenSSHKeyPairResourceParser reader = new OpenSSHKeyPairResourceParser();
+ return reader.loadKeyPairs(null, resourceKey, null, in);
}
}
+ }
- return Collections.singletonList(kp);
+ private boolean isJavaSerialization(BufferedInputStream in, NamedResource resourceKey) throws IOException {
+ in.mark(2);
+ try {
+ byte[] magicBytes = new byte[2];
+ int length = in.read(magicBytes);
+ if (length != 2) {
+ throw new StreamCorruptedException("File " + resourceKey + " is not a host key");
+ }
+ short magic = (short) (((magicBytes[0] & 0xFF) << 8) | (magicBytes[1] & 0xFF));
+ return magic == ObjectStreamConstants.STREAM_MAGIC;
+ } finally {
+ in.reset();
+ }
}
@Override
protected void doWriteKeyPair(NamedResource resourceKey, KeyPair kp, OutputStream outputStream)
throws IOException, GeneralSecurityException {
- try (ObjectOutputStream w = new ObjectOutputStream(outputStream)) {
- w.writeObject(kp);
+ OpenSSHKeyPairResourceWriter writer = new OpenSSHKeyPairResourceWriter();
+ try (OutputStream out = outputStream) {
+ writer.writePrivateKey(kp, "host key", null, out);
}
}
+
+ private static class ValidatingObjectInputStream extends ObjectInputStream {
+
+ private static final Set<String> ALLOWED = new HashSet<>();
+
+ static {
+ ALLOWED.add("[B"); // byte[], used in BC EC key serialization
+
+ ALLOWED.add("java.lang.Enum");
+ ALLOWED.add("java.lang.Number");
+ ALLOWED.add("java.lang.String");
+
+ ALLOWED.add("java.math.BigInteger"); // Used in BC DSA/RSA
+
+ ALLOWED.add("java.security.KeyPair");
+ ALLOWED.add("java.security.PublicKey");
+ ALLOWED.add("java.security.PrivateKey");
+ ALLOWED.add("java.security.KeyRep");
+ ALLOWED.add("java.security.KeyRep$Type");
+
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.dsa.BCDSAPrivateKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.dsa.BCDSAPublicKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateCrtKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPublicKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPrivateKey");
+ ALLOWED.add("org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey");
+
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.BCDSAPrivateKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.BCDSAPublicKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateCrtKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPublicKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPrivateKey");
+ ALLOWED.add("com.android.org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey");
+
+ // net.i2p EdDSA keys cannot be serialized anyway; so no need to whitelist any of their classes.
+ // They use the default serialization, which writes a great many different classes, but at least
+ // one of them does not implement Serializable, and thus writing runs into a NotSerializableException.
+ }
+
+ ValidatingObjectInputStream(InputStream in) throws IOException {
+ super(in);
+ }
+
+ @Override
+ protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
+ validate(desc.getName());
+ return super.resolveClass(desc);
+ }
+
+ private void validate(String className) throws IOException {
+ if (!ALLOWED.contains(className)) {
+ throw new IOException(className + " blocked for deserialization");
+ }
+ }
+ }
+
}
diff --git a/sshd-common/src/test/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProviderTest.java b/sshd-common/src/test/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProviderTest.java
index f0d3887..55c3064 100644
--- a/sshd-common/src/test/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProviderTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/server/keyprovider/SimpleGeneratorHostKeyProviderTest.java
@@ -19,6 +19,7 @@
package org.apache.sshd.server.keyprovider;
import java.io.IOException;
+import java.io.ObjectOutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.GeneralSecurityException;
@@ -88,7 +89,13 @@
new ECGenParameterSpec("P-521"));
}
- private Path testSimpleGeneratorHostKeyProvider(
+ @Test
+ public void testEdDSA() throws IOException, GeneralSecurityException {
+ Assume.assumeTrue("EdDSA not supported", SecurityUtils.isEDDSACurveSupported());
+ testSimpleGeneratorHostKeyProvider(SecurityUtils.EDDSA, KeyPairProvider.SSH_ED25519, -1, null);
+ }
+
+ private void testSimpleGeneratorHostKeyProvider(
String algorithm, String keyType, int keySize, AlgorithmParameterSpec keySpec)
throws IOException, GeneralSecurityException {
Path path = initKeyFileLocation(algorithm);
@@ -97,7 +104,16 @@
KeyPair kpRead = invokeSimpleGeneratorHostKeyProvider(path, algorithm, keyType, keySize, keySpec);
assertKeyPairEquals("Mismatched write/read key pairs", kpWrite, kpRead);
- return path;
+
+ if (!KeyPairProvider.SSH_ED25519.equals(keyType)) {
+ // Try the old way: use Java serialization. net.i2p EdDSA keys cannot be serialized.
+ path = initKeyFileLocation(algorithm, "ser");
+ try (ObjectOutputStream out = new ObjectOutputStream(Files.newOutputStream(path))) {
+ out.writeObject(kpWrite);
+ }
+ kpRead = invokeSimpleGeneratorHostKeyProvider(path, algorithm, keyType, keySize, keySpec);
+ assertKeyPairEquals("Mismatched serialized/deserialized key pairs", kpWrite, kpRead);
+ }
}
private static KeyPair invokeSimpleGeneratorHostKeyProvider(
@@ -135,8 +151,12 @@
}
private Path initKeyFileLocation(String algorithm) throws IOException {
+ return initKeyFileLocation(algorithm, "key");
+ }
+
+ private Path initKeyFileLocation(String algorithm, String extension) throws IOException {
Path path = assertHierarchyTargetFolderExists(getTempTargetRelativeFile(getClass().getSimpleName()));
- path = path.resolve(algorithm + "-simple.key");
+ path = path.resolve(algorithm + "-simple." + extension);
Files.deleteIfExists(path);
return path;
}
