Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git ssh private key binding(GSoC-21) #1111

Open
wants to merge 106 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
106 commits
Select commit Hold shift + click to select a range
e9c550a
Adding dependency
arpoch Jul 19, 2021
4fd5c4c
Bumping git-client plugin version
arpoch Jul 19, 2021
e212567
SSHPrivateKey Binding
arpoch Jul 19, 2021
78a47fb
Git Binding interface
arpoch Jul 19, 2021
818599a
Merge remote-tracking branch 'origin/master' into gitSSHPrivateKey
arpoch Jul 21, 2021
6eeecee
Adding dependency to support Git SSH binding
arpoch Jul 21, 2021
2bc3140
Contract to support Git SSH binding implementation
arpoch Jul 21, 2021
e45ed33
Support for OpenSSH key format
arpoch Jul 21, 2021
9030a2c
Adding display-name property for Git SSH binding
arpoch Jul 21, 2021
9312a70
Inline help for Git SSH credentials-id
arpoch Jul 21, 2021
a8f38f3
Inline help for Git SSH git-Tool-name
arpoch Jul 21, 2021
9e17779
Adding config file for Git SSH binding
arpoch Jul 21, 2021
b53385b
ListBox for git-tool-name
arpoch Jul 21, 2021
588c389
Check current node os environment
arpoch Jul 21, 2021
d70831c
Change to non-static inner class
arpoch Jul 21, 2021
443c0f7
Check if current git version is at-least the required version
arpoch Jul 21, 2021
65624c0
Adding imports
arpoch Jul 21, 2021
a55e3dd
Assert launcher
arpoch Jul 21, 2021
d38eea0
Code formatting
arpoch Jul 21, 2021
f57be59
Create SSH command argument
arpoch Jul 21, 2021
df3d33c
Additional parameter to tell current node os type
arpoch Jul 21, 2021
0996235
Bind Git SSH environment variable
arpoch Jul 21, 2021
47588ae
Code formatting
arpoch Jul 21, 2021
7549c54
Merge branch 'jenkinsci:master' into gitSSHPrivateKey
arpoch Jul 21, 2021
d1eeb22
Downgrading plugin artifact version from 20 to 19
arpoch Jul 23, 2021
96c0103
Consuming git-client incremental
arpoch Jul 23, 2021
d3ee23a
Using getSSHExecutable api
arpoch Jul 23, 2021
c218b0b
Replacing PEMWriter with JcaPEMWriter
arpoch Jul 23, 2021
d596240
Adding support for SSH binding in Windows
arpoch Jul 23, 2021
8922ada
Store SSH exe path in local variable
arpoch Jul 23, 2021
a44b388
Removing redundant code and fixing errors
arpoch Jul 23, 2021
a42ac94
Fixing private key value
arpoch Jul 23, 2021
598eb80
Replacing private key with passphrase value
arpoch Jul 23, 2021
5c47934
Set node type before calling getSSHExePathInWin
arpoch Jul 25, 2021
752a94a
Wrapper method for getSSHExePathInWin
arpoch Jul 25, 2021
035e6c2
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Jul 25, 2021
dcba7c9
Merge branch 'master' into gitSSHPrivateKey
arpoch Jul 26, 2021
fbba6f9
Merge branch 'master' into gitSSHPrivateKey
arpoch Jul 31, 2021
69fafe5
Format private key to retrieve base64 encoded
arpoch Jul 31, 2021
ee925a7
Convert private key string to keypair
arpoch Jul 31, 2021
4a5e7de
Bumping Jenkins version to 2.289.1
arpoch Aug 4, 2021
63680a0
Adding sshd-core dependency to support
arpoch Aug 4, 2021
8c525eb
Set file permission to readonly
arpoch Aug 4, 2021
4ec3a3d
Accessing methods as class methods
arpoch Aug 4, 2021
638989b
Handling null point exception
arpoch Aug 4, 2021
e830aef
Adhering to lower case letter consistency in symbol
arpoch Aug 4, 2021
be399fb
Header markers for openssh private key format
arpoch Aug 4, 2021
73b01ba
Using header markers to check if private key openssh formatted
arpoch Aug 4, 2021
7ef7820
Write decrypted openssh formatted private key
arpoch Aug 4, 2021
b419b4c
Using header markers to retrieve key body
arpoch Aug 4, 2021
9bc6d3a
Wrapper method to use writePrivateKeyOpenSSHFormatted method
arpoch Aug 4, 2021
82022ef
Using new getOpenSSHKeyFile method
arpoch Aug 4, 2021
3f82b61
Merge branch 'jenkinsci:master' into gitSSHPrivateKey
arpoch Aug 4, 2021
30064ed
Merge remote-tracking branch 'origin/gitSSHPrivateKey' into gitSSHPri…
arpoch Aug 6, 2021
319442e
Removing shell script $@
arpoch Aug 6, 2021
ea86cda
Update example capitalization
MarkEWaite Aug 6, 2021
dcc83b3
Renaming symbol name for ssh-binding inline-help
arpoch Aug 7, 2021
29cdcd7
Merge branch 'jenkinsci:master' into gitSSHPrivateKey
arpoch Aug 11, 2021
e13d669
Depending on sshd plugin v3.1.0
arpoch Aug 11, 2021
dc83e1c
Use 2.289.x bom
MarkEWaite Aug 11, 2021
583ee18
Discarding shaded sshd-core api imports
arpoch Aug 11, 2021
cbf6215
Code formatting
arpoch Aug 11, 2021
74500c1
Api name change for isOpenSSHFormat
arpoch Aug 11, 2021
1ff2fcb
Api name change for getOpenSSHKeyFile
arpoch Aug 11, 2021
fc81ba3
Using `NonNull` annotation to avoid `NullPointException`
arpoch Aug 11, 2021
c6d9b3b
Renaming api's in SSHKeyUtils
arpoch Aug 11, 2021
607a5ab
Renaming api's in GitSSHPrivateKeyBinding
arpoch Aug 11, 2021
1662e96
Adding test resource for ssh binding test cases
arpoch Aug 11, 2021
c0d14aa
Test cases for git ssh binding
arpoch Aug 11, 2021
99535a8
Test cases for `SSHKeyUtils`
arpoch Aug 11, 2021
85dfeed
Create new folder for each test case
arpoch Aug 12, 2021
d7f3b55
Using FilePath to provide remoting support
arpoch Aug 13, 2021
2efc015
Adhering to lowercase starting letter de facto
arpoch Aug 13, 2021
e9d572d
Changing git ssh binding symbol name in GitSSHPrivateKeyBindingTest c…
arpoch Aug 13, 2021
be8b2e9
Changing git ssh binding symbol name in help-credentialsId
arpoch Aug 13, 2021
a98514f
Adding new line
arpoch Aug 13, 2021
649cd85
Handling exceptions for operations on a private key using try/catch
arpoch Aug 14, 2021
1766ec2
Adding RequirePOST annotation
arpoch Aug 14, 2021
74b4f16
Removing default variable bindings for git ssh private key binding
arpoch Aug 15, 2021
f168ecb
Adding git ssh binding images
arpoch Aug 15, 2021
71bd8fb
Merge branch 'master' into gitSSHPrivateKey
arpoch Aug 15, 2021
b6eebfd
Documentation for git ssh private key binding in a pipeline job
arpoch Aug 15, 2021
b6eeac2
Documentation for git ssh private key binding in a freestyle project
arpoch Aug 15, 2021
66009a8
Merge remote-tracking branch 'origin/gitSSHPrivateKey' into gitSSHPri…
arpoch Aug 15, 2021
06c8073
Adhering to code style guideline
arpoch Aug 17, 2021
187b399
Removing redundant environment variables from batch script
arpoch Aug 17, 2021
e12419b
Removing redundant environment variables from shell script
arpoch Aug 17, 2021
828cc94
Adding |(or)
arpoch Aug 17, 2021
dd86551
Using `ssh` as ssh path in windows environment
arpoch Aug 19, 2021
cdefb57
Merge branch 'gitSSHPrivateKey' of https://github.com/arpoch/git-plug…
arpoch Aug 19, 2021
1ce0079
Storing passphrase as a secret
arpoch Aug 20, 2021
0e5b9fb
Adding javadocs for API's in SSHKeyUtils
arpoch Aug 20, 2021
1354b1f
Adding javadocs for public API's in OpenSSHKeyFormatImpl
arpoch Aug 20, 2021
eefb41d
Removing redundant throw statements
arpoch Aug 20, 2021
83171e3
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Aug 21, 2021
3a238c0
Use latest 2.289.x bom
MarkEWaite Aug 21, 2021
c63559c
Merge branch 'jenkinsci:master' into gitSSHPrivateKey
arpoch Aug 22, 2021
4654472
Merge branch 'master' into gitSSHPrivateKey
arpoch Aug 30, 2021
b3b2f69
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Sep 17, 2021
8396478
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Sep 23, 2021
b097fb5
Fix build break from merge accident
MarkEWaite Sep 26, 2021
0a40c33
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Sep 26, 2021
1785381
Merge branch 'master' into gitSSHPrivateKey
arpoch Oct 25, 2021
78651ae
Resolving merge conflicts
arpoch Oct 25, 2021
825eda8
Merge branch 'jenkinsci:master' into gitSSHPrivateKey
arpoch Nov 2, 2021
4a5fbdf
Merge branch 'master' into gitSSHPrivateKey
MarkEWaite Jun 22, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<argLine>-Dfile.encoding=${project.build.sourceEncoding}</argLine>
<jenkins.version>2.263.1</jenkins.version>
<jenkins.version>2.289.1</jenkins.version>
<java.level>8</java.level>
<no-test-jar>false</no-test-jar>
<useBeta>true</useBeta>
Expand Down Expand Up @@ -167,6 +167,17 @@
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials-binding</artifactId>
</dependency>
<!-- BouncyCastle API Plugin -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>bouncycastle-api</artifactId>
</dependency>
<!-- SSHD Plugin -->
<dependency>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>sshd</artifactId>
<version>3.1.0</version>
</dependency>
MarkEWaite marked this conversation as resolved.
Show resolved Hide resolved
<dependency><!-- we contribute AbstractBuildParameters for Git if it's available -->
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>parameterized-trigger</artifactId>
Expand Down Expand Up @@ -267,7 +278,7 @@
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.263.x</artifactId>
<artifactId>bom-2.289.x</artifactId>
<version>923.v08bdc07cd40f</version>
<scope>import</scope>
<type>pom</type>
Expand Down
224 changes: 224 additions & 0 deletions src/main/java/jenkins/plugins/git/GitSSHPrivateKeyBinding.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
package jenkins.plugins.git;

import com.cloudbees.jenkins.plugins.sshcredentials.SSHUserPrivateKey;
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.EnvVars;
import hudson.FilePath;
import hudson.Launcher;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.plugins.git.GitSCM;
import hudson.plugins.git.GitTool;
import hudson.util.ListBoxModel;
import hudson.Extension;
import jenkins.model.Jenkins;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor;
import org.jenkinsci.plugins.credentialsbinding.MultiBinding;
import org.jenkinsci.plugins.credentialsbinding.impl.AbstractOnDiskBinding;
import org.jenkinsci.plugins.credentialsbinding.impl.UnbindableDir;
import org.jenkinsci.plugins.gitclient.CliGitAPIImpl;
import org.jenkinsci.plugins.gitclient.Git;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nullable;
import java.io.IOException;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;


public class GitSSHPrivateKeyBinding extends MultiBinding<SSHUserPrivateKey> implements GitCredentialBindings, SSHKeyUtils {
final static private String PRIVATE_KEY = "PRIVATE_KEY";
final static private String PASSPHRASE = "PASSPHRASE";
final private String gitToolName;
private transient boolean unixNodeType;

@DataBoundConstructor
public GitSSHPrivateKeyBinding(String gitToolName, String credentialsId) {
super(credentialsId);
this.gitToolName = gitToolName;
//Variables could be added if needed
}

public String getGitToolName() {
return this.gitToolName;
}

private void setUnixNodeType(boolean value) {
this.unixNodeType = value;
}

@Override
public MultiEnvironment bind(@NonNull Run<?, ?> run, @Nullable FilePath filePath,
@Nullable Launcher launcher, @NonNull TaskListener taskListener) throws IOException, InterruptedException {
final Map<String, String> secretValues = new LinkedHashMap<>();
final Map<String, String> publicValues = new LinkedHashMap<>();
SSHUserPrivateKey credentials = getCredentials(run);
setCredentialPairBindings(credentials, secretValues, publicValues);
GitTool cliGitTool = getCliGitTool(run, this.gitToolName, taskListener);
if (cliGitTool != null && filePath != null && launcher != null) {
setUnixNodeType(isCurrentNodeOSUnix(launcher));
final UnbindableDir unbindTempDir = UnbindableDir.create(filePath);
final GitClient git = getGitClientInstance(cliGitTool.getGitExe(), unbindTempDir.getDirPath(), new EnvVars(), taskListener);
final String sshExePath = getSSHPath(git);
setGitEnvironmentVariables(git, publicValues);
if (isGitVersionAtLeast(git, 2, 3, 0, 0)) {
secretValues.put("GIT_SSH_COMMAND", getSSHCmd(credentials, unbindTempDir.getDirPath(), sshExePath));
} else {
SSHScriptFile sshScript = new SSHScriptFile(credentials, sshExePath, unixNodeType);
secretValues.put("GIT_SSH", sshScript.write(credentials, unbindTempDir.getDirPath()).getRemote());
}
return new MultiEnvironment(secretValues, publicValues, unbindTempDir.getUnbinder());
} else {
taskListener.getLogger().println("JGit and JGitApache type Git tools are not supported by this binding");
return new MultiEnvironment(secretValues, publicValues);
}
}

@Override
public Set<String> variables(@NonNull Run<?, ?> run) {
Set<String> keys = new LinkedHashSet<>();
keys.add(PRIVATE_KEY);
keys.add(PASSPHRASE);
return keys;
}

@Override
public void setCredentialPairBindings(@NonNull StandardCredentials credentials, Map<String, String> secretValues, Map<String, String> publicValues) {
SSHUserPrivateKey sshUserCredentials = (SSHUserPrivateKey) credentials;
secretValues.put(PRIVATE_KEY, SSHKeyUtils.getSinglePrivateKey(sshUserCredentials));
secretValues.put(PASSPHRASE, SSHKeyUtils.getPassphrase(sshUserCredentials));
}

/*package*/void setGitEnvironmentVariables(@NonNull GitClient git, Map<String, String> publicValues) throws IOException, InterruptedException {
setGitEnvironmentVariables(git, null, publicValues);
}

@Override
public void setGitEnvironmentVariables(@NonNull GitClient git, Map<String, String> secretValues, Map<String, String> publicValues) throws IOException, InterruptedException {
if (unixNodeType && isGitVersionAtLeast(git, 2, 3, 0, 0)) {
publicValues.put("GIT_TERMINAL_PROMPT", "false");
} else {
publicValues.put("GCM_INTERACTIVE", "false");
}
}

@Override
public GitClient getGitClientInstance(String gitToolExe, FilePath repository, EnvVars env, TaskListener listener) throws IOException, InterruptedException {
Git gitInstance = Git.with(listener, env).using(gitToolExe);
return gitInstance.getClient();
}

@Override
protected Class<SSHUserPrivateKey> type() {
return SSHUserPrivateKey.class;
}

private boolean isGitVersionAtLeast(GitClient git, int major, int minor, int rev, int bugfix) {
return ((CliGitAPIImpl) git).isCliGitVerAtLeast(major, minor, rev, bugfix);
}

private String getSSHPath(GitClient git) throws IOException, InterruptedException {
if (unixNodeType) {
return "ssh";
} else {
return getSSHExePathInWin(git);
}
}

private String getSSHCmd(SSHUserPrivateKey credentials, FilePath tempDir, String sshExePath) throws IOException, InterruptedException {
if (unixNodeType) {
return
sshExePath
+ " -i "
+ getPrivateKeyFile(credentials, tempDir).getRemote()
+ " -o StrictHostKeyChecking=no";
} else {
return "\"" + sshExePath + "\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: For better maintainability, personally I would recommend using

Suggested change
return "\"" + sshExePath + "\""
String.format("%1$s -i %2$s -o StrictHostKeyChecking=no", sshExePath, getPrivateKeyFile(credentials, tempDir).getRemote())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice suggestion, will check this out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...-o StrictHostKeyChecking=no"...

This seems to go against the https://github.com/jenkinsci/git-client-plugin#ssh-host-key-verification feature.

@MarkEWaite is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed @sboardwell . This needs to be extended to support the host key checking strategy before it is merged.

+ " -i "
+ "\""
+ getPrivateKeyFile(credentials, tempDir).getRemote()
+ "\""
+ " -o StrictHostKeyChecking=no";
}
}

protected final class SSHScriptFile extends AbstractOnDiskBinding<SSHUserPrivateKey> {

private final String sshExePath;
private final boolean unixNodeType;

protected SSHScriptFile(SSHUserPrivateKey credentials, String sshExePath, boolean unixNodeType) {
super(SSHKeyUtils.getSinglePrivateKey(credentials) + ":" + SSHKeyUtils.getPassphrase(credentials), credentials.getId());
this.sshExePath = sshExePath;
this.unixNodeType = unixNodeType;
}

@Override
protected FilePath write(SSHUserPrivateKey credentials, FilePath workspace) throws IOException, InterruptedException {
FilePath tempFile;
if (unixNodeType) {
tempFile = workspace.createTempFile("gitSSHScript", ".sh");
tempFile.write(
"ssh -i "
+ getPrivateKeyFile(credentials, workspace).getRemote()
+ " -o StrictHostKeyChecking=no $@", null);
tempFile.chmod(0500);
} else {
tempFile = workspace.createTempFile("gitSSHScript", ".bat");
tempFile.write("@echo off\r\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the same reason, I think whenever a string is getting complex, it should be formatted using a String.format().

+ "\""
+ this.sshExePath
+ "\""
+ " -i "
+ "\""
+ getPrivateKeyFile(credentials, workspace).getRemote()
+ "\""
+ " -o StrictHostKeyChecking=no", null);
}
return tempFile;
}

@Override
protected Class<SSHUserPrivateKey> type() {
return SSHUserPrivateKey.class;
}
}

@Symbol("gitSSHPrivateKey")
@Extension
public static final class DescriptorImpl extends BindingDescriptor<SSHUserPrivateKey> {

@NonNull
@Override
public String getDisplayName() {
return Messages.GitSSHPrivateKeyBinding_DisplayName();
}

public ListBoxModel doFillGitToolNameItems() {

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check

Potential missing permission check in DescriptorImpl#doFillGitToolNameItems
ListBoxModel items = new ListBoxModel();
List<GitTool> toolList = Jenkins.get().getDescriptorByType(GitSCM.DescriptorImpl.class).getGitTools();
for (GitTool t : toolList) {
if (t.getClass().equals(GitTool.class)) {
items.add(t.getName());
}
}
return items;
}

@Override
protected Class<SSHUserPrivateKey> type() {
return SSHUserPrivateKey.class;
}

@Override
public boolean requiresWorkspace() {
return true;
}
}
}
106 changes: 106 additions & 0 deletions src/main/java/jenkins/plugins/git/OpenSSHKeyFormatImpl.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package jenkins.plugins.git;

import hudson.FilePath;
import org.apache.sshd.common.config.keys.loader.openssh.OpenSSHKeyPairResourceParser;
import org.apache.sshd.common.NamedResource;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
import org.apache.sshd.common.session.SessionContext;
import org.apache.sshd.common.util.io.SecureByteArrayOutputStream;

import javax.naming.SizeLimitExceededException;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.FileOutputStream;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;

public class OpenSSHKeyFormatImpl {

private final String privateKey;

Check warning

Code scanning / Jenkins Security Scan

Jenkins: Plaintext password storage

Variable should be reviewed whether it stored a password and is serialized to disk: privateKey
private final String passphrase;
private static final String BEGIN_MARKER = OpenSSHKeyPairResourceParser.BEGIN_MARKER;
private static final String END_MARKER = OpenSSHKeyPairResourceParser.END_MARKER;
private static final String DASH_MARKER = "-----";

public OpenSSHKeyFormatImpl(final String privateKey, final String passphrase) {
this.privateKey = privateKey;
this.passphrase = passphrase;
}

private byte[] getEncData(String privateKey){
String data = privateKey
.replace(DASH_MARKER+BEGIN_MARKER+DASH_MARKER,"")
.replace(DASH_MARKER+END_MARKER+DASH_MARKER,"")
.replaceAll("\\s","");
Base64.Decoder decoder = Base64.getDecoder();
return decoder.decode(data);
}

private KeyPair getOpenSSHKeyPair(SessionContext session, NamedResource resourceKey,
String beginMarker, String endMarker,
FilePasswordProvider passwordProvider,
InputStream stream, Map<String, String> headers )
throws IOException, GeneralSecurityException, SizeLimitExceededException {
OpenSSHKeyPairResourceParser openSSHParser = new OpenSSHKeyPairResourceParser();
Collection<KeyPair> keyPairs = openSSHParser.extractKeyPairs(session,resourceKey,beginMarker,
endMarker, passwordProvider,
stream, headers);
if(keyPairs.size() > 1){
throw new SizeLimitExceededException("Expected KeyPair size to be 1");
}else {
return Collections.unmodifiableCollection(keyPairs).iterator().next();
}
}

private File writePrivateKeyOpenSSHFormatted(File tempFile) {
OpenSSHKeyPairResourceWriter privateKeyWriter = new OpenSSHKeyPairResourceWriter();
SecureByteArrayOutputStream privateKeyBuffer = new SecureByteArrayOutputStream();
ByteArrayInputStream stream = new ByteArrayInputStream(getEncData(this.privateKey));
KeyPair sshKeyPair = null;
try {
sshKeyPair = getOpenSSHKeyPair(null,null,"","",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we passing null values? If these arguments are not needed and are optional then why are we asking them here?

new AcquirePassphrase(this.passphrase),
stream,null);
privateKeyWriter.writePrivateKey(sshKeyPair, "", null, privateKeyBuffer);
FileOutputStream privateKeyFileStream = new FileOutputStream(tempFile);
privateKeyBuffer.writeTo(privateKeyFileStream);
privateKeyFileStream.close();
} catch (IOException | SizeLimitExceededException | GeneralSecurityException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caller function of writePrivateKeyOpenSSHFormatted is already throwing these exceptions, what is the reason of catching them here? Also should we not use the logger function to log the reason of error instead of dumping the whole stack trace?

}
return tempFile;
}

public static boolean isOpenSSHFormatted(String privateKey) {
final String HEADER = DASH_MARKER+BEGIN_MARKER+DASH_MARKER;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may use .concat() function instead of the + operator.

Copy link
Member

@nfalco79 nfalco79 Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are all constants that mean compile-time this will be replaced with its calculate valued, with concat it should remain a function called every time. When concat constants I think better + operator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead you could move HEADER definition as constant and use here and in getEncData function

return privateKey.regionMatches(false, 0, HEADER, 0, HEADER.length());
}

public FilePath writeDecryptedOpenSSHKey(FilePath tempKeyFile) throws IOException, InterruptedException, GeneralSecurityException, SizeLimitExceededException {
File tempFile = new File(tempKeyFile.toURI());
writePrivateKeyOpenSSHFormatted(tempFile);
return new FilePath(tempFile);
}

private final static class AcquirePassphrase implements FilePasswordProvider {

String passphrase;

AcquirePassphrase(String passphrase) {
this.passphrase = passphrase;
}

@Override
public String getPassword(SessionContext session, NamedResource resourceKey, int retryIndex) throws IOException {
return this.passphrase;
}
}
}
Loading