Simplified ExecRemoteAgent to use Launcher & FilePath for all remote calls.

Author: jglick

src/main/java/com/cloudbees/jenkins/plugins/sshagent/RemoteAgent.java

@@ -45,10 +45,10 @@ public interface RemoteAgent {
      * @param comment    the comment to give to the key.
      * @throws java.io.IOException if something went wrong.
      */
-    void addIdentity(String privateKey, String passphrase, String comment) throws IOException;
+    void addIdentity(String privateKey, String passphrase, String comment) throws IOException, InterruptedException;
 
     /**
      * Stops the agent.
      */
-    void stop();
+    void stop() throws IOException, InterruptedException;
 }

src/main/java/com/cloudbees/jenkins/plugins/sshagent/SSHAgentBuildWrapper.java

@@ -391,7 +391,7 @@ public SSHAgentEnvironment(Launcher launcher, BuildListener listener, @CheckForN
          * @throws IOException if the key cannot be added.
          * @since 1.9
          */
-        public void add(SSHUserPrivateKey key) throws IOException {
+        public void add(SSHUserPrivateKey key) throws IOException, InterruptedException {
             final Secret passphrase = key.getPassphrase();
             final String effectivePassphrase = passphrase == null ? null : passphrase.getPlainText();
             for (String privateKey : key.getPrivateKeys()) {

src/main/java/com/cloudbees/jenkins/plugins/sshagent/SSHAgentStepExecution.java

@@ -82,8 +82,9 @@ public void onResume() {
         try {
             purgeSockets();
             initRemoteAgent();
-        } catch (IOException io) {
+        } catch (IOException | InterruptedException x) {
             listener.getLogger().println(Messages.SSHAgentBuildWrapper_CouldNotStartAgent());
+            x.printStackTrace(listener.getLogger());
         }
     }
 
@@ -92,7 +93,7 @@ static FilePath tempDir(FilePath ws) {
         return ws.sibling(ws.getName() + System.getProperty(WorkspaceList.class.getName(), "@") + "tmp");
     }
 
-    private static class Callback extends BodyExecutionCallback {
+    private static class Callback extends BodyExecutionCallback.TailCall {
 
         private static final long serialVersionUID = 1L;
 
@@ -103,15 +104,8 @@ private static class Callback extends BodyExecutionCallback {
         }
 
         @Override
-        public void onSuccess(StepContext context, Object result) {
+        protected void finished(StepContext context) throws Exception {
             execution.cleanUp();
-            context.onSuccess(result);
-        }
-
-        @Override
-        public void onFailure(StepContext context, Throwable t) {
-            execution.cleanUp();
-            context.onFailure(t);
         }
 
     }
@@ -137,7 +131,7 @@ public void expand(EnvVars env) throws IOException, InterruptedException {
      *
      * @throws IOException
      */
-    private void initRemoteAgent() throws IOException {
+    private void initRemoteAgent() throws IOException, InterruptedException {
 
         List<SSHUserPrivateKey> userPrivateKeys = new ArrayList<SSHUserPrivateKey>();
         for (String id : new LinkedHashSet<String>(step.getCredentials())) {
@@ -197,17 +191,16 @@ private void initRemoteAgent() throws IOException {
     /**
      * Shuts down the current SSH Agent and purges socket files.
      */
-    private void cleanUp() {
+    private void cleanUp() throws Exception {
         try {
             TaskListener listener = getContext().get(TaskListener.class);
             if (agent != null) {
                 agent.stop();
                 listener.getLogger().println(Messages.SSHAgentBuildWrapper_Stopped());
             }
-        } catch (Throwable th) {
-            getContext().onFailure(th);
+        } finally {
+            purgeSockets();
         }
-        purgeSockets();
     }
 
     /**

src/main/java/com/cloudbees/jenkins/plugins/sshagent/exec/ExecRemoteAgent.java

@@ -1,7 +1,7 @@
 /*
  * The MIT License
  *
- * Copyright (c) 2014, Eccam s.r.o., Milan Kriz
+ * Copyright (c) 2014, Eccam s.r.o., Milan Kriz, CloudBees Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -25,27 +25,16 @@
 package com.cloudbees.jenkins.plugins.sshagent.exec;
 
 import com.cloudbees.jenkins.plugins.sshagent.RemoteAgent;
-
+import hudson.AbortException;
+import hudson.FilePath;
+import hudson.Launcher;
 import hudson.model.TaskListener;
-
-import java.lang.Process;
-import java.lang.ProcessBuilder;
-
-import java.util.Map;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Arrays;
-
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.OutputStream;
-import java.io.OutputStreamWriter;
-import java.io.Writer;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
-import org.apache.commons.io.IOUtils;
 
 /**
  * An implementation that uses native SSH agent installed on a system.
@@ -54,49 +43,44 @@ public class ExecRemoteAgent implements RemoteAgent {
     private static final String AuthSocketVar = "SSH_AUTH_SOCK";
     private static final String AgentPidVar = "SSH_AGENT_PID";
 
-    /** Process builder keeping environment for all ExecRemoteAgent related processes. */
-    private final ProcessBuilder processBuilder;
+    private final Launcher launcher;
 
     /**
      * The listener in case we need to report exceptions
      */
     private final TaskListener listener;
-    
-    /**
-     * Process in which the ssh-agent is running.
-     */
-    private final Process agent;
+
+    private final FilePath temp;
 
     /**
      * The socket bound by the agent.
      */
     private final String socket;
 
-    /**
-     * Constructor.
-     *
-     * @param listener the listener.
-     * @throws Exception if the agent could not start.
-     */
-    public ExecRemoteAgent(TaskListener listener) throws Exception {
-        this.processBuilder = new ProcessBuilder();
+    /** Agent environment used for {@code ssh-add} and {@code ssh-agent -k}. */
+    private final Map<String, String> agentEnv;
+
+    ExecRemoteAgent(Launcher launcher, TaskListener listener, FilePath temp) throws Exception {
+        this.launcher = launcher;
         this.listener = listener;
-        
-        this.agent = execProcess("ssh-agent");
-        Map<String, String> agentEnv = parseAgentEnv(this.agent);
+        this.temp = temp;
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        if (launcher.launch().cmds("ssh-agent").stdout(baos).start().joinWithTimeout(1, TimeUnit.MINUTES, listener) != 0) {
+            throw new AbortException("Failed to run ssh-agent");
+        }
+        agentEnv = parseAgentEnv(baos.toString()); // TODO could include local filenames, better to look up remote charset
 
         if (agentEnv.containsKey(AuthSocketVar))
             socket = agentEnv.get(AuthSocketVar);
         else
             socket = ""; // socket is not set
-        
-        // set agent environment to the process builder to provide it to ssh-add which will be executed later
-        processBuilder.environment().putAll(agentEnv);
     }
 
     /**
      * {@inheritDoc}
      */
+    @Override
     public String getSocket() {
         return socket;
     }
@@ -105,80 +89,46 @@ public String getSocket() {
      * {@inheritDoc}
      */
     @Override
-    public void addIdentity(String privateKey, final String passphrase, String comment) throws IOException {
-        File keyFile = File.createTempFile("private_key_", ".key");
-        try (FileOutputStream os = new FileOutputStream(keyFile); Writer keyWriter = new OutputStreamWriter(os, StandardCharsets.US_ASCII)) {
-            keyWriter.write(privateKey);
-        }
-        setReadOnlyForOwner(keyFile);
-        
-        File askpass = createAskpassScript();
-        
-        processBuilder.environment().put("SSH_PASSPHRASE", passphrase);
-        processBuilder.environment().put("DISPLAY", ":0"); // just to force using SSH_ASKPASS
-        processBuilder.environment().put("SSH_ASKPASS", askpass.getPath());
-
-        final Process sshAdd = execProcess("ssh-add " + keyFile.getPath());
-        
-        String errorString = IOUtils.toString(sshAdd.getErrorStream());
-        if (!errorString.isEmpty()) {
-            errorString += "Check the passphrase for the private key.";
-            listener.getLogger().println(errorString);
-        }
-        IOUtils.copy(sshAdd.getInputStream(), listener.getLogger()); // default encoding appropriate here: local process output
-        
+    public void addIdentity(String privateKey, final String passphrase, String comment) throws IOException, InterruptedException {
+        FilePath keyFile = temp.createTextTempFile("private_key_", ".key", privateKey);
         try {
-            sshAdd.waitFor();
-        }
-        catch (InterruptedException e) {
-            // waiting or process somehow interrupted
-        }
-        
-        processBuilder.environment().remove("SSH_ASKPASS");
-        processBuilder.environment().remove("DISPLAY");
-        processBuilder.environment().remove("SSH_PASSPHRASE");
-        
-        if (askpass.isFile() && !askpass.delete()) { // the ASKPASS script is self-deleting, anyway rather try to delete it in case of some error
-            listener.getLogger().println("ExecRemoteAgent::addIdentity - failed to delete " + askpass);
-        }
-        
-        if (!keyFile.delete()) {
-            listener.getLogger().println("ExecRemoteAgent::addIdentity - could NOT delete a temp file with a private key!");
+            keyFile.chmod(0600);
+
+            FilePath askpass = createAskpassScript();
+            try {
+
+                Map<String,String> env = new HashMap<>(agentEnv);
+                env.put("SSH_PASSPHRASE", passphrase);
+                env.put("DISPLAY", ":0"); // just to force using SSH_ASKPASS
+                env.put("SSH_ASKPASS", askpass.getRemote());
+                if (launcher.launch().cmds("ssh-add", keyFile.getRemote()).envs(env).stdout(listener).start().joinWithTimeout(1, TimeUnit.MINUTES, listener) != 0) {
+                    throw new AbortException("Failed to run ssh-add");
+                }
+            } finally {
+                if (askpass.exists()) { // the ASKPASS script is self-deleting, anyway rather try to delete it in case of some error
+                    askpass.delete();
+                }
+            }
+        } finally {
+            keyFile.delete();
         }
     }
 
     /**
      * {@inheritDoc}
      */
-    public void stop() {
-        try {
-            execProcess("ssh-agent -k");
-        }
-        catch (IOException e) {
-            listener.error("ExecRemoteAgent::stop - " + e.getCause());
+    @Override
+    public void stop() throws IOException, InterruptedException {
+        if (launcher.launch().cmds("ssh-agent", "-k").envs(agentEnv).stdout(listener).start().joinWithTimeout(1, TimeUnit.MINUTES, listener) != 0) {
+            throw new AbortException("Failed to run ssh-agent -k");
         }
-        agent.destroy();
-    }
-    
-    /**
-     * Executes a new process using ProcessBuilder with custom environment variables.
-     */
-    private Process execProcess(String command) throws IOException {
-        listener.getLogger().println("ExecRemoteAgent::execProcess - " + command);
-        List<String> command_list = Arrays.asList(command.split(" "));
-        processBuilder.command(command_list);
-        Process process = processBuilder.start();
-        processBuilder.command("");
-        return process;
     }
 
     /**
      * Parses ssh-agent output.
      */
-    private Map<String,String> parseAgentEnv(Process agent) throws Exception{
-        Map<String, String> env = new HashMap<String, String>();
-        
-        String agentOutput = IOUtils.toString(agent.getInputStream()); // default encoding appropriate here: local filenames
+    private Map<String,String> parseAgentEnv(String agentOutput) throws Exception{
+        Map<String, String> env = new HashMap<>();
 
         // get SSH_AUTH_SOCK
         env.put(AuthSocketVar, getAgentValue(agentOutput, AuthSocketVar));
@@ -196,46 +146,23 @@ private Map<String,String> parseAgentEnv(Process agent) throws Exception{
      */
     private String getAgentValue(String agentOutput, String envVar) {
         int pos = agentOutput.indexOf(envVar) + envVar.length() + 1; // +1 for '='
-        int end = agentOutput.indexOf(";", pos);
+        int end = agentOutput.indexOf(';', pos);
         return agentOutput.substring(pos, end);
     }
 
-    /**
-     * Sets file's permissions to readable only for an owner.
-     */
-    private boolean setReadOnlyForOwner(File file) {
-        boolean ok = file.setExecutable(false, false);
-        ok &= file.setWritable(false, false);
-        ok &= file.setReadable(false, false);
-        ok &= file.setReadable(true, true);
-        return ok;
-    }
-    
     /**
      * Creates a self-deleting script for SSH_ASKPASS. Self-deleting to be able to detect a wrong passphrase. 
      */
-    private File createAskpassScript() throws IOException {
+    private FilePath createAskpassScript() throws IOException, InterruptedException {
         // TODO: assuming that ssh-add runs the script in shell even on Windows, not cmd
         //       for cmd following could work
         //       suffix = ".bat";
         //       script = "@ECHO %SSH_PASSPHRASE%\nDEL \"" + askpass.getAbsolutePath() + "\"\n";
 
-        final String suffix;
-        final String script;
-
-        suffix = ".sh";
-
-        File askpass = File.createTempFile("askpass_", suffix);
+        FilePath askpass = temp.createTextTempFile("askpass_", ".sh", "#!/bin/sh\necho $SSH_PASSPHRASE\nrm $0\n");
 
-        script = "#!/bin/sh\necho $SSH_PASSPHRASE\nrm " + askpass.getAbsolutePath().replace("\\", "\\\\") + "\n"; // TODO try using `rm $0` instead
-
-        try (OutputStream os = new FileOutputStream(askpass); Writer askpassWriter = new OutputStreamWriter(os, /* due to presence of a local filename */Charset.defaultCharset())) {
-            askpassWriter.write(script);
-        }
-        
         // executable only for a current user
-        askpass.setExecutable(false, false);
-        askpass.setExecutable(true, true);
+        askpass.chmod(0700);
         return askpass;
     }
 }

src/main/java/com/cloudbees/jenkins/plugins/sshagent/exec/ExecRemoteAgentFactory.java

@@ -32,6 +32,7 @@
 import hudson.model.TaskListener;
 
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
 
 
 
@@ -55,7 +56,7 @@ public String getDisplayName() {
     @Override
     public boolean isSupported(Launcher launcher, final TaskListener listener) {
         try {             
-            int status = launcher.launch().cmds("ssh-agent", "-k").join();
+            int status = launcher.launch().cmds("ssh-agent", "-k").quiet(true).start().joinWithTimeout(1, TimeUnit.MINUTES, listener);
             /* 
              * `ssh-agent -k` returns 0 if terminates running agent or 1 if
              * it fails to terminate it. On Linux, 
@@ -78,6 +79,6 @@ public boolean isSupported(Launcher launcher, final TaskListener listener) {
      */
     @Override
     public RemoteAgent start(Launcher launcher, final TaskListener listener, FilePath temp) throws Throwable {
-        return launcher.getChannel().call(new ExecRemoteAgentStarter(listener));
+        return new ExecRemoteAgent(launcher, listener, temp);
     }
 }