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 <[email protected]> Authored: Wed Jan 31 12:51:29 2018 +0200 Committer: Goldstein Lyor <[email protected]> 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:[email protected]";>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:[email protected]";>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==", "[email protected]", "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...==", "[email protected]", "tunnel=\"0\"", "command=\"sh /etc/netstart tun0\""); + addData("ssh-rsa AAAA1C8...32Tv==", "[email protected]", "!restrict", "command=\"uptime\""); + addData("ssh-rsa AAAA1f8...IrrC5==", "[email protected]", "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 + "]"; + } +}
