You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/11/22 07:18:55 UTC
mina-sshd git commit: [SSHD-584] Check keys and configuration files
permissions as closely as possible like OpenSSH by default
Repository: mina-sshd
Updated Branches:
refs/heads/master 071755a20 -> b3eecb19d
[SSHD-584] Check keys and configuration files permissions as closely as possible like OpenSSH by default
* Based on https://github.com/apache/mina-sshd/pull/17
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/b3eecb19
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/b3eecb19
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/b3eecb19
Branch: refs/heads/master
Commit: b3eecb19d56f597a5791310a450c1da96419b90a
Parents: 071755a
Author: Alon Bar-Lev <al...@gmail.com>
Authored: Sun Nov 22 08:18:44 2015 +0200
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Sun Nov 22 08:18:44 2015 +0200
----------------------------------------------------------------------
.../java/org/apache/sshd/client/SshClient.java | 3 +-
.../DefaultConfigFileHostEntryResolver.java | 12 ++-
.../client/config/hosts/HostConfigEntry.java | 3 +-
.../sshd/client/config/keys/ClientIdentity.java | 5 +-
.../config/keys/ClientIdentityFileWatcher.java | 6 +-
.../sshd/common/config/keys/KeyUtils.java | 57 +++++++++++--
.../org/apache/sshd/common/util/OsUtils.java | 43 ++++++++++
.../org/apache/sshd/common/util/io/IoUtils.java | 18 +++++
.../common/util/io/ModifiableFileWatcher.java | 84 ++++++++++++++++++++
.../DefaultAuthorizedKeysAuthenticator.java | 14 +++-
.../apache/sshd/common/util/io/IoUtilsTest.java | 1 +
.../java/org/apache/sshd/server/ServerTest.java | 5 +-
.../DefaultAuthorizedKeysAuthenticatorTest.java | 3 +-
13 files changed, 230 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java
index c5024e0..fdb4dbb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java
@@ -89,6 +89,7 @@ import org.apache.sshd.common.keyprovider.AbstractFileKeyPairProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.AbstractSession;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.SecurityUtils;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.io.IoUtils;
@@ -661,7 +662,7 @@ public class SshClient extends AbstractFactoryManager implements ClientFactoryMa
}
if (login == null) {
- login = System.getProperty("user.name");
+ login = OsUtils.getCurrentUser();
}
if (port <= 0) {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/DefaultConfigFileHostEntryResolver.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/DefaultConfigFileHostEntryResolver.java b/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/DefaultConfigFileHostEntryResolver.java
index 8e1bb9a..6b1e86b 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/DefaultConfigFileHostEntryResolver.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/DefaultConfigFileHostEntryResolver.java
@@ -23,10 +23,10 @@ import java.io.File;
import java.io.IOException;
import java.nio.file.LinkOption;
import java.nio.file.Path;
-import java.nio.file.attribute.PosixFilePermission;
+import java.util.Collections;
import java.util.List;
-import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.io.IoUtils;
@@ -75,12 +75,16 @@ public class DefaultConfigFileHostEntryResolver extends ConfigFileHostEntryResol
log.debug("reloadHostConfigEntries({}@{}:{}) check permissions of {}", username, host, port, path);
}
- PosixFilePermission violation = KeyUtils.validateStrictKeyFilePermissions(path);
+ Pair<String, Object> violation = validateStrictConfigFilePermissions(path);
if (violation != null) {
- throw new IOException("String permission violation (" + violation + ") for " + path);
+ log.warn("reloadHostConfigEntries({}@{}:{}) invalid file={} permissions: {}",
+ username, host, port, path, violation.getFirst());
+ updateReloadAttributes();
+ return Collections.emptyList();
}
}
return super.reloadHostConfigEntries(path, host, port, username);
}
+
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/HostConfigEntry.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/HostConfigEntry.java b/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/HostConfigEntry.java
index 884aee5..7cbbf8f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/HostConfigEntry.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/config/hosts/HostConfigEntry.java
@@ -54,6 +54,7 @@ import org.apache.sshd.common.config.SshConfigFileReader;
import org.apache.sshd.common.config.keys.IdentityUtils;
import org.apache.sshd.common.config.keys.PublicKeyEntry;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.io.IoUtils;
@@ -1364,7 +1365,7 @@ public class HostConfigEntry implements UsernameHolder {
appendUserHome(sb);
break;
case LOCAL_USER_MACRO:
- sb.append(ValidateUtils.checkNotNullAndNotEmpty(System.getProperty("user.name"), "No local user name value"));
+ sb.append(ValidateUtils.checkNotNullAndNotEmpty(OsUtils.getCurrentUser(), "No local user name value"));
break;
case LOCAL_HOST_MACRO:
{
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentity.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentity.java b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentity.java
index dcf2013..f226c7c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentity.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentity.java
@@ -23,7 +23,6 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
-import java.nio.file.attribute.PosixFilePermission;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.util.Collection;
@@ -293,6 +292,7 @@ public final class ClientIdentity {
* <U>insensitive</U>), value=the {@link Path} of the file holding the key
* @throws IOException If failed to access the file system
* @see KeyUtils#validateStrictKeyFilePermissions(Path, LinkOption...)
+ * @see KeyUtils#validateStrictKeyFileOwner(Path, LinkOption...)
*/
public static Map<String, Path> scanIdentitiesFolder(
Path dir, boolean strict, Collection<String> types, Transformer<String, String> idGenerator, LinkOption... options)
@@ -316,8 +316,7 @@ public final class ClientIdentity {
}
if (strict) {
- PosixFilePermission perm = KeyUtils.validateStrictKeyFilePermissions(p, options);
- if (perm != null) {
+ if (KeyUtils.validateStrictKeyFilePermissions(p, options) != null) {
continue;
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentityFileWatcher.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentityFileWatcher.java b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentityFileWatcher.java
index 3ff3f90..bdd2bf5 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentityFileWatcher.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentityFileWatcher.java
@@ -21,13 +21,13 @@ package org.apache.sshd.client.config.keys;
import java.io.IOException;
import java.nio.file.Path;
-import java.nio.file.attribute.PosixFilePermission;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.common.util.io.ModifiableFileWatcher;
@@ -91,10 +91,10 @@ public class ClientIdentityFileWatcher extends ModifiableFileWatcher implements
protected KeyPair reloadClientIdentity(Path path) throws IOException, GeneralSecurityException {
if (isStrict()) {
- PosixFilePermission violation = KeyUtils.validateStrictKeyFilePermissions(path, IoUtils.EMPTY_LINK_OPTIONS);
+ Pair<String, Object> violation = KeyUtils.validateStrictKeyFilePermissions(path, IoUtils.EMPTY_LINK_OPTIONS);
if (violation != null) {
if (log.isDebugEnabled()) {
- log.debug("reloadClientIdentity({}) ignore due to permission violation: {}", path, violation);
+ log.debug("reloadClientIdentity({}) ignore due to {}", path, violation.getFirst());
}
return null;
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
index 4231c2f..3af34a9 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
@@ -47,6 +47,7 @@ import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -61,6 +62,7 @@ import org.apache.sshd.common.digest.DigestUtils;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.OsUtils;
+import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
@@ -123,17 +125,32 @@ public final class KeyUtils {
* (For {@code Unix}) If the path is a file, then its folder may not have
* group or others permissions
* </P></LI>
+ *
+ * <LI><P>
+ * The path must be owned by current user.
+ * </P></LI>
+ *
+ * <LI><P>
+ * (For {@code Unix}) The path may be owned by root.
+ * </P></LI>
+ *
+ * <LI><P>
+ * (For {@code Unix}) If the path is a file, then its folder must also
+ * have valid owner.
+ * </P></LI>
+ *
* </UL>
*
* @param path The {@link Path} to be checked - ignored if {@code null}
* or does not exist
* @param options The {@link LinkOption}s to use to query the file's permissions
- * @return The violated {@link PosixFilePermission} - {@code null} if
+ * @return The violated permission as {@link Pair} first is a message second is
+ * offending object {@link PosixFilePermission} or {@link String} for owner - {@code null} if
* no violations detected
* @throws IOException If failed to retrieve the permissions
* @see #STRICTLY_PROHIBITED_FILE_PERMISSION
*/
- public static PosixFilePermission validateStrictKeyFilePermissions(Path path, LinkOption... options) throws IOException {
+ public static Pair<String, Object> validateStrictKeyFilePermissions(Path path, LinkOption... options) throws IOException {
if ((path == null) || (!Files.exists(path, options))) {
return null;
}
@@ -144,20 +161,50 @@ public final class KeyUtils {
}
if (perms.contains(PosixFilePermission.OTHERS_EXECUTE)) {
- return PosixFilePermission.OTHERS_EXECUTE;
+ PosixFilePermission p = PosixFilePermission.OTHERS_EXECUTE;
+ return new Pair<String, Object>(String.format("Permissions violation (%s)", p), p);
}
if (OsUtils.isUNIX()) {
PosixFilePermission p = IoUtils.validateExcludedPermissions(perms, STRICTLY_PROHIBITED_FILE_PERMISSION);
if (p != null) {
- return p;
+ return new Pair<String, Object>(String.format("Permissions violation (%s)", p), p);
}
if (Files.isRegularFile(path, options)) {
Path parent = path.getParent();
p = IoUtils.validateExcludedPermissions(IoUtils.getPermissions(parent, options), STRICTLY_PROHIBITED_FILE_PERMISSION);
if (p != null) {
- return p;
+ return new Pair<String, Object>(String.format("Parent permissions violation (%s)", p), p);
+ }
+ }
+ }
+
+ String owner = IoUtils.getFileOwner(path, options);
+ if (GenericUtils.isEmpty(owner)) {
+ // we cannot get owner
+ // general issue: jvm does not support permissions
+ // security issue: specific filesystem does not support permissions
+ return null;
+ }
+
+ String current = OsUtils.getCurrentUser();
+ Set<String> expected = new HashSet<>();
+ expected.add(current);
+ if (OsUtils.isUNIX()) {
+ // Windows "Administrator" was considered however in Windows most likely a group is used.
+ expected.add(OsUtils.ROOT_USER);
+ }
+
+ if (!expected.contains(owner)) {
+ return new Pair<String, Object>(String.format("Owner violation (%s)", owner), owner);
+ }
+
+ if (OsUtils.isUNIX()) {
+ if (Files.isRegularFile(path, options)) {
+ String parentOwner = IoUtils.getFileOwner(path.getParent(), options);
+ if ((!GenericUtils.isEmpty(parentOwner)) && (!expected.contains(parentOwner))) {
+ return new Pair<String, Object>(String.format("Parent owner violation (%s)", parentOwner), parentOwner);
}
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/common/util/OsUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/OsUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/OsUtils.java
index 24f2fdd..55cf1e0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/OsUtils.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/OsUtils.java
@@ -21,6 +21,7 @@ package org.apache.sshd.common.util;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
/**
* Operating system dependent utility methods.
@@ -28,14 +29,24 @@ import java.util.List;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public final class OsUtils {
+
+ /**
+ * Property that can be used to override the reported value from {@link #getCurrentUser()}.
+ * If not set then "user.name" system property is used
+ */
+ public static final String CURRENT_USER_OVERRIDE_PROP = "org.apache.sshd.currentUser";
+
public static final String WINDOWS_SHELL_COMMAND_NAME = "cmd.exe";
public static final String LINUX_SHELL_COMMAND_NAME = "/bin/sh";
+ public static final String ROOT_USER = "root";
+
public static final List<String> LINUX_COMMAND =
Collections.unmodifiableList(Arrays.asList(LINUX_SHELL_COMMAND_NAME, "-i", "-l"));
public static final List<String> WINDOWS_COMMAND =
Collections.unmodifiableList(Collections.singletonList(WINDOWS_SHELL_COMMAND_NAME));
+ private static final AtomicReference<String> CURRENT_USER_HOLDER = new AtomicReference<>(null);
private static final boolean WIN32 = GenericUtils.trimToEmpty(System.getProperty("os.name")).toLowerCase().contains("windows");
private OsUtils() {
@@ -68,4 +79,36 @@ public final class OsUtils {
}
}
+ /**
+ * Get current user name
+ *
+ * @return Current user
+ * @see #CURRENT_USER_OVERRIDE_PROP
+ */
+ public static String getCurrentUser() {
+ String username = null;
+ synchronized (CURRENT_USER_HOLDER) {
+ username = CURRENT_USER_HOLDER.get();
+ if (username != null) { // have we already resolved it ?
+ return username;
+ }
+
+ username = System.getProperty(CURRENT_USER_OVERRIDE_PROP, System.getProperty("user.name"));
+ ValidateUtils.checkNotNullAndNotEmpty(username, "No username available");
+ CURRENT_USER_HOLDER.set(username);
+ }
+
+ return username;
+ }
+
+ /**
+ * Can be used to programmatically set the username reported by {@link #getCurrentUser()}
+ * @param username The username to set - if {@code null} then {@link #CURRENT_USER_OVERRIDE_PROP}
+ * will be consulted
+ */
+ public static void setCurrentUser(String username) {
+ synchronized (CURRENT_USER_HOLDER) {
+ CURRENT_USER_HOLDER.set(username);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/common/util/io/IoUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/io/IoUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/io/IoUtils.java
index 1b8d01b..f8e692c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/io/IoUtils.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/io/IoUtils.java
@@ -32,6 +32,7 @@ import java.nio.file.LinkOption;
import java.nio.file.OpenOption;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.UserPrincipal;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -257,6 +258,23 @@ public final class IoUtils {
}
/**
+ * <P>Get file owner.</P>
+ *
+ * @param path The {@link Path}
+ * @param options The {@link LinkOption}s to use when querying the owner
+ * @return Owner of the file or null if unsupported
+ * @throws IOException If failed to access the file system
+ */
+ public static String getFileOwner(Path path, LinkOption... options) throws IOException {
+ try {
+ UserPrincipal principal = Files.getOwner(path, options);
+ return (principal == null) ? null : principal.getName();
+ } catch (UnsupportedOperationException e) {
+ return null;
+ }
+ }
+
+ /**
* <P>Checks if a file exists - <B>Note:</B> according to the
* <A HREF="http://docs.oracle.com/javase/tutorial/essential/io/check.html">Java tutorial - Checking a File or Directory</A>:
* </P>
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/common/util/io/ModifiableFileWatcher.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/io/ModifiableFileWatcher.java b/sshd-core/src/main/java/org/apache/sshd/common/util/io/ModifiableFileWatcher.java
index 6d61b31..94b8f28 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/io/ModifiableFileWatcher.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/io/ModifiableFileWatcher.java
@@ -26,10 +26,19 @@ import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.HashSet;
import java.util.Objects;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
+import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
@@ -41,6 +50,14 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean;
*/
public class ModifiableFileWatcher extends AbstractLoggingBean {
+ /**
+ * The {@link Set} of {@link PosixFilePermission} <U>not</U> allowed if strict
+ * permissions are enforced on key files
+ */
+ public static final Set<PosixFilePermission> STRICTLY_PROHIBITED_FILE_PERMISSION =
+ Collections.unmodifiableSet(
+ EnumSet.of(PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE));
+
protected final LinkOption[] options;
private final Path file;
@@ -174,4 +191,71 @@ public class ModifiableFileWatcher extends AbstractLoggingBean {
public String toString() {
return Objects.toString(getPath());
}
+
+ /**
+ * <P>Checks if a path has strict permissions</P>
+ * <UL>
+ *
+ * <LI><P>
+ * (For {@code Unix}) The path may not have group or others write permissions
+ * </P></LI>
+ *
+ * <LI><P>
+ * The path must be owned by current user.
+ * </P></LI>
+ *
+ * <LI><P>
+ * (For {@code Unix}) The path may be owned by root.
+ * </P></LI>
+ *
+ * </UL>
+ *
+ * @param path The {@link Path} to be checked - ignored if {@code null}
+ * or does not exist
+ * @param options The {@link LinkOption}s to use to query the file's permissions
+ * @return The violated permission as {@link Pair} first is a message second is
+ * offending object {@link PosixFilePermission} or {@link String} for owner - {@code null} if
+ * no violations detected
+ * @throws IOException If failed to retrieve the permissions
+ * @see #STRICTLY_PROHIBITED_FILE_PERMISSION
+ */
+ public static Pair<String, Object> validateStrictConfigFilePermissions(Path path, LinkOption... options) throws IOException {
+ if ((path == null) || (!Files.exists(path, options))) {
+ return null;
+ }
+
+ Collection<PosixFilePermission> perms = IoUtils.getPermissions(path, options);
+ if (GenericUtils.isEmpty(perms)) {
+ return null;
+ }
+
+ if (OsUtils.isUNIX()) {
+ PosixFilePermission p = IoUtils.validateExcludedPermissions(perms, STRICTLY_PROHIBITED_FILE_PERMISSION);
+ if (p != null) {
+ return new Pair<String, Object>(String.format("Permissions violation (%s)", p), p);
+ }
+ }
+
+ String owner = IoUtils.getFileOwner(path, options);
+ if (GenericUtils.isEmpty(owner)) {
+ // we cannot get owner
+ // general issue: jvm does not support permissions
+ // security issue: specific filesystem does not support permissions
+ return null;
+ }
+
+ String current = OsUtils.getCurrentUser();
+ Set<String> expected = new HashSet<>();
+ expected.add(current);
+ if (OsUtils.isUNIX()) {
+ // Windows "Administrator" was considered however in Windows most likely a group is used.
+ expected.add(OsUtils.ROOT_USER);
+ }
+
+ if (!expected.contains(owner)) {
+ return new Pair<String, Object>(String.format("Owner violation (%s)", owner), owner);
+ }
+
+ return null;
+ }
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/main/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticator.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticator.java b/sshd-core/src/main/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticator.java
index d0b1a01..6cdc7ea 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticator.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticator.java
@@ -26,9 +26,12 @@ import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Collection;
+import java.util.Collections;
import org.apache.sshd.common.auth.UsernameHolder;
import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.util.OsUtils;
+import org.apache.sshd.common.util.Pair;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.session.ServerSession;
@@ -57,7 +60,7 @@ public class DefaultAuthorizedKeysAuthenticator extends AuthorizedKeysAuthentica
* does not check these permissions
*/
public DefaultAuthorizedKeysAuthenticator(boolean strict) {
- this(System.getProperty("user.name"), strict);
+ this(OsUtils.getCurrentUser(), strict);
}
public DefaultAuthorizedKeysAuthenticator(String user, boolean strict) {
@@ -73,7 +76,7 @@ public class DefaultAuthorizedKeysAuthenticator extends AuthorizedKeysAuthentica
}
public DefaultAuthorizedKeysAuthenticator(Path path, boolean strict, LinkOption... options) {
- this(System.getProperty("user.name"), path, strict, options);
+ this(OsUtils.getCurrentUser(), path, strict, options);
}
public DefaultAuthorizedKeysAuthenticator(String user, Path path, boolean strict, LinkOption... options) {
@@ -108,9 +111,12 @@ public class DefaultAuthorizedKeysAuthenticator extends AuthorizedKeysAuthentica
log.debug("reloadAuthorizedKeys({})[{}] check permissions of {}", username, session, path);
}
- PosixFilePermission violation = KeyUtils.validateStrictKeyFilePermissions(path);
+ Pair<String, Object> violation = KeyUtils.validateStrictKeyFilePermissions(path);
if (violation != null) {
- throw new IOException("String permission violation (" + violation + ") for " + path);
+ log.warn("reloadAuthorizedKeys({})[{}] invalid file={} permissions: {}",
+ username, session, path, violation.getFirst());
+ updateReloadAttributes();
+ return Collections.emptyList();
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java b/sshd-core/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
index 09f7232..0b9151a 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
@@ -54,4 +54,5 @@ public class IoUtilsTest extends BaseTestSupport {
assertArrayEquals("Mismatched bytes at iteration " + index, expected, actual);
}
}
+
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
index bc35530..32d77fe 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
@@ -48,8 +48,8 @@ import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.client.session.ClientSessionImpl;
import org.apache.sshd.client.session.SessionFactory;
import org.apache.sshd.common.FactoryManager;
-import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.channel.Channel;
import org.apache.sshd.common.channel.TestChannelListener;
import org.apache.sshd.common.channel.WindowClosedException;
@@ -60,6 +60,7 @@ import org.apache.sshd.common.session.AbstractSession;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.session.SessionListener;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.deprecated.ClientUserAuthServiceOld;
import org.apache.sshd.server.command.ScpCommandFactory;
@@ -529,7 +530,7 @@ public class ServerTest extends BaseTestSupport {
{
put("test", getCurrentTestName());
put("port", Integer.toString(port));
- put("user", System.getProperty("user.name"));
+ put("user", OsUtils.getCurrentUser());
}
};
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/b3eecb19/sshd-core/src/test/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticatorTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticatorTest.java b/sshd-core/src/test/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticatorTest.java
index d9cb973..23883fe 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticatorTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/config/keys/DefaultAuthorizedKeysAuthenticatorTest.java
@@ -27,6 +27,7 @@ import java.nio.file.Path;
import java.security.PublicKey;
import java.util.Collection;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.auth.pubkey.PublickeyAuthenticator;
import org.apache.sshd.util.test.BaseTestSupport;
@@ -57,7 +58,7 @@ public class DefaultAuthorizedKeysAuthenticatorTest extends BaseTestSupport {
Collection<AuthorizedKeyEntry> entries = AuthorizedKeyEntry.readAuthorizedKeys(file);
Collection<PublicKey> keySet = AuthorizedKeyEntry.resolveAuthorizedKeys(entries);
PublickeyAuthenticator auth = new DefaultAuthorizedKeysAuthenticator(file, false);
- String thisUser = System.getProperty("user.name");
+ String thisUser = OsUtils.getCurrentUser();
for (String username : new String[]{null, "", thisUser, getClass().getName() + "#" + getCurrentTestName()}) {
boolean expected = thisUser.equals(username);
for (PublicKey key : keySet) {