Skip to content

Commit

Permalink
Fix CVE-2019-1003061 Secrets stored in plain text (#58)
Browse files Browse the repository at this point in the history 
Convert String type to Secret type to allow Jenkins to encrypt the aws secret key when stored on disk.

Fixes https://www.jenkins.io/security/advisory/2019-04-03/#SECURITY-1042
  • Loading branch information
@bitle
bitle authored Nov 12, 2022
1 parent d7c93e8 commit d492ecc
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 21 deletions.
20 changes: 20 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ creates an instance and outputs its IP address you can potentially pass
the IP address into another stack as a parameter and use it in whatever
way you need.

### Using with Credentials Binding Plugin
It is highly recommended to use this plugin together with [Credentials Binding Plugin](https://plugins.jenkins.io/credentials-binding).
You can save your AWS IAM Credentials in Jenkins Credentials and then access them with Credentials Binding (see screenshot).

![](docs/images/credentials_binding.png)

This makes your AWS credentials available as environmental variables.
Now you can configure the CloudFormation plugin to use these env vars.

![](docs/images/env_vars.png)

## Vulnerabilities
### CVE-2019-1003061
Link: [SECURITY-1042/CVE-2019-1003061](https://www.jenkins.io/security/advisory/2019-04-03/#SECURITY-1042)

Previous versions of this plugin store AWS Secret Key in plain text. This issue is resolved in version 1.4 and later.

You can still use previous version of this plugin and avoid storing your AWS Credentials in plain text by using
Credentials Binding plugin and referencing the AWS Keys with env vars. See Using with Credentials Binding Plugin section above.

## Contributing

To contribute to this project, please read the
Expand Down
Binary file added docs/images/credentials_binding.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/env_vars.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 6 additions & 5 deletions ...main/java/com/syncapse/jenkinsci/plugins/awscloudformationwrapper/PostBuildStackBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.HashMap;
import java.util.Map;

import hudson.util.Secret;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -57,7 +58,7 @@ public class PostBuildStackBean extends AbstractDescribableImpl<PostBuildStackBe
/**
* The secret key to call Amazon's APIs
*/
private String awsSecretKey;
private Secret awsSecretKey;


private long sleep;
Expand All @@ -68,7 +69,7 @@ public class PostBuildStackBean extends AbstractDescribableImpl<PostBuildStackBe
@DataBoundConstructor
public PostBuildStackBean(String stackName, String description,
String cloudFormationRecipe, String parameters, long timeout,
String awsAccessKey, String awsSecretKey, Region awsRegion,long sleep) {
String awsAccessKey, Secret awsSecretKey, Region awsRegion,long sleep) {
super();
this.stackName = stackName;
this.description = description;
Expand Down Expand Up @@ -105,7 +106,7 @@ public String getAwsAccessKey() {
return awsAccessKey;
}

public String getAwsSecretKey() {
public Secret getAwsSecretKey() {
return awsSecretKey;
}
public long getSleep() {
Expand Down Expand Up @@ -147,7 +148,7 @@ public String getParsedAwsAccessKey(EnvVars env) {


public String getParsedAwsSecretKey(EnvVars env) {
return env.expand(getAwsSecretKey());
return env.expand(getAwsSecretKey().getPlainText());
}

@Extension
Expand Down Expand Up @@ -229,4 +230,4 @@ public ListBoxModel doFillAwsRegionItems() {
}


}
}
9 changes: 5 additions & 4 deletions src/main/java/com/syncapse/jenkinsci/plugins/awscloudformationwrapper/SimpleStackBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.io.IOException;

import hudson.util.Secret;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
Expand All @@ -36,7 +37,7 @@ public class SimpleStackBean extends AbstractDescribableImpl<SimpleStackBean> {
/**
* The secret key to call Amazon's APIs
*/
private String awsSecretKey;
private Secret awsSecretKey;

/**
* The AWS Region to work against.
Expand All @@ -49,7 +50,7 @@ public class SimpleStackBean extends AbstractDescribableImpl<SimpleStackBean> {

@DataBoundConstructor
public SimpleStackBean(String stackName, String awsAccessKey,
String awsSecretKey, Region awsRegion,Boolean isPrefixSelected) {
Secret awsSecretKey, Region awsRegion, Boolean isPrefixSelected) {
this.stackName = stackName;
this.awsAccessKey = awsAccessKey;
this.awsSecretKey = awsSecretKey;
Expand All @@ -67,7 +68,7 @@ public String getAwsAccessKey() {
return awsAccessKey;
}

public String getAwsSecretKey() {
public Secret getAwsSecretKey() {
return awsSecretKey;
}
public Boolean getIsPrefixSelected() {
Expand All @@ -79,7 +80,7 @@ public String getParsedAwsAccessKey(EnvVars env) {
}

public String getParsedAwsSecretKey(EnvVars env) {
return env.expand(getAwsSecretKey());
return env.expand(getAwsSecretKey().getPlainText());
}

public Region getAwsRegion() {
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/com/syncapse/jenkinsci/plugins/awscloudformationwrapper/StackBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.HashMap;
import java.util.Map;

import hudson.util.Secret;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -57,7 +58,7 @@ public class StackBean extends AbstractDescribableImpl<StackBean> {
/**
* The secret key to call Amazon's APIs
*/
private String awsSecretKey;
private Secret awsSecretKey;

/**
* Whether or not the stack should be deleted automatically when the job completes
Expand All @@ -69,7 +70,7 @@ public class StackBean extends AbstractDescribableImpl<StackBean> {
@DataBoundConstructor
public StackBean(String stackName, String description,
String cloudFormationRecipe, String parameters, long timeout,
String awsAccessKey, String awsSecretKey, boolean autoDeleteStack, Region awsRegion) {
String awsAccessKey, Secret awsSecretKey, boolean autoDeleteStack, Region awsRegion) {
super();
this.stackName = stackName;
this.description = description;
Expand Down Expand Up @@ -106,7 +107,7 @@ public String getAwsAccessKey() {
return awsAccessKey;
}

public String getAwsSecretKey() {
public Secret getAwsSecretKey() {
return awsSecretKey;
}

Expand Down Expand Up @@ -148,7 +149,7 @@ public String getParsedAwsAccessKey(EnvVars env) {


public String getParsedAwsSecretKey(EnvVars env) {
return env.expand(getAwsSecretKey());
return env.expand(getAwsSecretKey().getPlainText());
}

@Extension
Expand Down
7 changes: 4 additions & 3 deletions ...m/syncapse/jenkinsci/plugins/awscloudformationwrapper/CloudFormationBuildWrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.ArrayList;
import java.util.List;

import hudson.util.Secret;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -77,9 +78,9 @@ private void and_2nd_stack_fails_to_create() throws Exception {
private void when_2_stack_are_entered() throws Exception {
List<StackBean> stackBeans = new ArrayList<StackBean>();
stackBeans.add(new StackBean("stack1", "stack description",
"{resources: }", "", 0, "accessKey", "secretKey", true, null));
"{resources: }", "", 0, "accessKey", Secret.fromString("secretKey"), true, null));
stackBeans.add(new StackBean("stack2", "stack2 description",
"{resources: }", "", 0, "accessKey", "secretKey", true, null));
"{resources: }", "", 0, "accessKey", Secret.fromString("secretKey"), true, null));

wrapper = spy(new CloudFormationBuildWrapper(stackBeans));

Expand Down Expand Up @@ -108,7 +109,7 @@ private void then_1_stack_is_created_and_deleted() throws Exception {
private void when_1_stack_is_entered() throws Exception {
List<StackBean> stackBeans = new ArrayList<StackBean>();
stackBeans.add(new StackBean("stack1", "stack description",
"{resources: }", "", 0, "accessKey", "secretKey", true, null));
"{resources: }", "", 0, "accessKey", Secret.fromString("secretKey"), true, null));

wrapper = spy(new CloudFormationBuildWrapper(stackBeans));

Expand Down
11 changes: 6 additions & 5 deletions src/test/java/com/syncapse/jenkinsci/plugins/awscloudformationwrapper/StackBeanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.Assert.assertTrue;
import hudson.EnvVars;

import hudson.util.Secret;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -20,7 +21,7 @@ public void setup() throws Exception {
public void parameterParsing_1_Param() {

String parameters = "key1=value1";
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", "awsSecretKey", true, null);
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", Secret.fromString("awsSecretKey"), true, null);

assertTrue(stackBean.getParsedParameters(env).get("key1").equals("value1"));
assertTrue(stackBean.getParsedParameters(env).values().size() == 1);
Expand All @@ -31,7 +32,7 @@ public void parameterParsing_1_Param() {
public void parameterParsing_2_Params() {

String parameters = "key1=value1, key2=value2"; // make sure spaces don't mess things up.
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", "awsSecretKey", true, null);
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", Secret.fromString("awsSecretKey"), true, null);

assertTrue(stackBean.getParsedParameters(env).get("key1").equals("value1"));
assertTrue(stackBean.getParsedParameters(env).get("key2").equals("value2"));
Expand All @@ -43,7 +44,7 @@ public void parameterParsing_2_Params() {
public void parameterParsing_Is_Resilient_to_spaces() {

String parameters = "key1 = value1, key2 = value2"; // make sure spaces don't mess things up.
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", "awsSecretKey", true, null);
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", Secret.fromString("awsSecretKey"), true, null);

assertTrue(stackBean.getParsedParameters(env).get("key1").equals("value1"));
assertTrue(stackBean.getParsedParameters(env).get("key2").equals("value2"));
Expand All @@ -56,7 +57,7 @@ public void parsingParameters_Expand_Variables() throws Exception {
env.put("value1", "expandedValue1");
env.put("value2", "expandedValue2");
String parameters = "key1=$value1,key2=${value2}, key3=$value3, key4=${value4}";
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", "awsSecretKey", true, null);
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", Secret.fromString("awsSecretKey"), true, null);

assertTrue(stackBean.getParsedParameters(env).get("key1").equals("expandedValue1"));
assertTrue(stackBean.getParsedParameters(env).get("key2").equals("expandedValue2"));
Expand All @@ -71,7 +72,7 @@ public void parsingParameters_With_Semicolons_Expand_Variables() throws Exceptio
env.put("value1", "expandedValue1");
env.put("value2", "expandedValue2");
String parameters = "key1=$value1;key2=${value2}; key3=v1,v2,v3,v4; key4=${value4}";
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", "awsSecretKey", true, null);
stackBean = new StackBean("name", "description", "aRecipe", parameters, 0, "awsAccessKey", Secret.fromString("awsSecretKey"), true, null);

assertTrue(stackBean.getParsedParameters(env).get("key1").equals("expandedValue1"));
assertTrue(stackBean.getParsedParameters(env).get("key2").equals("expandedValue2"));
Expand Down

0 comments on commit d492ecc

Please sign in to comment.