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 2018/01/31 14:02:55 UTC

mina-sshd git commit: [SSHD-796] Fixed handling of spaces, commas and multiple options in authorized keys data parsing of login options

Repository: mina-sshd
Updated Branches:
  refs/heads/master 6a63c251f -> 885bbdbf0


[SSHD-796] Fixed handling of spaces, commas and multiple options in authorized keys data parsing of login options


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/885bbdbf
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/885bbdbf
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/885bbdbf

Branch: refs/heads/master
Commit: 885bbdbf0e5d29d480d4d400096996796262ba5a
Parents: 6a63c25
Author: Goldstein Lyor <ly...@c-b4.com>
Authored: Wed Jan 31 12:51:29 2018 +0200
Committer: Goldstein Lyor <ly...@c-b4.com>
Committed: Wed Jan 31 16:02:49 2018 +0200

----------------------------------------------------------------------
 .../common/config/keys/AuthorizedKeyEntry.java  | 160 ++++++++++++++++---
 ...AuthorizedKeyEntryLoginOptionsParseTest.java | 132 +++++++++++++++
 2 files changed, 272 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/885bbdbf/sshd-core/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java b/sshd-core/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java
index 342f831..6d90d30 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java
@@ -34,6 +34,7 @@ import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.security.GeneralSecurityException;
 import java.security.PublicKey;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -57,8 +58,10 @@ import org.apache.sshd.server.auth.pubkey.RejectAllPublickeyAuthenticator;
  * comment and/or login options are not considered part of equality
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ * @see <A HREF="http://man.openbsd.org/sshd.8#AUTHORIZED_KEYS_FILE_FORMAT">sshd(8) - AUTHORIZED_KEYS_FILE_FORMAT</A>
  */
 public class AuthorizedKeyEntry extends PublicKeyEntry {
+    public static final char BOOLEAN_OPTION_NEGATION_INDICATOR = '!';
 
     private static final long serialVersionUID = -9007505285002809156L;
 
@@ -325,10 +328,12 @@ public class AuthorizedKeyEntry extends PublicKeyEntry {
         String keyType = line.substring(0, startPos);
         PublicKeyEntryDecoder<?, ?> decoder = KeyUtils.getPublicKeyEntryDecoder(keyType);
         AuthorizedKeyEntry entry;
-        if (decoder == null) {  // assume this is due to the fact that it starts with login options
-            entry = parseAuthorizedKeyEntry(line.substring(startPos + 1).trim());
+        // assume this is due to the fact that it starts with login options
+        if (decoder == null) {
+            Map.Entry<String, String> comps = resolveEntryComponents(line);
+            entry = parseAuthorizedKeyEntry(comps.getValue());
             ValidateUtils.checkTrue(entry != null, "Bad format (no key data after login options): %s", line);
-            entry.setLoginOptions(parseLoginOptions(keyType));
+            entry.setLoginOptions(parseLoginOptions(comps.getKey()));
         } else {
             String encData = (endPos < (line.length() - 1)) ? line.substring(0, endPos).trim() : line;
             String comment = (endPos < (line.length() - 1)) ? line.substring(endPos + 1).trim() : null;
@@ -339,34 +344,149 @@ public class AuthorizedKeyEntry extends PublicKeyEntry {
         return entry;
     }
 
+    /**
+     * Parses a single line from an <code>authorized_keys</code> file that is <U>known</U>
+     * to contain login options and separates it to the options and the rest of the line.
+     *
+     * @param line The line to be parsed
+     * @return A {@link SimpleImmutableEntry} representing the parsed data where key=login options part
+     * and value=rest of the data - {@code null} if no data in line or line starts with comment character
+     * @see <A HREF="http://man.openbsd.org/sshd.8#AUTHORIZED_KEYS_FILE_FORMAT">sshd(8) - AUTHORIZED_KEYS_FILE_FORMAT</A>
+     */
+    public static SimpleImmutableEntry<String, String> resolveEntryComponents(String value) {
+        String line = GenericUtils.replaceWhitespaceAndTrim(value);
+        if (GenericUtils.isEmpty(line) || (line.charAt(0) == COMMENT_CHAR) /* comment ? */) {
+            return null;
+        }
+
+        for (int lastPos = 0; lastPos < line.length();) {
+            int startPos = line.indexOf(' ', lastPos);
+            if (startPos < lastPos) {
+                throw new IllegalArgumentException("Bad format (no key data delimiter): " + line);
+            }
+
+            int quotePos = line.indexOf('"', startPos + 1);
+            // If found quotes after the space then assume part of a login option
+            if (quotePos > startPos) {
+                lastPos = quotePos + 1;
+                continue;
+            }
+
+            String loginOptions = line.substring(0, startPos).trim();
+            String remainder = line.substring(startPos + 1).trim();
+            return new SimpleImmutableEntry<>(loginOptions, remainder);
+        }
+
+        throw new IllegalArgumentException("Bad format (no key data contents): " + line);
+    }
+
+    /**
+     * <P>
+     * Parses login options line according to
+     * <A HREF="http://man.openbsd.org/sshd.8#AUTHORIZED_KEYS_FILE_FORMAT">sshd(8) - AUTHORIZED_KEYS_FILE_FORMAT</A>
+     * guidelines. <B>Note:</B>
+     * </P>
+     *
+     * <UL>
+     *      <P><LI>
+     *      Options that have a value are automatically stripped of any surrounding double quotes./
+     *      </LI></P>
+     *
+     *      <P><LI>
+     *      Options that have no value are marked as {@code true/false} - according
+     *      to the {@link #BOOLEAN_OPTION_NEGATION_INDICATOR}.
+     *      </LI></P>
+     *
+     *      <P><LI>
+     *      Options that appear multiple times are simply concatenated using comma as separator.
+     *      </LI></P>
+     * </UL>
+     *
+     * @param options The options line to parse - ignored if {@code null}/empty/blank
+     * @param A {@link NavigableMap} where key=case <U>insensitive</U> option name and value=the parsed value.
+     * @see #addLoginOption(Map, String) addLoginOption
+     */
     public static NavigableMap<String, String> parseLoginOptions(String options) {
-        // TODO add support if quoted values contain ','
-        String[] pairs = GenericUtils.split(GenericUtils.replaceWhitespaceAndTrim(options), ',');
-        if (GenericUtils.isEmpty(pairs)) {
+        String line = GenericUtils.replaceWhitespaceAndTrim(options);
+        int len = GenericUtils.length(line);
+        if (len <= 0) {
             return Collections.emptyNavigableMap();
         }
 
         NavigableMap<String, String> optsMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
-        for (String p : pairs) {
-            p = GenericUtils.trimToEmpty(p);
-            if (GenericUtils.isEmpty(p)) {
-                continue;
+        int lastPos = 0;
+        for (int curPos = 0; curPos < len; curPos++) {
+            int nextPos = line.indexOf(',', curPos);
+            if (nextPos < curPos) {
+                break;
             }
 
-            int pos = p.indexOf('=');
-            String name = (pos < 0) ? p : GenericUtils.trimToEmpty(p.substring(0, pos));
-            CharSequence value = (pos < 0) ? null : GenericUtils.trimToEmpty(p.substring(pos + 1));
-            value = GenericUtils.stripQuotes(value);
-            if (value == null) {
-                value = Boolean.TRUE.toString();
-            }
+            // check if "true" comma or one inside quotes
+            int quotePos = line.indexOf('"', curPos);
+            if ((quotePos >= lastPos) && (quotePos < nextPos)) {
+                nextPos = line.indexOf('"', quotePos + 1);
+                if (nextPos <= quotePos) {
+                    throw new IllegalArgumentException("Bad format (imbalanced quoted command): " + line);
+                }
 
-            String prev = optsMap.put(name, value.toString());
-            if (prev != null) {
-                throw new IllegalArgumentException("Multiple values for key=" + name + ": old=" + prev + ", new=" + value);
+                // Make sure either comma or no more options follow the 2nd quote
+                for (nextPos++; nextPos < len; nextPos++) {
+                    char ch = line.charAt(nextPos);
+                    if (ch == ',') {
+                        break;
+                    }
+
+                    if (ch != ' ') {
+                        throw new IllegalArgumentException("Bad format (incorrect list format): " + line);
+                    }
+                }
             }
+
+            addLoginOption(optsMap, line.substring(lastPos, nextPos));
+            lastPos = nextPos + 1;
+            curPos = lastPos;
+        }
+
+        // Any leftovers at end of line ?
+        if (lastPos < len) {
+            addLoginOption(optsMap, line.substring(lastPos));
         }
 
         return optsMap;
     }
+
+    /**
+     * Parses and adds a new option to the options map. If a valued option is re-specified then
+     * its value(s) are concatenated using comma as separator.
+     *
+     * @param optsMap Options map to add to
+     * @param option The option data to parse - ignored if {@code null}/empty/blank
+     * @return The updated entry - {@code null} if no option updated in the map
+     * @throws IllegalStateException If a boolean option is re-specified
+     */
+    public static SimpleImmutableEntry<String, String> addLoginOption(Map<String, String> optsMap, String option) {
+        String p = GenericUtils.trimToEmpty(option);
+        if (GenericUtils.isEmpty(p)) {
+            return null;
+        }
+
+        int pos = p.indexOf('=');
+        String name = (pos < 0) ? p : GenericUtils.trimToEmpty(p.substring(0, pos));
+        CharSequence value = (pos < 0) ? null : GenericUtils.trimToEmpty(p.substring(pos + 1));
+        value = GenericUtils.stripQuotes(value);
+        if (value == null) {
+            value = Boolean.toString(name.charAt(0) != BOOLEAN_OPTION_NEGATION_INDICATOR);
+        }
+
+        SimpleImmutableEntry<String, String> entry = new SimpleImmutableEntry<>(name, value.toString());
+        String prev = optsMap.put(entry.getKey(), entry.getValue());
+        if (prev != null) {
+            if (pos < 0) {
+                throw new IllegalStateException("Bad format (boolean option (" + name + ") re-specified): " + p);
+            }
+            optsMap.put(entry.getKey(), prev + "," + entry.getValue());
+        }
+
+        return entry;
+    }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/885bbdbf/sshd-core/src/test/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntryLoginOptionsParseTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntryLoginOptionsParseTest.java b/sshd-core/src/test/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntryLoginOptionsParseTest.java
new file mode 100644
index 0000000..7f3be56
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntryLoginOptionsParseTest.java
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sshd.common.config.keys;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.MethodSorters;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.junit.runners.Parameterized.UseParametersRunnerFactory;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+@RunWith(Parameterized.class)   // see https://github.com/junit-team/junit/wiki/Parameterized-tests
+@UseParametersRunnerFactory(JUnit4ClassRunnerWithParametersFactory.class)
+public class AuthorizedKeyEntryLoginOptionsParseTest extends BaseTestSupport {
+    private final String value;
+    private final String loginPart;
+    private final String keyPart;
+    private final Map<String, String> options;
+
+    public AuthorizedKeyEntryLoginOptionsParseTest(String value, String loginPart, String keyPart, Map<String, String> options) {
+        this.value = value;
+        this.loginPart = loginPart;
+        this.keyPart = keyPart;
+        this.options = options;
+    }
+
+    @Parameters(name = "{0}")
+    public static List<Object[]> parameters() {
+        return Collections.unmodifiableList(new ArrayList<Object[]>() {
+            // not serializing it
+            private static final long serialVersionUID = 1L;
+
+            private final StringBuilder sb = new StringBuilder(Byte.MAX_VALUE);
+
+            {
+                addData("ssh-rsa AAAAB2...19Q==", "john@example.net", "from=\"*.sales.example.net,!pc.sales.example.net\"");
+                addData("ssh-dss AAAAC3...51R==", "example.net", "command=\"dump /home\"", "no-pty", "no-port-forwarding");
+                addData("ssh-dss AAAAB5...21S==", "", "permitopen=\"192.0.2.1:80\"", "permitopen=\"192.0.2.2:25\"");
+                addData("ssh-rsa AAAA...==", "jane@example.net", "tunnel=\"0\"", "command=\"sh /etc/netstart tun0\"");
+                addData("ssh-rsa AAAA1C8...32Tv==", "user@example.net", "!restrict", "command=\"uptime\"");
+                addData("ssh-rsa AAAA1f8...IrrC5==", "user@example.net", "restrict", "!pty", "command=\"nethack\"");
+            }
+
+            private void addData(String keyData, String comment, String... comps) {
+                sb.setLength(0);
+
+                Map<String, String> optionsMap = new HashMap<>();
+                for (String c : comps) {
+                    if (sb.length() > 0) {
+                        sb.append(',');
+                    }
+                    sb.append(c);
+
+                    int pos = c.indexOf('=');
+                    if (pos > 0) {
+                        String name = c.substring(0, pos);
+                        String value = GenericUtils.stripQuotes(c.substring(pos + 1)).toString();
+                        String prev = optionsMap.put(name, value);
+                        if (prev != null) {
+                            optionsMap.put(name, prev + "," + value);
+                        }
+                    } else {
+                        optionsMap.put(c, Boolean.toString(c.charAt(0) != AuthorizedKeyEntry.BOOLEAN_OPTION_NEGATION_INDICATOR));
+                    }
+                }
+
+                int pos = sb.length();
+
+                sb.append(' ').append(keyData);
+                if (GenericUtils.isNotEmpty(comment)) {
+                    sb.append(' ').append(comment);
+                }
+
+                String value = sb.toString();
+                add(new Object[] {value, value.substring(0, pos), value.substring(pos + 1), optionsMap});
+            }
+        });
+    }
+
+    @Test
+    public void testResolveEntryComponents() {
+        Map.Entry<String, String> actual = AuthorizedKeyEntry.resolveEntryComponents(value);
+        assertNotNull(value, actual);
+        assertEquals("login(" + value + ")", loginPart, actual.getKey());
+        assertEquals("remainder(" + value + ")", keyPart, actual.getValue());
+    }
+
+    @Test
+    public void testParseLoginOptions() {
+        Map<String, String> parsed = AuthorizedKeyEntry.parseLoginOptions(loginPart);
+        options.forEach((key, expected) -> {
+            String actual = parsed.get(key);
+            assertEquals(key, expected, actual);
+        });
+        assertEquals("Mismatched size", options.size(), parsed.size());
+    }
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "[" + value + "]";
+    }
+}