This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 5a8fe830b2a2308a2b24ac8115a391af477f64f5
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sat Nov 5 21:00:40 2022 +0100
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.
---
.../AbstractGeneratorHostKeyProvider.java | 88 ++++++++++++++---
.../SimpleGeneratorHostKeyProvider.java | 109 ++++++++++++++++++---
.../SimpleGeneratorHostKeyProviderTest.java | 26 ++++-
3 files changed, 194 insertions(+), 29 deletions(-)
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 f8cb53334..29b6e3d80 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.Files;
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.AbstractKeyPairProvider;
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 @@ public abstract class AbstractGeneratorHostKeyProvider
}
}
- @Override // co-variant return
+ @Override
public synchronized List<KeyPair> loadKeys(SessionContext session) {
Path keyPath = getPath();
Iterable<KeyPair> ids;
@@ -140,7 +147,7 @@ public abstract class AbstractGeneratorHostKeyProvider
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 @@ public abstract class AbstractGeneratorHostKeyProvider
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 @@ public abstract class AbstractGeneratorHostKeyProvider
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 @@ public abstract class AbstractGeneratorHostKeyProvider
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 @@ public abstract class AbstractGeneratorHostKeyProvider
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 @@ public abstract class AbstractGeneratorHostKeyProvider
} 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 95c6bb0a1..38ba9113b 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:d...@mina.apache.org";>Apache MINA SSHD Project</a>
*/
public class SimpleGeneratorHostKeyProvider extends
AbstractGeneratorHostKeyProvider {
+
public SimpleGeneratorHostKeyProvider() {
super();
}
@@ -49,23 +59,100 @@ public class SimpleGeneratorHostKeyProvider extends
AbstractGeneratorHostKeyProv
@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 f0d388732..55c3064b2 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 @@ public class SimpleGeneratorHostKeyProviderTest extends
JUnitTestSupport {
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 @@ public class SimpleGeneratorHostKeyProviderTest extends
JUnitTestSupport {
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 @@ public class SimpleGeneratorHostKeyProviderTest extends
JUnitTestSupport {
}
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;
}
