ssh config: do environment variable replacement

OpenSSH 8.4 has introduced simple environment variable substitution
for some keys. Implement that feature in our ssh config file parser,
too.

Bug: 572103
Change-Id: I360f2c5510eea4ec3329aeedf3d29dfefc9163f0
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 4c7e99ea80104c4e93678436ae543622d5e12179..4be2271a8cfb3afa4e0778cfc336da4ef5c1738e 100644 (file)
--- 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
@@ -58,6 +58,7 @@ public class OpenSshConfigTest extends RepositoryTestCase {
                FileUtils.mkdir(configFile.getParentFile());
 
                mockSystemReader.setProperty(Constants.OS_USER_NAME_KEY, "jex_junit");
+               mockSystemReader.setProperty("TST_VAR", "TEST");
                osc = new OpenSshConfig(home, configFile);
        }
 
@@ -497,4 +498,23 @@ public class OpenSshConfigTest extends RepositoryTestCase {
                assertEquals("^ssh-rsa",
                                c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS));
        }
+
+       @Test
+       public void testEnVarSubstitution() throws Exception {
+               config("Host orcz\nIdentityFile /tmp/${TST_VAR}\n"
+                               + "CertificateFile /tmp/${}/foo\nUser ${TST_VAR}\nIdentityAgent /tmp/${TST_VAR/bar");
+               Host h = osc.lookup("orcz");
+               assertNotNull(h);
+               Config c = h.getConfig();
+               assertEquals("/tmp/TEST",
+                               c.getValue(SshConstants.IDENTITY_FILE));
+               // No variable name
+               assertEquals("/tmp/${}/foo", c.getValue(SshConstants.CERTIFICATE_FILE));
+               // User doesn't get env var substitution:
+               assertEquals("${TST_VAR}", c.getValue(SshConstants.USER));
+               assertEquals("${TST_VAR}", h.getUser());
+               // Unterminated:
+               assertEquals("/tmp/${TST_VAR/bar",
+                               c.getValue(SshConstants.IDENTITY_AGENT));
+       }
 }

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 c514270f5b6f2e4b373574965c1f3f561047de09..de6a346cb298c39ef2d7375e5ba3cc456716da7b 100644 (file)
--- 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
@@ -708,10 +708,10 @@ public class OpenSshConfigFile implements SshConfigStore {
                }
 
                private List<String> substitute(List<String> values, String allowed,
-                               Replacer r) {
+                               Replacer r, boolean withEnv) {
                        List<String> result = new ArrayList<>(values.size());
                        for (String value : values) {
-                               result.add(r.substitute(value, allowed));
+                               result.add(r.substitute(value, allowed, withEnv));
                        }
                        return result;
                }
@@ -743,7 +743,7 @@ public class OpenSshConfigFile implements SshConfigStore {
                                if (hostName == null || hostName.isEmpty()) {
                                        options.put(SshConstants.HOST_NAME, originalHostName);
                                } else {
-                                       hostName = r.substitute(hostName, "h"); //$NON-NLS-1$
+                                       hostName = r.substitute(hostName, "h", false); //$NON-NLS-1$
                                        options.put(SshConstants.HOST_NAME, hostName);
                                        r.update('h', hostName);
                                }
@@ -752,13 +752,13 @@ public class OpenSshConfigFile implements SshConfigStore {
                                List<String> values = multiOptions
                                                .get(SshConstants.IDENTITY_FILE);
                                if (values != null) {
-                                       values = substitute(values, "dhlru", r); //$NON-NLS-1$
+                                       values = substitute(values, "dhlru", r, true); //$NON-NLS-1$
                                        values = replaceTilde(values, home);
                                        multiOptions.put(SshConstants.IDENTITY_FILE, values);
                                }
                                values = multiOptions.get(SshConstants.CERTIFICATE_FILE);
                                if (values != null) {
-                                       values = substitute(values, "dhlru", r); //$NON-NLS-1$
+                                       values = substitute(values, "dhlru", r, true); //$NON-NLS-1$
                                        values = replaceTilde(values, home);
                                        multiOptions.put(SshConstants.CERTIFICATE_FILE, values);
                                }
@@ -775,29 +775,29 @@ public class OpenSshConfigFile implements SshConfigStore {
                                // HOSTNAME already done above
                                String value = options.get(SshConstants.IDENTITY_AGENT);
                                if (value != null) {
-                                       value = r.substitute(value, "dhlru"); //$NON-NLS-1$
+                                       value = r.substitute(value, "dhlru", true); //$NON-NLS-1$
                                        value = toFile(value, home).getPath();
                                        options.put(SshConstants.IDENTITY_AGENT, value);
                                }
                                value = options.get(SshConstants.CONTROL_PATH);
                                if (value != null) {
-                                       value = r.substitute(value, "ChLlnpru"); //$NON-NLS-1$
+                                       value = r.substitute(value, "ChLlnpru", true); //$NON-NLS-1$
                                        value = toFile(value, home).getPath();
                                        options.put(SshConstants.CONTROL_PATH, value);
                                }
                                value = options.get(SshConstants.LOCAL_COMMAND);
                                if (value != null) {
-                                       value = r.substitute(value, "CdhlnprTu"); //$NON-NLS-1$
+                                       value = r.substitute(value, "CdhlnprTu", false); //$NON-NLS-1$
                                        options.put(SshConstants.LOCAL_COMMAND, value);
                                }
                                value = options.get(SshConstants.REMOTE_COMMAND);
                                if (value != null) {
-                                       value = r.substitute(value, "Cdhlnpru"); //$NON-NLS-1$
+                                       value = r.substitute(value, "Cdhlnpru", false); //$NON-NLS-1$
                                        options.put(SshConstants.REMOTE_COMMAND, value);
                                }
                                value = options.get(SshConstants.PROXY_COMMAND);
                                if (value != null) {
-                                       value = r.substitute(value, "hpr"); //$NON-NLS-1$
+                                       value = r.substitute(value, "hpr", false); //$NON-NLS-1$
                                        options.put(SshConstants.PROXY_COMMAND, value);
                                }
                        }
@@ -871,7 +871,7 @@ public class OpenSshConfigFile implements SshConfigStore {
                        replacements.put(Character.valueOf('r'), user == null ? "" : user); //$NON-NLS-1$
                        replacements.put(Character.valueOf('u'), localUserName);
                        replacements.put(Character.valueOf('C'),
-                                       substitute("%l%h%p%r", "hlpr")); //$NON-NLS-1$ //$NON-NLS-2$
+                                       substitute("%l%h%p%r", "hlpr", false)); //$NON-NLS-1$ //$NON-NLS-2$
                        replacements.put(Character.valueOf('T'), "NONE"); //$NON-NLS-1$
                }
 
@@ -879,36 +879,63 @@ public class OpenSshConfigFile implements SshConfigStore {
                        replacements.put(Character.valueOf(key), value);
                        if ("lhpr".indexOf(key) >= 0) { //$NON-NLS-1$
                                replacements.put(Character.valueOf('C'),
-                                               substitute("%l%h%p%r", "hlpr")); //$NON-NLS-1$ //$NON-NLS-2$
+                                               substitute("%l%h%p%r", "hlpr", false)); //$NON-NLS-1$ //$NON-NLS-2$
                        }
                }
 
-               public String substitute(String input, String allowed) {
+               public String substitute(String input, String allowed,
+                               boolean withEnv) {
                        if (input == null || input.length() <= 1
-                                       || input.indexOf('%') < 0) {
+                                       || input.indexOf('%') < 0
+                                                       && (!withEnv || input.indexOf("${") < 0)) { //$NON-NLS-1$
                                return input;
                        }
                        StringBuilder builder = new StringBuilder();
                        int start = 0;
                        int length = input.length();
                        while (start < length) {
-                               int percent = input.indexOf('%', start);
-                               if (percent < 0 || percent + 1 >= length) {
-                                       builder.append(input.substring(start));
+                               char ch = input.charAt(start);
+                               switch (ch) {
+                               case '%':
+                                       if (start + 1 >= length) {
+                                               break;
+                                       }
+                                       String replacement = null;
+                                       ch = input.charAt(start + 1);
+                                       if (ch == '%' || allowed.indexOf(ch) >= 0) {
+                                               replacement = replacements.get(Character.valueOf(ch));
+                                       }
+                                       if (replacement == null) {
+                                               builder.append('%').append(ch);
+                                       } else {
+                                               builder.append(replacement);
+                                       }
+                                       start += 2;
+                                       continue;
+                               case '$':
+                                       if (!withEnv || start + 2 >= length) {
+                                               break;
+                                       }
+                                       ch = input.charAt(start + 1);
+                                       if (ch == '{') {
+                                               int close = input.indexOf('}', start + 2);
+                                               if (close > start + 2) {
+                                                       String variable = SystemReader.getInstance()
+                                                                       .getenv(input.substring(start + 2, close));
+                                                       if (!StringUtils.isEmptyOrNull(variable)) {
+                                                               builder.append(variable);
+                                                       }
+                                                       start = close + 1;
+                                                       continue;
+                                               }
+                                       }
+                                       ch = '$';
+                                       break;
+                               default:
                                        break;
                                }
-                               String replacement = null;
-                               char ch = input.charAt(percent + 1);
-                               if (ch == '%' || allowed.indexOf(ch) >= 0) {
-                                       replacement = replacements.get(Character.valueOf(ch));
-                               }
-                               if (replacement == null) {
-                                       builder.append(input.substring(start, percent + 2));
-                               } else {
-                                       builder.append(input.substring(start, percent))
-                                                       .append(replacement);
-                               }
-                               start = percent + 2;
+                               builder.append(ch);
+                               start++;
                        }
                        return builder.toString();
                }