sshd: modernize ssh config file parsing
OpenSSH has changed some things in ssh config files. Update our parser
to implement some of these changes:
* ignore trailing comments on a line
* rename PubkeyAcceptedKeyTypes to PubkeyAcceptedAlgorithms
Note that for the rename, openSSH still accepts both names. We do the
same, translating names whenever we get or set values.
Change-Id: Icccca060e6a4350a7acf05ff9e260f2c8c60ee1a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
index af09f49..4c7e99e 100644
--- a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
+++ b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
@@ -467,4 +467,34 @@
new File(new File(home, ".ssh"), localhost + "_id_dsa"),
h.getIdentityFile());
}
+
+ @Test
+ public void testPubKeyAcceptedAlgorithms() throws Exception {
+ config("Host=orcz\n\tPubkeyAcceptedAlgorithms ^ssh-rsa");
+ Host h = osc.lookup("orcz");
+ Config c = h.getConfig();
+ assertEquals("^ssh-rsa",
+ c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS));
+ assertEquals("^ssh-rsa", c.getValue("PubkeyAcceptedKeyTypes"));
+ }
+
+ @Test
+ public void testPubKeyAcceptedKeyTypes() throws Exception {
+ config("Host=orcz\n\tPubkeyAcceptedKeyTypes ^ssh-rsa");
+ Host h = osc.lookup("orcz");
+ Config c = h.getConfig();
+ assertEquals("^ssh-rsa",
+ c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS));
+ assertEquals("^ssh-rsa", c.getValue("PubkeyAcceptedKeyTypes"));
+ }
+
+ @Test
+ public void testEolComments() throws Exception {
+ config("#Comment\nHost=orcz #Comment\n\tPubkeyAcceptedAlgorithms ^ssh-rsa # Comment\n#Comment");
+ Host h = osc.lookup("orcz");
+ assertNotNull(h);
+ Config c = h.getConfig();
+ assertEquals("^ssh-rsa",
+ c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS));
+ }
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java
index 98c63cd..c514270 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java
@@ -23,7 +23,6 @@
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
-import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
@@ -224,8 +223,17 @@
entries.put(DEFAULT_NAME, defaults);
while ((line = reader.readLine()) != null) {
+ // OpenSsh ignores trailing comments on a line. Anything after the
+ // first # on a line is trimmed away (yes, even if the hash is
+ // inside quotes).
+ //
+ // See https://github.com/openssh/openssh-portable/commit/2bcbf679
+ int i = line.indexOf('#');
+ if (i >= 0) {
+ line = line.substring(0, i);
+ }
line = line.trim();
- if (line.isEmpty() || line.startsWith("#")) { //$NON-NLS-1$
+ if (line.isEmpty()) {
continue;
}
String[] parts = line.split("[ \t]*[= \t]", 2); //$NON-NLS-1$
@@ -484,12 +492,30 @@
LIST_KEYS.add(SshConstants.USER_KNOWN_HOSTS_FILE);
}
+ /**
+ * OpenSSH has renamed some config keys. This maps old names to new
+ * names.
+ */
+ private static final Map<String, String> ALIASES = new TreeMap<>(
+ String.CASE_INSENSITIVE_ORDER);
+
+ static {
+ // See https://github.com/openssh/openssh-portable/commit/ee9c0da80
+ ALIASES.put("PubkeyAcceptedKeyTypes", //$NON-NLS-1$
+ SshConstants.PUBKEY_ACCEPTED_ALGORITHMS);
+ }
+
private Map<String, String> options;
private Map<String, List<String>> multiOptions;
private Map<String, List<String>> listOptions;
+ private static String toKey(String key) {
+ String k = ALIASES.get(key);
+ return k != null ? k : key;
+ }
+
/**
* Retrieves the value of a single-valued key, or the first if the key
* has multiple values. Keys are case-insensitive, so
@@ -501,15 +527,15 @@
*/
@Override
public String getValue(String key) {
- String result = options != null ? options.get(key) : null;
+ String k = toKey(key);
+ String result = options != null ? options.get(k) : null;
if (result == null) {
// Let's be lenient and return at least the first value from
// a list-valued or multi-valued key.
- List<String> values = listOptions != null ? listOptions.get(key)
+ List<String> values = listOptions != null ? listOptions.get(k)
: null;
if (values == null) {
- values = multiOptions != null ? multiOptions.get(key)
- : null;
+ values = multiOptions != null ? multiOptions.get(k) : null;
}
if (values != null && !values.isEmpty()) {
result = values.get(0);
@@ -529,10 +555,11 @@
*/
@Override
public List<String> getValues(String key) {
- List<String> values = listOptions != null ? listOptions.get(key)
+ String k = toKey(key);
+ List<String> values = listOptions != null ? listOptions.get(k)
: null;
if (values == null) {
- values = multiOptions != null ? multiOptions.get(key) : null;
+ values = multiOptions != null ? multiOptions.get(k) : null;
}
if (values == null || values.isEmpty()) {
return new ArrayList<>();
@@ -551,34 +578,35 @@
* to set or add
*/
public void setValue(String key, String value) {
+ String k = toKey(key);
if (value == null) {
if (multiOptions != null) {
- multiOptions.remove(key);
+ multiOptions.remove(k);
}
if (listOptions != null) {
- listOptions.remove(key);
+ listOptions.remove(k);
}
if (options != null) {
- options.remove(key);
+ options.remove(k);
}
return;
}
- if (MULTI_KEYS.contains(key)) {
+ if (MULTI_KEYS.contains(k)) {
if (multiOptions == null) {
multiOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
- List<String> values = multiOptions.get(key);
+ List<String> values = multiOptions.get(k);
if (values == null) {
values = new ArrayList<>(4);
- multiOptions.put(key, values);
+ multiOptions.put(k, values);
}
values.add(value);
} else {
if (options == null) {
options = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
- if (!options.containsKey(key)) {
- options.put(key, value);
+ if (!options.containsKey(k)) {
+ options.put(k, value);
}
}
}
@@ -595,20 +623,21 @@
if (values.isEmpty()) {
return;
}
+ String k = toKey(key);
// Check multi-valued keys first; because of the replacement
// strategy, they must take precedence over list-valued keys
// which always follow the "first occurrence wins" strategy.
//
// Note that SendEnv is a multi-valued list-valued key. (It's
// rather immaterial for JGit, though.)
- if (MULTI_KEYS.contains(key)) {
+ if (MULTI_KEYS.contains(k)) {
if (multiOptions == null) {
multiOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
- List<String> items = multiOptions.get(key);
+ List<String> items = multiOptions.get(k);
if (items == null) {
items = new ArrayList<>(values);
- multiOptions.put(key, items);
+ multiOptions.put(k, items);
} else {
items.addAll(values);
}
@@ -616,8 +645,8 @@
if (listOptions == null) {
listOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
- if (!listOptions.containsKey(key)) {
- listOptions.put(key, values);
+ if (!listOptions.containsKey(k)) {
+ listOptions.put(k, values);
}
}
}
@@ -630,7 +659,7 @@
* @return {@code true} if the key is a list-valued key.
*/
public static boolean isListKey(String key) {
- return LIST_KEYS.contains(key.toUpperCase(Locale.ROOT));
+ return LIST_KEYS.contains(toKey(key));
}
void merge(HostEntry entry) {
