Skip to content

Commit 5f7dd90

Browse files
Mi-Lajanvrany
authored andcommitted
Fixes a deadlock when a wrong passphrase is associated with a private key.
1 parent 1c25e27 commit 5f7dd90

File tree

1 file changed

+31
-16
lines changed

1 file changed

+31
-16
lines changed

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

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@
5151
* An implementation that uses native SSH agent installed on a system.
5252
*/
5353
public class ExecRemoteAgent implements RemoteAgent {
54-
private final String AuthSocketVar = "SSH_AUTH_SOCK";
55-
private final String AgentPidVar = "SSH_AGENT_PID";
54+
private static final String AuthSocketVar = "SSH_AUTH_SOCK";
55+
private static final String AgentPidVar = "SSH_AGENT_PID";
5656

5757
/** Process builder keeping environment for all ExecRemoteAgent related processes. */
5858
private final ProcessBuilder processBuilder;
@@ -117,11 +117,13 @@ public void addIdentity(String privateKey, final String passphrase, String comme
117117
processBuilder.environment().put("DISPLAY", ":0"); // just to force using SSH_ASKPASS
118118
processBuilder.environment().put("SSH_ASKPASS", askpass.getPath());
119119

120-
// TODO: kill agent in case of wrong password
121-
122120
final Process sshAdd = execProcess("ssh-add " + keyFile.getPath());
123121

124-
IOUtils.copy(sshAdd.getErrorStream(), listener.getLogger());
122+
String errorString = streamToString(sshAdd.getErrorStream());
123+
if (!errorString.isEmpty()) {
124+
errorString += "Check the passphrase for the private key.";
125+
listener.getLogger().println(errorString);
126+
}
125127
IOUtils.copy(sshAdd.getInputStream(), listener.getLogger());
126128

127129
try {
@@ -135,7 +137,7 @@ public void addIdentity(String privateKey, final String passphrase, String comme
135137
processBuilder.environment().remove("DISPLAY");
136138
processBuilder.environment().remove("SSH_PASSPHRASE");
137139

138-
askpass.delete();
140+
askpass.delete(); // the ASKPASS script is self-deleting, anyway rather try to delete it in case of some error
139141

140142
if (!keyFile.delete()) {
141143
listener.getLogger().println("ExecRemoteAgent::addIdentity - could NOT delete a temp file with a private key!");
@@ -167,16 +169,19 @@ private Process execProcess(String command) throws IOException {
167169
return process;
168170
}
169171

172+
private String streamToString(InputStream is) throws IOException {
173+
ByteArrayOutputStream os = new ByteArrayOutputStream();
174+
IOUtils.copy(is, os);
175+
return os.toString();
176+
}
177+
170178
/**
171179
* Parses ssh-agent output.
172180
*/
173181
private Map<String,String> parseAgentEnv(Process agent) throws Exception{
174182
Map<String, String> env = new HashMap<String, String>();
175183

176-
InputStream agentOutputReader = agent.getInputStream();
177-
ByteArrayOutputStream agentOutputStream = new ByteArrayOutputStream();
178-
IOUtils.copy(agentOutputReader, agentOutputStream);
179-
String agentOutput = agentOutputStream.toString();
184+
String agentOutput = streamToString(agent.getInputStream());
180185

181186
// get SSH_AUTH_SOCK
182187
env.put(AuthSocketVar, getAgentValue(agentOutput, AuthSocketVar));
@@ -198,6 +203,9 @@ private String getAgentValue(String agentOutput, String envVar) {
198203
return agentOutput.substring(pos, end);
199204
}
200205

206+
/**
207+
* Sets file's permissions to readable only for an owner.
208+
*/
201209
private boolean setReadOnlyForOwner(File file) {
202210
boolean ok = file.setExecutable(false, false);
203211
ok &= file.setWritable(false, false);
@@ -206,17 +214,24 @@ private boolean setReadOnlyForOwner(File file) {
206214
return ok;
207215
}
208216

217+
/**
218+
* Creates a self-deleting script for SSH_ASKPASS. Self-deleting to be able to detect a wrong passphrase.
219+
*/
209220
private File createAskpassScript() throws IOException {
221+
// TODO: assuming that ssh-add runs the script in shell even on Windows, not cmd
222+
// for cmd following could work
223+
// suffix = ".bat";
224+
// script = "@ECHO %SSH_PASSPHRASE%\nDEL \"" + askpass.getAbsolutePath() + "\"\n";
225+
210226
final String suffix;
211227
final String script;
212-
// TODO: assuming that ssh-add runs the script in shell, not cmd
228+
213229
suffix = ".sh";
214-
script = "#!/bin/sh\necho $SSH_PASSPHRASE\n";
215-
// for cmd following should work
216-
// suffix = ".bat";
217-
// script = "@echo %SSH_PASSPHRASE%\n";
218-
230+
219231
File askpass = File.createTempFile("askpass_", suffix);
232+
233+
script = "#!/bin/sh\necho $SSH_PASSPHRASE\nrm " + askpass.getAbsolutePath().replace("\\", "\\\\") + "\n";
234+
220235
FileWriter askpassWriter = new FileWriter(askpass);
221236
askpassWriter.write(script);
222237
askpassWriter.close();

0 commit comments

Comments
 (0)