Skip to content

J
jenkins

xxadev / jenkins
与 Fork 源项目一致

从无法访问的项目Fork

通知 3
Star 0
Fork 0
J
jenkins

体验新版 GitCode，发现更多精彩内容 >>

提交 92556fd1 编写于 作者: A Antonio Muñiz
浏览文件
操作

[SECURITY-170] Whitelisted parameters system property added (as suggested by @jglick)

上级 84174a92
隐藏空白更改
内联 并排
Showing with 98 addition and 70 deletion
core/src/main/java/hudson/model/ParametersAction.java
浏览文件 @ 92556fd1
......@@ -68,6 +68,13 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
@Restricted(NoExternalUse.class)
public static final String KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME = ParametersAction.class.getName() +
".keepUndefinedParameters";
@Restricted(NoExternalUse.class)
public static final String SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME = ParametersAction.class.getName() +
".safeParameters";
private transient List<String> safeParameters;
private final List<ParameterValue> parameters;
private List<String> parameterDefinitionNames;
......@@ -235,12 +242,11 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
@Override
public void onAttached(Run<?, ?> r) {
Job<?, ?> job = r.getParent();
if (r.getParent() != null) {
ParametersDefinitionProperty p = job.getProperty(ParametersDefinitionProperty.class);
if (p != null) {
this.parameterDefinitionNames = p.getParameterDefinitionNames();
}
ParametersDefinitionProperty p = r.getParent().getProperty(ParametersDefinitionProperty.class);
if (p != null) {
this.parameterDefinitionNames = p.getParameterDefinitionNames();
} else {
this.parameterDefinitionNames = Collections.emptyList();
}
this.run = r;
}
......@@ -255,11 +261,6 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
return parameters;
}
Job<?, ?> parent = run.getParent();
if (parent == null) {
return parameters;
}
if (this.parameterDefinitionNames == null) {
return parameters;
}
......@@ -271,13 +272,12 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
List<ParameterValue> filteredParameters = new ArrayList<ParameterValue>();
for (ParameterValue v : this.parameters) {
if (this.parameterDefinitionNames.contains(v.getName())) {
if (this.parameterDefinitionNames.contains(v.getName()) || isSafeParameter(v.getName())) {
filteredParameters.add(v);
} else {
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE, "Skipped parameter '" + v.getName() + "' as it is undefined on '" +
run.getParent().getFullName() + "'");
}
LOGGER.log(Level.WARNING, "Skipped parameter `{0}` as it is undefined on `{1}`. Set `-D{2}`=true to allow "
+ "undefined parameters to be injected as environment variables, even though it represents a security breach",
new Object [] { v.getName(), run.getParent().getFullName(), KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME });
}
}
......@@ -285,7 +285,11 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
}
/**
* Returns all parameters. Be careful in how you process them.
* Returns all parameters.
*
* Be careful in how you process them. It will return parameters even not being defined as
* {@link ParametersDefinitionProperty} in the job, so any external
* caller could inject any parameter (using any key) here. <strong>Treat it as untrusted data</strong>.
*
* @return all parameters defined here.
* @since TODO
......@@ -294,6 +298,18 @@ public class ParametersAction implements RunAction2, Iterable<ParameterValue>, Q
return Collections.unmodifiableList(parameters);
}
private boolean isSafeParameter(String name) {
if (safeParameters == null) {
String paramNames = System.getProperty(SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
if (paramNames != null) {
safeParameters = Arrays.asList(paramNames.split(","));
} else {
safeParameters = Collections.emptyList();
}
}
return safeParameters.contains(name);
}
private static final Logger LOGGER = Logger.getLogger(ParametersAction.class.getName());
}
test/src/test/java/hudson/model/ParametersActionTest2.java
浏览文件 @ 92556fd1
......@@ -35,9 +35,6 @@ public class ParametersActionTest2 {
new StringParameterValue("foo", "baz"),
new StringParameterValue("undef", "undef")
)));
if (b.t != null) {
throw b.t;
}
}
@Test
......@@ -58,9 +55,6 @@ public class ParametersActionTest2 {
new StringParameterValue("foo", "baz"),
new StringParameterValue("undef", "undef")
)));
if (b.t != null) {
throw b.t;
}
} finally {
System.clearProperty(ParametersAction.KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME);
}
......@@ -75,10 +69,10 @@ public class ParametersActionTest2 {
FreeStyleProject p = j.jenkins.getItemByFullName("parameterized", FreeStyleProject.class);
FreeStyleBuild b1 = p.getBuildByNumber(1);
ParametersAction pa = b1.getAction(ParametersAction.class);
ParametersCheckBuilder.hasParameterWithName(pa, "FOO");
ParametersCheckBuilder.hasParameterWithName(pa, "BAR");
hasParameterWithName(pa, "FOO");
hasParameterWithName(pa, "BAR");
// legacy behaviour expected (UNDEF returned by getParameters())
ParametersCheckBuilder.hasParameterWithName(pa, "UNDEF");
hasParameterWithName(pa, "UNDEF");
// A new build should work as expected (undef is not published to env)
ParametersCheckBuilder b = new ParametersCheckBuilder(false);
......@@ -89,19 +83,15 @@ public class ParametersActionTest2 {
new StringParameterValue("foo", "baz"),
new StringParameterValue("undef", "undef")
)));
if (b.t != null) {
throw b.t;
}
}
@Test
@Issue("SECURITY-170")
public void parametersDefinitionChange() throws Exception {
FreeStyleProject p = j.createFreeStyleProject();
p.addProperty(new ParametersDefinitionProperty(Arrays.asList(new ParameterDefinition[] {
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"),
new StringParameterDefinition("bar", "bar")
})));
new StringParameterDefinition("bar", "bar"))));
FreeStyleBuild build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(
new StringParameterValue("foo", "baz"),
......@@ -110,28 +100,64 @@ public class ParametersActionTest2 {
)));
assertTrue("undef parameter is not listed in getParameters",
!ParametersCheckBuilder.hasParameterWithName(build.getAction(ParametersAction.class), "undef"));
!hasParameterWithName(build.getAction(ParametersAction.class), "undef"));
p.removeProperty(ParametersDefinitionProperty.class);
p.addProperty(new ParametersDefinitionProperty(Arrays.asList(new ParameterDefinition[] {
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"),
new StringParameterDefinition("bar", "bar"),
new StringParameterDefinition("undef", "undef")
})));
new StringParameterDefinition("undef", "undef"))));
// undef is still not listed even after being added to the job parameters definition
assertTrue("undef parameter is not listed in getParameters",
!ParametersCheckBuilder.hasParameterWithName(build.getAction(ParametersAction.class), "undef"));
!hasParameterWithName(build.getAction(ParametersAction.class), "undef"));
// remove bar and undef from parameters definition
p.removeProperty(ParametersDefinitionProperty.class);
p.addProperty(new ParametersDefinitionProperty(Arrays.asList(new ParameterDefinition[] {
new StringParameterDefinition("foo", "foo")
})));
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"))));
assertTrue("the build still have 2 parameters", build.getAction(ParametersAction.class).getParameters().size() == 2);
p.removeProperty(ParametersDefinitionProperty.class);
assertTrue("the build still have 2 parameters", build.getAction(ParametersAction.class).getParameters().size() == 2);
}
@Test
@Issue("SECURITY-170")
public void whitelistedParameter() throws Exception {
FreeStyleProject p = j.createFreeStyleProject();
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"),
new StringParameterDefinition("bar", "bar"))));
try {
System.setProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME, "whitelisted1,whitelisted2");
FreeStyleBuild build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(
new StringParameterValue("foo", "baz"),
new StringParameterValue("bar", "bar"),
new StringParameterValue("whitelisted1", "x"),
new StringParameterValue("whitelisted2", "y")
)));
assertTrue("whitelisted1 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted1"));
assertTrue("whitelisted2 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted2"));
} finally {
System.clearProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
}
}
public static boolean hasParameterWithName(Iterable<ParameterValue> values, String name) {
for (ParameterValue v : values) {
if (v.getName().equals(name)) {
return true;
}
}
return false;
}
public static class ParametersCheckBuilder extends Builder {
private final boolean expectLegacyBehavior;
......@@ -140,42 +166,28 @@ public class ParametersActionTest2 {
this.expectLegacyBehavior = expectLegacyBehavior;
}
private Exception t;
@Override
public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener)
throws InterruptedException, IOException {
try {
ParametersAction pa = build.getAction(ParametersAction.class);
assertEquals("foo value expected changed", "baz", pa.getParameter("foo").getValue());
if (expectLegacyBehavior) {
assertTrue("undef parameter is listed in getParameters", hasParameterWithName(pa.getParameters(), "undef"));
assertTrue("undef parameter is listed in iterator", hasParameterWithName(pa, "undef"));
assertTrue("undef in environment", build.getEnvironment(listener).keySet().contains("undef"));
assertTrue("UNDEF in environment", build.getEnvironment(listener).keySet().contains("UNDEF"));
} else {
assertFalse("undef parameter is not listed in getParameters", hasParameterWithName(pa.getParameters(), "undef"));
assertFalse("undef parameter is not listed in iterator", hasParameterWithName(pa, "undef"));
assertFalse("undef not in environment", build.getEnvironment(listener).keySet().contains("undef"));
assertFalse("UNDEF not in environment", build.getEnvironment(listener).keySet().contains("UNDEF"));
}
assertTrue("undef parameter is listed in getAllParameters", hasParameterWithName(pa.getAllParameters(), "undef"));
assertEquals("undef parameter direct access expected to work", "undef", pa.getParameter("undef").getValue());
} catch (Exception e) {
t = e;
ParametersAction pa = build.getAction(ParametersAction.class);
assertEquals("foo value expected changed", "baz", pa.getParameter("foo").getValue());
if (expectLegacyBehavior) {
assertTrue("undef parameter is listed in getParameters", hasParameterWithName(pa.getParameters(), "undef"));
assertTrue("undef parameter is listed in iterator", hasParameterWithName(pa, "undef"));
assertTrue("undef in environment", build.getEnvironment(listener).keySet().contains("undef"));
assertTrue("UNDEF in environment", build.getEnvironment(listener).keySet().contains("UNDEF"));
} else {
assertFalse("undef parameter is not listed in getParameters", hasParameterWithName(pa.getParameters(), "undef"));
assertFalse("undef parameter is not listed in iterator", hasParameterWithName(pa, "undef"));
assertFalse("undef not in environment", build.getEnvironment(listener).keySet().contains("undef"));
assertFalse("UNDEF not in environment", build.getEnvironment(listener).keySet().contains("UNDEF"));
}
return true;
}
assertTrue("undef parameter is listed in getAllParameters", hasParameterWithName(pa.getAllParameters(), "undef"));
assertEquals("undef parameter direct access expected to work", "undef", pa.getParameter("undef").getValue());
public static boolean hasParameterWithName(Iterable<ParameterValue> values, String name) {
for (ParameterValue v : values) {
if (v.getName().equals(name)) {
return true;
}
}
return false;
return true;
}
}
}
Markdown is supported
0% 或 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑！
想要评论请 注册 或