Merge pull request #2224 in ITERATE/cyberduck from bugfix/TRAC-11227-…

…passwordstore to master * commit '62d7bbd5e46b47d430e9b8b6cc239814c55649e7': Do not attempt to resolve hostname that may only be reachable in internal network from jump host. Save credentials for jump host in keychain. Review. Logging.
iterate-ch · Nov 25, 2020 · 0c1cd16 · 0c1cd16
2 parents eb9b3b9 + 62d7bbd
commit 0c1cd16
Show file tree

Hide file tree

Showing 15 changed files with 172 additions and 82 deletions.
diff --git a/core/src/main/java/ch/cyberduck/core/AbstractProtocol.java b/core/src/main/java/ch/cyberduck/core/AbstractProtocol.java
@@ -332,12 +332,17 @@ public int hashCode() {
 
     @Override
     public CredentialsConfigurator getCredentialsFinder() {
-        return new DisabledCredentialsConfigurator();
+        return CredentialsConfigurator.DISABLED;
     }
 
     @Override
     public HostnameConfigurator getHostnameFinder() {
-        return new DisabledHostnameConfigurator();
+        return HostnameConfigurator.DISABLED;
+    }
+
+    @Override
+    public JumphostConfigurator getJumpHostFinder() {
+        return JumphostConfigurator.DISABLED;
     }
 
     @Override

diff --git a/core/src/main/java/ch/cyberduck/core/CredentialsConfigurator.java b/core/src/main/java/ch/cyberduck/core/CredentialsConfigurator.java
@@ -28,4 +28,16 @@ public interface CredentialsConfigurator {
     Credentials configure(Host host);
 
     CredentialsConfigurator reload();
+
+    CredentialsConfigurator DISABLED = new CredentialsConfigurator() {
+        @Override
+        public Credentials configure(final Host host) {
+            return host.getCredentials();
+        }
+
+        @Override
+        public CredentialsConfigurator reload() {
+            return this;
+        }
+    };
 }
diff --git a/core/src/main/java/ch/cyberduck/core/DisabledCredentialsConfigurator.java b/core/src/main/java/ch/cyberduck/core/DisabledCredentialsConfigurator.java
diff --git a/core/src/main/java/ch/cyberduck/core/DisabledHostnameConfigurator.java b/core/src/main/java/ch/cyberduck/core/DisabledHostnameConfigurator.java
diff --git a/core/src/main/java/ch/cyberduck/core/HostnameConfigurator.java b/core/src/main/java/ch/cyberduck/core/HostnameConfigurator.java
@@ -35,4 +35,21 @@ public interface HostnameConfigurator {
     int getPort(String alias);
 
     HostnameConfigurator reload();
+
+    HostnameConfigurator DISABLED = new HostnameConfigurator() {
+        @Override
+        public String getHostname(String alias) {
+            return alias;
+        }
+
+        @Override
+        public int getPort(final String alias) {
+            return -1;
+        }
+
+        @Override
+        public HostnameConfigurator reload() {
+            return this;
+        }
+    };
 }
diff --git a/core/src/main/java/ch/cyberduck/core/JumpHostConfiguratorFactory.java b/core/src/main/java/ch/cyberduck/core/JumpHostConfiguratorFactory.java
@@ -0,0 +1,34 @@
+package ch.cyberduck.core;
+
+/*
+ * Copyright (c) 2012 David Kocher. All rights reserved.
+ * http://cyberduck.ch/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Bug fixes, suggestions and comments should be sent to:
+ * dkocher@cyberduck.ch
+ */
+
+public final class JumpHostConfiguratorFactory {
+
+    private JumpHostConfiguratorFactory() {
+        //
+    }
+
+    /**
+     * @param protocol Protocol
+     * @return Configurator for default settings
+     */
+    public static JumphostConfigurator get(final Protocol protocol) {
+        return protocol.getJumpHostFinder().reload();
+    }
+}
diff --git a/core/src/main/java/ch/cyberduck/core/JumphostConfigurator.java b/core/src/main/java/ch/cyberduck/core/JumphostConfigurator.java
@@ -0,0 +1,38 @@
+package ch.cyberduck.core;
+
+/*
+ * Copyright (c) 2012 David Kocher. All rights reserved.
+ * http://cyberduck.ch/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Bug fixes, suggestions and comments should be sent to:
+ * dkocher@cyberduck.ch
+ */
+
+public interface JumphostConfigurator {
+
+    Host getJumphost(String alias);
+
+    JumphostConfigurator reload();
+
+    JumphostConfigurator DISABLED = new JumphostConfigurator() {
+        @Override
+        public Host getJumphost(final String alias) {
+            return null;
+        }
+
+        @Override
+        public JumphostConfigurator reload() {
+            return this;
+        }
+    };
+}
diff --git a/core/src/main/java/ch/cyberduck/core/KeychainLoginService.java b/core/src/main/java/ch/cyberduck/core/KeychainLoginService.java
@@ -182,7 +182,7 @@ else if(credentials.isCertificateAuthentication()) {
                     keychain.save(bookmark);
                 }
                 catch(LocalAccessDeniedException e) {
-                    log.error(String.format("Failure saving credentials for %s in keychain. %s", session, e));
+                    log.error(String.format("Failure saving credentials for %s in keychain. %s", bookmark, e));
                 }
             }
             else {

diff --git a/core/src/main/java/ch/cyberduck/core/LoginConnectionService.java b/core/src/main/java/ch/cyberduck/core/LoginConnectionService.java
@@ -129,25 +129,26 @@ public void connect(final Session<?> session, final CancelCallback callback) thr
         }
         final Host bookmark = session.getHost();
         // Try to resolve the hostname first
-        final HostnameConfigurator configurator = HostnameConfiguratorFactory.get(bookmark.getProtocol());
-        final String hostname = configurator.getHostname(bookmark.getHostname());
-        listener.message(MessageFormat.format(LocaleFactory.localizedString("Resolving {0}", "Status"),
-            hostname));
-        final Proxy proxyhost = proxy.find(bookmark);
-        if(proxyhost == Proxy.DIRECT) {
+        final String hostname = HostnameConfiguratorFactory.get(bookmark.getProtocol()).getHostname(bookmark.getHostname());
+        listener.message(MessageFormat.format(LocaleFactory.localizedString("Resolving {0}", "Status"), hostname));
+        final Proxy proxy = this.proxy.find(bookmark);
+        if(proxy == Proxy.DIRECT) {
             // Only try to resolve target hostname if direct connection
-            try {
-                resolver.resolve(hostname, callback);
-            }
-            catch(ResolveFailedException e) {
-                log.warn(String.format("DNS resolver failed for %s", hostname));
-                throw e;
+            if(null == JumpHostConfiguratorFactory.get(bookmark.getProtocol()).getJumphost(bookmark.getHostname())) {
+                // Do not attempt to resolve hostname that may only be reachable in internal network from jump host
+                try {
+                    resolver.resolve(hostname, callback);
+                }
+                catch(ResolveFailedException e) {
+                    log.warn(String.format("DNS resolver failed for %s", hostname));
+                    throw e;
+                }
             }
         }
         listener.message(MessageFormat.format(LocaleFactory.localizedString("Opening {0} connection to {1}", "Status"),
             bookmark.getProtocol().getName(), hostname));
         // The IP address could successfully be determined
-        session.open(proxyhost, key, prompt);
+        session.open(proxy, key, prompt);
         listener.message(MessageFormat.format(LocaleFactory.localizedString("{0} connection opened", "Status"),
             bookmark.getProtocol().getName()));
         // Update last accessed timestamp
@@ -166,7 +167,7 @@ public void connect(final Session<?> session, final CancelCallback callback) thr
         }
         // Login
         try {
-            this.authenticate(proxyhost, session, callback);
+            this.authenticate(proxy, session, callback);
         }
         catch(BackgroundException e) {
             this.close(session);

diff --git a/core/src/main/java/ch/cyberduck/core/Profile.java b/core/src/main/java/ch/cyberduck/core/Profile.java
@@ -289,6 +289,11 @@ public HostnameConfigurator getHostnameFinder() {
         return parent.getHostnameFinder();
     }
 
+    @Override
+    public JumphostConfigurator getJumpHostFinder() {
+        return parent.getJumpHostFinder();
+    }
+
     @Override
     public Case getCaseSensitivity() {
         return parent.getCaseSensitivity();

diff --git a/core/src/main/java/ch/cyberduck/core/Protocol.java b/core/src/main/java/ch/cyberduck/core/Protocol.java
@@ -35,10 +35,21 @@ public interface Protocol extends Comparable<Protocol> {
      */
     boolean validate(Credentials credentials, LoginOptions options);
 
+    /**
+     * @return Configurator for resolving credentials for bookmark
+     */
     CredentialsConfigurator getCredentialsFinder();
 
+    /**
+     * @return Configurator for resolving hostname from alias
+     */
     HostnameConfigurator getHostnameFinder();
 
+    /**
+     * @return Configurator for resolving jump host configuration for bookmark
+     */
+    JumphostConfigurator getJumpHostFinder();
+
     /**
      * @return Case sensitivity of system
      */

diff --git a/ssh/src/main/java/ch/cyberduck/core/sftp/SFTPProtocol.java b/ssh/src/main/java/ch/cyberduck/core/sftp/SFTPProtocol.java
@@ -20,17 +20,20 @@
 import ch.cyberduck.core.AbstractProtocol;
 import ch.cyberduck.core.CredentialsConfigurator;
 import ch.cyberduck.core.HostnameConfigurator;
+import ch.cyberduck.core.JumphostConfigurator;
 import ch.cyberduck.core.LocaleFactory;
 import ch.cyberduck.core.Scheme;
 import ch.cyberduck.core.sftp.openssh.OpenSSHCredentialsConfigurator;
 import ch.cyberduck.core.sftp.openssh.OpenSSHHostnameConfigurator;
+import ch.cyberduck.core.sftp.openssh.OpenSSHJumpHostConfigurator;
 
 import org.apache.commons.lang3.StringUtils;
 
 public class SFTPProtocol extends AbstractProtocol {
 
     private final CredentialsConfigurator credentials = new OpenSSHCredentialsConfigurator();
     private final HostnameConfigurator hostnmame = new OpenSSHHostnameConfigurator();
+    private final JumphostConfigurator jumphost = new OpenSSHJumpHostConfigurator();
 
     @Override
     public Type getType() {
@@ -81,4 +84,9 @@ public CredentialsConfigurator getCredentialsFinder() {
     public HostnameConfigurator getHostnameFinder() {
         return hostnmame;
     }
+
+    @Override
+    public JumphostConfigurator getJumpHostFinder() {
+        return jumphost;
+    }
 }
diff --git a/ssh/src/main/java/ch/cyberduck/core/sftp/SFTPSession.java b/ssh/src/main/java/ch/cyberduck/core/sftp/SFTPSession.java
@@ -26,6 +26,7 @@
 import ch.cyberduck.core.exception.ConnectionCanceledException;
 import ch.cyberduck.core.exception.ConnectionRefusedException;
 import ch.cyberduck.core.exception.InteroperabilityException;
+import ch.cyberduck.core.exception.LocalAccessDeniedException;
 import ch.cyberduck.core.exception.LoginCanceledException;
 import ch.cyberduck.core.exception.LoginFailureException;
 import ch.cyberduck.core.features.*;
@@ -146,6 +147,18 @@ protected SSHClient connect(final HostKeyCallback key, final LoginCallback promp
                 proxy.setCredentials(new OpenSSHCredentialsConfigurator().configure(proxy));
                 // Authenticate with jump host
                 this.authenticate(hop, proxy, prompt, new DisabledCancelCallback());
+                if(log.isDebugEnabled()) {
+                    log.debug(String.format("Authenticated with jump host %s", proxy));
+                }
+                if(proxy.getCredentials().isSaved()) {
+                    // Write credentials to keychain
+                    try {
+                        PasswordStoreFactory.get().save(proxy);
+                    }
+                    catch(LocalAccessDeniedException e) {
+                        log.error(String.format("Failure saving credentials for %s in keychain. %s", proxy, e));
+                    }
+                }
                 final DirectConnection tunnel = hop.newDirectConnection(
                     new OpenSSHHostnameConfigurator().getHostname(host.getHostname()), host.getPort());
                 // Connect to internal host

diff --git a/ssh/src/main/java/ch/cyberduck/core/sftp/auth/SFTPPasswordAuthentication.java b/ssh/src/main/java/ch/cyberduck/core/sftp/auth/SFTPPasswordAuthentication.java
@@ -75,7 +75,7 @@ public String getMethod() {
         return "password";
     }
 
-    public boolean authenticate(final Host host, final Credentials credentials, final LoginCallback callback, final CancelCallback cancel)
+    private boolean authenticate(final Host host, final Credentials credentials, final LoginCallback callback, final CancelCallback cancel)
         throws BackgroundException {
         if(log.isDebugEnabled()) {
             log.debug(String.format("Login using password authentication with credentials %s", credentials));

diff --git a/ssh/src/main/java/ch/cyberduck/core/sftp/openssh/OpenSSHJumpHostConfigurator.java b/ssh/src/main/java/ch/cyberduck/core/sftp/openssh/OpenSSHJumpHostConfigurator.java
@@ -15,6 +15,7 @@
 
 import ch.cyberduck.core.Host;
 import ch.cyberduck.core.HostParser;
+import ch.cyberduck.core.JumphostConfigurator;
 import ch.cyberduck.core.LocalFactory;
 import ch.cyberduck.core.ProtocolFactory;
 import ch.cyberduck.core.exception.HostParserException;
@@ -26,7 +27,7 @@
 
 import java.util.Collections;
 
-public class OpenSSHJumpHostConfigurator {
+public class OpenSSHJumpHostConfigurator implements JumphostConfigurator {
     private static final Logger log = Logger.getLogger(OpenSSHJumpHostConfigurator.class);
 
     private final OpenSshConfig configuration;
@@ -43,6 +44,7 @@ public OpenSSHJumpHostConfigurator(final OpenSshConfig configuration) {
         this.credentials = new OpenSSHCredentialsConfigurator(configuration);
     }
 
+    @Override
     public Host getJumphost(final String alias) {
         if(StringUtils.isBlank(alias)) {
             return null;
@@ -68,4 +70,11 @@ public Host getJumphost(final String alias) {
             return null;
         }
     }
+
+    @Override
+    public JumphostConfigurator reload() {
+        hostname.reload();
+        credentials.reload();
+        return this;
+    }
 }