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

[JENKINS-61808] Always transmit f:password values as Secret #4630

Merged
merged 2 commits into from May 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 52 additions & 6 deletions core/src/main/java/hudson/Functions.java
Expand Up @@ -166,6 +166,7 @@
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import org.apache.commons.io.IOUtils;
Expand Down Expand Up @@ -2013,21 +2014,66 @@ public List<String> getLoggerNames() {
* Used by {@code <f:password/>} so that we send an encrypted value to the client.
*/
public String getPasswordValue(Object o) {
if (o==null) return null;
if (o instanceof Secret) {
StaplerRequest req = Stapler.getCurrentRequest();
if (o == null) {
return null;
}

/*
Return plain value if it's the default value for PasswordParameterDefinition.
This needs to work even when the user doesn't have CONFIGURE permission
*/
if (o.equals(PasswordParameterDefinition.DEFAULT_VALUE)) {
return o.toString();
}

/* Mask from Extended Read */
StaplerRequest req = Stapler.getCurrentRequest();
if (o instanceof Secret || Secret.BLANK_NONSECRET_PASSWORD_FIELDS_WITHOUT_ITEM_CONFIGURE) {
if (req != null) {
Item item = req.findAncestorObject(Item.class);
if (item != null && !item.hasPermission(Item.CONFIGURE)) {
return "********";
}
}
}

/* Return encrypted value if it's a Secret */
if (o instanceof Secret) {
return ((Secret) o).getEncryptedValue();
}
if (getIsUnitTest() && !o.equals(PasswordParameterDefinition.DEFAULT_VALUE)) {
throw new SecurityException("attempted to render plaintext ‘" + o + "’ in password field; use a getter of type Secret instead");

/* Log a warning if we're in development mode (core or plugin): There's an f:password backed by a non-Secret */
if (req != null && (Boolean.getBoolean("hudson.hpi.run") || Boolean.getBoolean("hudson.Main.development"))) {
LOGGER.log(Level.WARNING, () -> "<f:password/> form control in " + getJellyViewsInformationForCurrentRequest() +
" is not backed by hudson.util.Secret. Learn more: https://jenkins.io/redirect/hudson.util.Secret");
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this redirect be pointing here https://www.jenkins.io/doc/developer/security/secrets/ instead of the wiki?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, the redirect target is terrible, needs to be changed. But not worth doing another redirect for it. We probably need a new docs page to talk about this + toString.

}

/* Return plain value if it's not a Secret and the escape hatch is set */
if (!Secret.AUTO_ENCRYPT_PASSWORD_CONTROL) {
return o.toString();
}

/* Make it a Secret and return its encrypted value */
return Secret.fromString(o.toString()).getEncryptedValue();
}

private String getJellyViewsInformationForCurrentRequest() {
final Thread thread = Thread.currentThread();
String threadName = thread.getName();

// try to simplify based on org.kohsuke.stapler.jelly.JellyViewScript
// Views are expected to contain a slash and a period, neither as the first char, and the last slash before the first period: Class/view.jelly
// Nested classes use slashes, so we do not expect period before: Class/Nested/view.jelly
String views = Arrays.stream(threadName.split(" ")).filter(part -> {
int slash = part.lastIndexOf("/");
int firstPeriod = part.indexOf(".");
return slash > 0 && firstPeriod > 0 && slash < firstPeriod;
}).collect(Collectors.joining(" "));
if (StringUtils.isBlank(views)) {
// fallback to full thread name if there are no apparent views
return threadName;
}
return o.toString();
return views;
}

public List filterDescriptors(Object context, Iterable descriptors) {
Expand Down
Expand Up @@ -46,12 +46,18 @@ public class PasswordParameterDefinition extends SimpleParameterDefinition {

private Secret defaultValue;

@DataBoundConstructor
@Deprecated
public PasswordParameterDefinition(String name, String defaultValue, String description) {
super(name, description);
this.defaultValue = Secret.fromString(defaultValue);
}

@DataBoundConstructor
public PasswordParameterDefinition(String name, Secret defaultValueAsSecret, String description) {
super(name, description);
this.defaultValue = defaultValueAsSecret;
}

@Override
public ParameterDefinition copyWithDefaultValue(ParameterValue defaultValue) {
if (defaultValue instanceof PasswordParameterValue) {
Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/hudson/model/PasswordParameterValue.java
Expand Up @@ -44,12 +44,18 @@ public PasswordParameterValue(String name, String value) {
this(name, value, null);
}

@DataBoundConstructor
@Deprecated
public PasswordParameterValue(String name, String value, String description) {
super(name, description);
this.value = Secret.fromString(value);
}

@DataBoundConstructor
public PasswordParameterValue(String name, Secret value, String description) {
super(name, description);
this.value = value;
}

@Override
public void buildEnvironment(Run<?,?> build, EnvVars env) {
String v = Secret.toString(value);
Expand Down
22 changes: 21 additions & 1 deletion core/src/main/java/hudson/util/Secret.java
Expand Up @@ -301,6 +301,12 @@ public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingCont

private static final long serialVersionUID = 1L;

@Restricted(NoExternalUse.class)
public static final boolean AUTO_ENCRYPT_PASSWORD_CONTROL = SystemProperties.getBoolean(Secret.class.getName() + ".AUTO_ENCRYPT_PASSWORD_CONTROL", true);

@Restricted(NoExternalUse.class)
public static /* non-final */ boolean BLANK_NONSECRET_PASSWORD_FIELDS_WITHOUT_ITEM_CONFIGURE = SystemProperties.getBoolean(Secret.class.getName() + ".BLANK_NONSECRET_PASSWORD_FIELDS_WITHOUT_ITEM_CONFIGURE", true);

static {
Stapler.CONVERT_UTILS.register(new org.apache.commons.beanutils.Converter() {
public Secret convert(Class type, Object value) {
Expand All @@ -310,9 +316,23 @@ public Secret convert(Class type, Object value) {
if (value instanceof Secret) {
return (Secret) value;
}

return Secret.fromString(value.toString());
}
}, Secret.class);
if (AUTO_ENCRYPT_PASSWORD_CONTROL) {
Stapler.CONVERT_UTILS.register(new org.apache.commons.beanutils.Converter() {
public String convert(Class type, Object value) {
if (value == null) {
return null;
}
Secret decrypted = Secret.decrypt(value.toString());
if (decrypted == null) {
return value.toString();
} else {
return decrypted.getPlainText();
}
}
}, String.class);
}
}
}
Expand Up @@ -29,8 +29,8 @@ THE SOFTWARE.
<f:entry title="${%Name}" help="/help/parameter/name.html">
<f:textbox name="parameter.name" value="${instance.name}" />
</f:entry>
<f:entry title="${%Default Value}" help="/help/parameter/string-default.html">
<f:password name="parameter.defaultValue" value="${instance.defaultValueAsSecret}" />
<f:entry field="defaultValueAsSecret" title="${%Default Value}" help="/help/parameter/string-default.html">
<f:password />
</f:entry>
<f:entry title="${%Description}" help="/help/parameter/description.html">
<f:textarea name="parameter.description" value="${instance.description}" codemirror-mode="${app.markupFormatter.codeMirrorMode}" codemirror-config="${app.markupFormatter.codeMirrorConfig}" previewEndpoint="/markupFormatter/previewDescription" />
Expand Down
Expand Up @@ -26,8 +26,9 @@ THE SOFTWARE.
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"
xmlns:i="jelly:fmt" xmlns:p="/lib/hudson/project">
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<f:password name="value" value="********" readonly="true" />
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<j:set var="readOnlyMode" value="true"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<em>${%hidden}</em>
</f:entry>
</j:jelly>
@@ -0,0 +1 @@
hidden=(password value not shown)
Copy link
Member

Choose a reason for hiding this comment

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