From 6faee128f8930b851d33f1f06cb77b3e1b9a0cc5 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 19 Mar 2021 09:24:31 +0100 Subject: 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 --- .../eclipse/jgit/transport/OpenSshConfigTest.java | 30 +++++++++ .../internal/transport/ssh/OpenSshConfigFile.java | 73 +++++++++++++++------- 2 files changed, 81 insertions(+), 22 deletions(-) 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 af09f499f5..4c7e99ea80 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 @@ public class OpenSshConfigTest extends RepositoryTestCase { 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 98c63cdcdd..c514270f5b 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.Collections; 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 @@ public class OpenSshConfigFile implements SshConfigStore { 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 @@ public class OpenSshConfigFile implements SshConfigStore { 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 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 options; private Map> multiOptions; private Map> 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 @@ public class OpenSshConfigFile implements SshConfigStore { */ @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 values = listOptions != null ? listOptions.get(key) + List 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 @@ public class OpenSshConfigFile implements SshConfigStore { */ @Override public List getValues(String key) { - List values = listOptions != null ? listOptions.get(key) + String k = toKey(key); + List 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 @@ public class OpenSshConfigFile implements SshConfigStore { * 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 values = multiOptions.get(key); + List 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 @@ public class OpenSshConfigFile implements SshConfigStore { 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 items = multiOptions.get(key); + List 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 @@ public class OpenSshConfigFile implements SshConfigStore { 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 @@ public class OpenSshConfigFile implements SshConfigStore { * @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) { -- cgit v1.2.3