Merge branch 'security' into security-rc

Conflicts:
	core/src/main/java/hudson/Functions.java
	core/src/main/java/hudson/markup/MyspacePolicy.java
	core/src/main/java/hudson/util/RobustReflectionConverter.java
	core/src/main/resources/lib/layout/layout.jelly
	war/pom.xml

core/src/main/java/hudson/Functions.java

@@ -510,6 +510,15 @@ public class Functions {
         return c.getValue();
     }
 
+    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");
+    @Restricted(NoExternalUse.class)
+    public static String validateIconSize(String iconSize) throws SecurityException {
+        if (!ICON_SIZE.matcher(iconSize).matches()) {
+            throw new SecurityException("invalid iconSize");
+        }
+        return iconSize;
+    }
+
     /**
      * Gets the suffix to use for YUI JavaScript.
      */
@@ -1678,6 +1687,9 @@ public class Functions {
     public String getPasswordValue(Object o) {
         if (o==null)    return null;
         if (o instanceof Secret)    return ((Secret)o).getEncryptedValue();
+        if (getIsUnitTest()) {
+            throw new SecurityException("attempted to render plaintext ‘" + o + "’ in password field; use a getter of type Secret instead");
+        }
         return o.toString();
     }
 

core/src/main/java/hudson/cli/CreateJobCommand.java

@@ -72,6 +72,7 @@ public class CreateJobCommand extends CLICommand {
             name = name.substring(i + 1);
         }
 
+        Jenkins.checkGoodName(name);
         ig.createProjectFromXML(name, stdin);
         return 0;
     }

core/src/main/java/hudson/markup/MyspacePolicy.java

@@ -48,7 +48,7 @@ public class MyspacePolicy {
                     "script","noscript",/*"iframe",*/"frameset","frame");
 
             tag("label",    "for");
-            tag("form",     "action",ONSITE_OR_OFFSITE_URL,
+            tag("form",     "action",ONSITE_URL,
                             "method");
             tag("button",   "value", "type");
             tag("input",    "maxlength","checked",
@@ -66,7 +66,6 @@ public class MyspacePolicy {
             tag("span,div");
             tag("img",      "src",ONSITE_OR_OFFSITE_URL,
                             "hspace","vspace");
-            tag("iframe",   "src");
             tag("ul,ol,li,dd,dl,dt,thead,tbody,tfoot");
             tag("table",    "noresize");
             tag("td,th,tr");

core/src/main/java/hudson/model/AbstractProject.java

@@ -2023,7 +2023,14 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
 
         for (Publisher _t : Descriptor.newInstancesFromHeteroList(req, json, "publisher", Jenkins.getInstance().getExtensionList(BuildTrigger.DescriptorImpl.class))) {
             BuildTrigger t = (BuildTrigger) _t;
-            for (AbstractProject downstream : t.getChildProjects(this)) {
+            List<AbstractProject> childProjects;
+            SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
+            try {
+                childProjects = t.getChildProjects(this);
+            } finally {
+                SecurityContextHolder.setContext(orig);
+            }
+            for (AbstractProject downstream : childProjects) {
                 downstream.checkPermission(BUILD);
             }
         }

core/src/main/java/hudson/model/Cause.java

@@ -34,6 +34,7 @@ import jenkins.model.Jenkins;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
 import javax.annotation.CheckForNull;
@@ -412,10 +413,13 @@ public abstract class Cause {
         @Override
         public String getShortDescription() {
             if(note != null) {
-                return Messages.Cause_RemoteCause_ShortDescriptionWithNote(addr, note);
-            } else {
-                return Messages.Cause_RemoteCause_ShortDescription(addr);
+                try {
+                    return Messages.Cause_RemoteCause_ShortDescriptionWithNote(addr, Jenkins.getInstance().getMarkupFormatter().translate(note));
+                } catch (IOException x) {
+                    // ignore
+                }
             }
+            return Messages.Cause_RemoteCause_ShortDescription(addr);
         }
 
         @Override

core/src/main/java/hudson/model/PasswordParameterDefinition.java

@@ -28,6 +28,8 @@ import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.DataBoundConstructor;
 import hudson.Extension;
 import hudson.util.Secret;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
 
 /**
  * Parameter whose value is a {@link Secret} and is hidden from the UI.
@@ -76,6 +78,11 @@ public class PasswordParameterDefinition extends SimpleParameterDefinition {
         return Secret.toString(defaultValue);
     }
 
+    @Restricted(DoNotUse.class) // used from Jelly
+    public Secret getDefaultValueAsSecret() {
+        return defaultValue;
+    }
+
     // kept for backward compatibility
     public void setDefaultValue(String defaultValue) {
         this.defaultValue = Secret.fromString(defaultValue);

core/src/main/java/hudson/model/View.java

@@ -783,7 +783,7 @@ public abstract class View extends AbstractModelObject implements AccessControll
 
         {
             StaplerRequest req = Stapler.getCurrentRequest();
-            iconSize = req != null ? Functions.getCookie(req, "iconSize", "32x32") : "32x32";
+            iconSize = req != null ? Functions.validateIconSize(Functions.getCookie(req, "iconSize", "32x32")) : "32x32";
         }
 
         @Override protected void compute() throws Exception {

core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java

@@ -86,6 +86,7 @@ public class AuthenticationProcessingFilter2 extends AuthenticationProcessingFil
         // HttpSessionContextIntegrationFilter stores the updated SecurityContext object into this session later
         // (either when a redirect is issued, via its HttpResponseWrapper, or when the execution returns to its
         // doFilter method.
+        request.getSession().invalidate();
         request.getSession();
         SecurityListener.fireLoggedIn(authResult.getName());
     }

core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

@@ -76,6 +76,8 @@ import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.MissingResourceException;
+import java.util.ResourceBundle;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -171,8 +173,15 @@ public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRea
     @Override
     protected Details authenticate(String username, String password) throws AuthenticationException {
         Details u = loadUserByUsername(username);
-        if (!u.isPasswordCorrect(password))
-            throw new BadCredentialsException("Failed to login as "+username);
+        if (!u.isPasswordCorrect(password)) {
+            String message;
+            try {
+                message = ResourceBundle.getBundle("org.acegisecurity.messages").getString("AbstractUserDetailsAuthenticationProvider.badCredentials");
+            } catch (MissingResourceException x) {
+                message = "Bad credentials";
+            }
+            throw new BadCredentialsException(message);
+        }
         return u;
     }
 

core/src/main/java/hudson/util/RemotingDiagnostics.java

@@ -191,7 +191,7 @@ public final class RemotingDiagnostics {
 
         @WebMethod(name="heapdump.hprof")
         public void doHeapDump(StaplerRequest req, StaplerResponse rsp) throws IOException, InterruptedException {
-            owner.checkPermission(Jenkins.ADMINISTER);
+            owner.checkPermission(Jenkins.RUN_SCRIPTS);
             rsp.setContentType("application/octet-stream");
 
             FilePath dump = obtain();

core/src/main/java/hudson/util/RobustReflectionConverter.java

@@ -44,6 +44,7 @@ import hudson.model.Saveable;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -72,6 +73,8 @@ public class RobustReflectionConverter implements Converter {
     protected transient SerializationMethodInvoker serializationMethodInvoker;
     private transient ReflectionProvider pureJavaReflectionProvider;
     private final @Nonnull XStream2.ClassOwnership classOwnership;
+    /** {@code pkg.Clazz#fieldName} */
+    private final Set<String> criticalFields = Collections.synchronizedSet(new HashSet<String>());
 
     public RobustReflectionConverter(Mapper mapper, ReflectionProvider reflectionProvider) {
         this(mapper, reflectionProvider, new XStream2().new PluginClassOwnership());
@@ -84,6 +87,10 @@ public class RobustReflectionConverter implements Converter {
         serializationMethodInvoker = new SerializationMethodInvoker();
     }
 
+    void addCriticalField(Class<?> clazz, String field) {
+        criticalFields.add(clazz.getName() + '#' + field);
+    }
+
     public boolean canConvert(Class type) {
         return true;
     }
@@ -260,8 +267,16 @@ public class RobustReflectionConverter implements Converter {
         while (reader.hasMoreChildren()) {
             reader.moveDown();
 
+            boolean critical = false;
             try {
                 String fieldName = mapper.realMember(result.getClass(), reader.getNodeName());
+                for (Class<?> concrete = result.getClass(); concrete != null; concrete = concrete.getSuperclass()) {
+                    // Not quite right since a subclass could shadow a field, but probably suffices:
+                    if (criticalFields.contains(concrete.getName() + '#' + fieldName)) {
+                        critical = true;
+                        break;
+                    }
+                }
                 boolean implicitCollectionHasSameName = mapper.getImplicitCollectionDefForFieldName(result.getClass(), reader.getNodeName()) != null;
 
                 Class classDefiningField = determineWhichClassDefinesField(reader);
@@ -293,8 +308,14 @@ public class RobustReflectionConverter implements Converter {
                     }
                 }
             } catch (XStreamException e) {
+                if (critical) {
+                    throw e;
+                }
                 addErrorInContext(context, e);
             } catch (LinkageError e) {
+                if (critical) {
+                    throw e;
+                }
                 addErrorInContext(context, e);
             }
 

core/src/main/java/hudson/util/XStream2.java

@@ -65,6 +65,8 @@ import java.nio.charset.Charset;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.CheckForNull;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * {@link XStream} enhanced for additional Java5 support and improved robustness.
@@ -121,6 +123,16 @@ public class XStream2 extends XStream {
         return reflectionConverter;
     }
 
+    /**
+     * Specifies that a given field of a given class should not be treated with laxity by {@link RobustCollectionConverter}.
+     * @param clazz a class which we expect to hold a non-{@code transient} field
+     * @param field a field name in that class
+     */
+    @Restricted(NoExternalUse.class) // TODO could be opened up later
+    public void addCriticalField(Class<?> clazz, String field) {
+        reflectionConverter.addCriticalField(clazz, field);
+    }
+
     static String trimVersion(String version) {
         // TODO seems like there should be some trick with VersionNumber to do this
         return version.replaceFirst(" .+$", "");

core/src/main/java/jenkins/model/Jenkins.java

@@ -294,7 +294,6 @@ import java.util.logging.Level;
 import static java.util.logging.Level.SEVERE;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
-import java.util.regex.Pattern;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -3473,9 +3472,9 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      */
     public void doIconSize( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
         String qs = req.getQueryString();
-        if(qs==null || !ICON_SIZE.matcher(qs).matches())
+        if(qs==null)
             throw new ServletException();
-        Cookie cookie = new Cookie("iconSize", qs);
+        Cookie cookie = new Cookie("iconSize", Functions.validateIconSize(qs));
         cookie.setMaxAge(/* ~4 mo. */9999999); // #762
         rsp.addCookie(cookie);
         String ref = req.getHeader("Referer");
@@ -4077,8 +4076,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
 
     private static final Logger LOGGER = Logger.getLogger(Jenkins.class.getName());
 
-    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");
-
     public static final PermissionGroup PERMISSIONS = Permission.HUDSON_PERMISSIONS;
     public static final Permission ADMINISTER = Permission.HUDSON_ADMINISTER;
     public static final Permission READ = new Permission(PERMISSIONS,"Read",Messages._Hudson_ReadPermission_Description(),Permission.READ,PermissionScope.JENKINS);
@@ -4101,6 +4098,8 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
         // for backward compatibility with <1.75, recognize the tag name "view" as well.
         XSTREAM.alias("view", ListView.class);
         XSTREAM.alias("listView", ListView.class);
+        XSTREAM2.addCriticalField(Jenkins.class, "securityRealm");
+        XSTREAM2.addCriticalField(Jenkins.class, "authorizationStrategy");
         // this seems to be necessary to force registration of converter early enough
         Mode.class.getEnumConstants();
 

core/src/main/java/jenkins/security/ApiTokenFilter.java

@@ -2,9 +2,13 @@ package jenkins.security;
 
 import hudson.model.User;
 import hudson.security.ACL;
+import hudson.security.UserMayOrMayNotExistException;
 import hudson.util.Scrambler;
+import jenkins.model.Jenkins;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
+import org.acegisecurity.userdetails.UsernameNotFoundException;
+import org.springframework.dao.DataAccessException;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -41,6 +45,17 @@ public class ApiTokenFilter implements Filter {
             int idx = uidpassword.indexOf(':');
             if (idx >= 0) {
                 String username = uidpassword.substring(0, idx);
+                try {
+                    Jenkins.getInstance().getSecurityRealm().loadUserByUsername(username);
+                } catch (UserMayOrMayNotExistException x) {
+                    // OK, give them the benefit of the doubt.
+                } catch (UsernameNotFoundException x) {
+                    // Not/no longer a user; deny the API token. (But do not leak the information that this happened.)
+                    chain.doFilter(request, response);
+                    return;
+                } catch (DataAccessException x) {
+                    throw new ServletException(x);
+                }
                 String password = uidpassword.substring(idx+1);
 
                 // attempt to authenticate as API token

core/src/main/resources/hudson/model/PasswordParameterDefinition/config.jelly

@@ -30,7 +30,7 @@ THE SOFTWARE.
 		<f:textbox name="parameter.name" value="${instance.name}" />
 	</f:entry>
 	<f:entry title="${%Default Value}" help="/help/parameter/string-default.html">
-		<f:password name="parameter.defaultValue" value="${instance.defaultValue}" />
+		<f:password name="parameter.defaultValue" value="${instance.defaultValueAsSecret}" />
 	</f:entry>
     <f:entry title="${%Description}" help="/help/parameter/description.html">
         <f:textarea name="parameter.description" value="${instance.description}" />

core/src/main/resources/lib/hudson/setIconSize.jelly

@@ -27,7 +27,7 @@ THE SOFTWARE.
 -->
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:x="jelly:xml" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-  <j:set scope="parent" var="iconSize" value="${h.getCookie(request,'iconSize','32x32')}" />
+  <j:set scope="parent" var="iconSize" value="${h.validateIconSize(h.getCookie(request,'iconSize','32x32'))}" />
   <!--
     balls look smaller than their actual size,
     so we try not to make the secondary icons look bigger than the icon.

core/src/main/resources/lib/layout/layout.jelly

@@ -56,6 +56,7 @@ THE SOFTWARE.
 <st:setHeader name="Expires" value="0" />
 <st:setHeader name="Cache-Control" value="no-cache,must-revalidate" />
 <st:setHeader name="X-Hudson-Theme" value="default" />
+<st:setHeader name="X-Frame-Options" value="sameorigin" />
 <st:contentType value="text/html;charset=UTF-8" />
 
   <j:new var="h" className="hudson.Functions" /><!-- instead of JSP functions -->

core/src/test/java/hudson/markup/MyspacePolicyTest.java

@@ -34,9 +34,7 @@ public class MyspacePolicyTest extends Assert {
         assertIntact("<table><tr><th>header</th></tr><tr><td>something</td></tr></table>");
         assertIntact("<h1>title</h1><blockquote>blurb</blockquote>");
 
-        assertIntact("<iframe src='nested'></iframe>");
-        assertIntact("<iframe src='http://kohsuke.org'></iframe>");
-        assertReject("javascript","<iframe src='javascript:foo'></iframe>");
+        assertReject("iframe", "<iframe src='nested'></iframe>");
 
         assertReject("script","<script>window.alert(5);</script>");
         assertReject("script","<script src='http://foo/evil.js'></script>");
@@ -47,7 +45,7 @@ public class MyspacePolicyTest extends Assert {
         assertIntact("<div style='background-color:white'>inline CSS</div>");
         assertIntact("<br><hr>");
 
-        assertIntact("<form method='post' action='http://sun.com/'><input type='text' name='foo'><input type='password' name='pass'></form>");
+        assertReject("sun.com", "<form method='post' action='http://sun.com/'><input type='text' name='foo'><input type='password' name='pass'></form>");
     }
 
     private void assertIntact(String input) {

test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java

+/*
+ * The MIT License
+ *
+ * Copyright 2013 Jesse Glick.
+ *
+ * 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.model;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class PasswordParameterDefinitionTest {
+
+    @Rule public JenkinsRule j = new JenkinsRule();
+
+    @Test public void defaultValueKeptSecret() throws Exception {
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr3t", "")));
+        j.configRoundtrip(p);
+        assertEquals("s3cr3t", ((PasswordParameterDefinition) p.getProperty(ParametersDefinitionProperty.class).getParameterDefinition("p")).getDefaultValue());
+    }
+
+}

test/src/test/java/hudson/tasks/BuildTriggerTest.java

@@ -23,14 +23,26 @@
  */
 package hudson.tasks;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
 import hudson.maven.MavenModuleSet;
 import hudson.maven.MavenModuleSetBuild;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
+import hudson.model.Item;
 import hudson.model.Result;
 import hudson.model.Run;
+import hudson.security.AuthorizationMatrixProperty;
+import hudson.security.LegacySecurityRealm;
+import hudson.security.Permission;
+import hudson.security.ProjectMatrixAuthorizationStrategy;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import jenkins.model.Jenkins;
 import org.jvnet.hudson.test.ExtractResourceSCM;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.MockBuilder;
@@ -131,4 +143,36 @@ public class BuildTriggerTest extends HudsonTestCase {
     public void testMavenTriggerEvenWhenUnstable() throws Exception {
         doMavenTriggerTest(true);
     }
+
+    public void testConfigureDownstreamProjectSecurity() throws Exception {
+        jenkins.setSecurityRealm(new LegacySecurityRealm());
+        ProjectMatrixAuthorizationStrategy auth = new ProjectMatrixAuthorizationStrategy();
+        auth.add(Jenkins.READ, "alice");
+        jenkins.setAuthorizationStrategy(auth);
+        FreeStyleProject upstream = createFreeStyleProject("upstream");
+        Map<Permission,Set<String>> perms = new HashMap<Permission,Set<String>>();
+        perms.put(Item.READ, Collections.singleton("alice"));
+        perms.put(Item.CONFIGURE, Collections.singleton("alice"));
+        upstream.addProperty(new AuthorizationMatrixProperty(perms));
+        FreeStyleProject downstream = createFreeStyleProject("downstream");
+        /* Original SECURITY-55 test case:
+        downstream.addProperty(new AuthorizationMatrixProperty(Collections.singletonMap(Item.READ, Collections.singleton("alice"))));
+        */
+        WebClient wc = createWebClient();
+        wc.login("alice");
+        HtmlPage page = wc.getPage(upstream, "configure");
+        HtmlForm config = page.getFormByName("config");
+        config.getButtonByCaption("Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly
+        page.getAnchorByText("Build other projects").click();
+        HtmlTextInput childProjects = config.getInputByName("buildTrigger.childProjects");
+        childProjects.setValueAttribute("downstream");
+        try {
+            submit(config);
+            fail();
+        } catch (FailingHttpStatusCodeException x) {
+            assertEquals(403, x.getStatusCode());
+        }
+        assertEquals(Collections.emptyList(), upstream.getDownstreamProjects());
+    }
+
 }