From 22ec4ac8cee91149b662d4892cf29aff00be8a34 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Wed, 18 Sep 2024 08:01:07 +0000 Subject: [PATCH] [SECURITY-3373] --- .../main/java/hudson/model/AbstractItem.java | 21 +++++------ .../java/hudson/model/ItemGroupMixIn.java | 26 +++++++------- .../security/ExtendedReadRedaction.java | 36 +++++++++++++++++++ .../security/ExtendedReadSecretRedaction.java | 28 +++++++++++++++ test/src/test/java/lib/form/PasswordTest.java | 3 ++ 5 files changed, 88 insertions(+), 26 deletions(-) create mode 100644 core/src/main/java/jenkins/security/ExtendedReadRedaction.java create mode 100644 core/src/main/java/jenkins/security/ExtendedReadSecretRedaction.java diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java index 5bd1d6a02d79..822ab6b80058 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -65,8 +65,6 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.servlet.ServletException; import javax.xml.transform.Source; import javax.xml.transform.TransformerException; @@ -77,6 +75,7 @@ import jenkins.model.Jenkins; import jenkins.model.Loadable; import jenkins.model.queue.ItemDeletion; +import jenkins.security.ExtendedReadRedaction; import jenkins.security.NotReallyRoleSensitiveCallable; import jenkins.util.SystemProperties; import jenkins.util.xml.XMLUtils; @@ -862,11 +861,11 @@ public void doConfigDotXml(StaplerRequest req, StaplerResponse rsp) rsp.sendError(SC_BAD_REQUEST); } - static final Pattern SECRET_PATTERN = Pattern.compile(">(" + Secret.ENCRYPTED_VALUE_PATTERN + ")<"); /** * Writes {@code config.xml} to the specified output stream. * The user must have at least {@link #EXTENDED_READ}. - * If he lacks {@link #CONFIGURE}, then any {@link Secret}s detected will be masked out. + * If he lacks {@link #CONFIGURE}, then any {@link Secret}s or other sensitive information detected will be masked out. + * @see jenkins.security.ExtendedReadRedaction */ @Restricted(NoExternalUse.class) @@ -878,15 +877,13 @@ public void writeConfigDotXml(OutputStream os) throws IOException { } else { String encoding = configFile.sniffEncoding(); String xml = Files.readString(Util.fileToPath(configFile.getFile()), Charset.forName(encoding)); - Matcher matcher = SECRET_PATTERN.matcher(xml); - StringBuilder cleanXml = new StringBuilder(); - while (matcher.find()) { - if (Secret.decrypt(matcher.group(1)) != null) { - matcher.appendReplacement(cleanXml, ">********<"); - } + + for (ExtendedReadRedaction redaction : ExtendedReadRedaction.all()) { + LOGGER.log(Level.FINE, () -> "Applying redaction " + redaction.getClass().getName()); + xml = redaction.apply(xml); } - matcher.appendTail(cleanXml); - org.apache.commons.io.IOUtils.write(cleanXml.toString(), os, encoding); + + org.apache.commons.io.IOUtils.write(xml, os, encoding); } } diff --git a/core/src/main/java/hudson/model/ItemGroupMixIn.java b/core/src/main/java/hudson/model/ItemGroupMixIn.java index 666b6cee84c8..91e3c1d66e78 100644 --- a/core/src/main/java/hudson/model/ItemGroupMixIn.java +++ b/core/src/main/java/hudson/model/ItemGroupMixIn.java @@ -31,7 +31,6 @@ import hudson.security.AccessControlled; import hudson.util.CopyOnWriteMap; import hudson.util.Function1; -import hudson.util.Secret; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -41,13 +40,13 @@ import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Matcher; import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; import javax.xml.transform.TransformerException; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; import jenkins.model.Jenkins; +import jenkins.security.ExtendedReadRedaction; import jenkins.security.NotReallyRoleSensitiveCallable; import jenkins.util.xml.XMLUtils; import org.kohsuke.stapler.StaplerRequest; @@ -222,18 +221,17 @@ public synchronized T copy(T src, String name) throws I src.checkPermission(Item.EXTENDED_READ); XmlFile srcConfigFile = Items.getConfigFile(src); if (!src.hasPermission(Item.CONFIGURE)) { - Matcher matcher = AbstractItem.SECRET_PATTERN.matcher(srcConfigFile.asString()); - while (matcher.find()) { - if (Secret.decrypt(matcher.group(1)) != null) { - // AccessDeniedException2 does not permit a custom message, and anyway redirecting the user to the login screen is obviously pointless. - throw new AccessDeniedException( - Messages.ItemGroupMixIn_may_not_copy_as_it_contains_secrets_and_( - src.getFullName(), - Jenkins.getAuthentication2().getName(), - Item.PERMISSIONS.title, - Item.EXTENDED_READ.name, - Item.CONFIGURE.name)); - } + final String originalConfigDotXml = srcConfigFile.asString(); + final String redactedConfigDotXml = ExtendedReadRedaction.applyAll(originalConfigDotXml); + if (!originalConfigDotXml.equals(redactedConfigDotXml)) { + // AccessDeniedException2 does not permit a custom message, and anyway redirecting the user to the login screen is obviously pointless. + throw new AccessDeniedException( + Messages.ItemGroupMixIn_may_not_copy_as_it_contains_secrets_and_( + src.getFullName(), + Jenkins.getAuthentication2().getName(), + Item.PERMISSIONS.title, + Item.EXTENDED_READ.name, + Item.CONFIGURE.name)); } } src.getDescriptor().checkApplicableIn(parent); diff --git a/core/src/main/java/jenkins/security/ExtendedReadRedaction.java b/core/src/main/java/jenkins/security/ExtendedReadRedaction.java new file mode 100644 index 000000000000..31436c5622f2 --- /dev/null +++ b/core/src/main/java/jenkins/security/ExtendedReadRedaction.java @@ -0,0 +1,36 @@ +package jenkins.security; + +import hudson.ExtensionList; +import hudson.ExtensionPoint; + +/** + * Redact {@code config.xml} contents for users with ExtendedRead permission + * while lacking the required Configure permission to see the full unredacted + * configuration. + * + * @see SECURITY-266 + * @see Jenkins Security Advisory 2016-05-11 + * @since TODO + */ +public interface ExtendedReadRedaction extends ExtensionPoint { + /** + * Redacts sensitive information from the provided {@code config.xml} file content. + * Input may already have redactions applied; output may be passed through further redactions. + * These methods are expected to retain the basic structure of the XML document contained in input/output strings. + * + * @param configDotXml String representation of (potentially already redacted) config.xml file + * @return Redacted config.xml file content + */ + String apply(String configDotXml); + + static ExtensionList all() { + return ExtensionList.lookup(ExtendedReadRedaction.class); + } + + static String applyAll(String configDotXml) { + for (ExtendedReadRedaction redaction : all()) { + configDotXml = redaction.apply(configDotXml); + } + return configDotXml; + } +} diff --git a/core/src/main/java/jenkins/security/ExtendedReadSecretRedaction.java b/core/src/main/java/jenkins/security/ExtendedReadSecretRedaction.java new file mode 100644 index 000000000000..91a79f354d71 --- /dev/null +++ b/core/src/main/java/jenkins/security/ExtendedReadSecretRedaction.java @@ -0,0 +1,28 @@ +package jenkins.security; + +import hudson.Extension; +import hudson.util.Secret; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +@Restricted(NoExternalUse.class) +@Extension +public class ExtendedReadSecretRedaction implements ExtendedReadRedaction { + + private static final Pattern SECRET_PATTERN = Pattern.compile(">(" + Secret.ENCRYPTED_VALUE_PATTERN + ")<"); + + @Override + public String apply(String configDotXml) { + Matcher matcher = SECRET_PATTERN.matcher(configDotXml); + StringBuilder cleanXml = new StringBuilder(); + while (matcher.find()) { + if (Secret.decrypt(matcher.group(1)) != null) { + matcher.appendReplacement(cleanXml, ">********<"); + } + } + matcher.appendTail(cleanXml); + return cleanXml.toString(); + } +} diff --git a/test/src/test/java/lib/form/PasswordTest.java b/test/src/test/java/lib/form/PasswordTest.java index e71fb93f8e8b..48e9381b02af 100644 --- a/test/src/test/java/lib/form/PasswordTest.java +++ b/test/src/test/java/lib/form/PasswordTest.java @@ -67,6 +67,7 @@ import jenkins.model.GlobalConfiguration; import jenkins.model.Jenkins; import jenkins.model.TransientActionFactory; +import jenkins.security.ExtendedReadSecretRedaction; import jenkins.tasks.SimpleBuildStep; import org.htmlunit.Page; import org.htmlunit.html.DomElement; @@ -76,6 +77,7 @@ import org.htmlunit.html.HtmlTextInput; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.For; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; @@ -124,6 +126,7 @@ public String getUrlName() { @Issue({"SECURITY-266", "SECURITY-304"}) @Test + @For(ExtendedReadSecretRedaction.class) public void testExposedCiphertext() throws Exception { boolean saveEnabled = Item.EXTENDED_READ.getEnabled(); Item.EXTENDED_READ.setEnabled(true);