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

[SECURITY-2774] Fix private key stored in plain text #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 33 additions & 3 deletions src/main/java/org/thoughtslive/jenkins/plugins/jira/Site.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials;
import com.cloudbees.plugins.credentials.domains.DomainRequirement;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import com.thoughtworks.xstream.converters.UnmarshallingContext;

import hudson.Extension;
import hudson.Util;
import hudson.diagnosis.OldDataMonitor;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.model.Item;
Expand All @@ -20,6 +23,8 @@
import hudson.util.FormValidation.Kind;
import hudson.util.ListBoxModel;
import hudson.util.Secret;
import hudson.util.XStream2;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
Expand All @@ -32,6 +37,8 @@
import lombok.extern.java.Log;
import org.acegisecurity.Authentication;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
Expand Down Expand Up @@ -68,8 +75,9 @@ public class Site extends AbstractDescribableImpl<Site> {
@Setter(onMethod = @__({@DataBoundSetter}))
private String consumerKey;
@Getter
@Setter(onMethod = @__({@DataBoundSetter}))
private String privateKey;
private transient String privateKey;
@Getter
private Secret privateKeySecret;
@Getter
private Secret secret;
@Getter
Expand Down Expand Up @@ -122,6 +130,12 @@ public void setToken(final String token) {
this.token = Secret.fromString(Util.fixEmpty(token));
}

@DataBoundSetter
public void setPrivateKeySecret(final String privateKey) {
this.privateKeySecret = Secret.fromString(Util.fixEmpty(privateKey));
}


public JiraService getService() {
if (jiraService == null) {
this.jiraService = new JiraService(this);
Expand All @@ -133,6 +147,22 @@ public enum LoginType {
BASIC, OAUTH, CREDENTIAL
}

@Restricted(DoNotUse.class)
public static class ConverterImpl extends XStream2.PassthruConverter<Site> {
public ConverterImpl(XStream2 xstream) {
super(xstream);
}

@Override
protected void callback(Site site, UnmarshallingContext context) {
String privateKey = site.getPrivateKey();
if (privateKey != null && !privateKey.isEmpty()) {
site.setPrivateKeySecret(privateKey);
OldDataMonitor.report(context, "2.361.1");
}
}
}

@Extension
public static class DescriptorImpl extends Descriptor<Site> {

Expand Down Expand Up @@ -414,7 +444,7 @@ public FormValidation doValidateOAuth(@QueryParameter String name, @QueryParamet
return FormValidation.error("Token is empty or null.");
}
site.setConsumerKey(consumerKey);
site.setPrivateKey(privateKey);
site.setPrivateKeySecret(privateKey);
site.setSecret(secret);
site.setToken(token);
site.setReadTimeout(rt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public Response intercept(Interceptor.Chain chain) throws IOException {
} else if (Site.LoginType.OAUTH.name().equalsIgnoreCase(jiraSite.getLoginType())) {
Request request = chain.request();
OAuthConsumer consumer =
new OAuthConsumer(jiraSite.getConsumerKey(), jiraSite.getPrivateKey());
new OAuthConsumer(jiraSite.getConsumerKey(), jiraSite.getPrivateKeySecret().getPlainText());
consumer.setTokenWithSecret(jiraSite.getToken().getPlainText(),
jiraSite.getSecret().getPlainText());
consumer.setMessageSigner(new RsaSha1MessageSigner());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
<f:entry field="consumerKey" title="Consumer Key">
<f:textbox/>
</f:entry>
<f:entry field="privateKey" title="Private Key">
<f:expandableTextbox/>
<f:entry field="privateKeySecret" title="Private Key">
<f:secretTextarea/>
</f:entry>
<f:entry field="secret" title="Secret">
<f:password/>
Expand Down