CVE-2018-1000107

@@ -8,7 +8,7 @@
     
     <groupId>com.synopsys.jenkinsci</groupId>
     <artifactId>ownership</artifactId>
-    <version>0.11.1-SNAPSHOT</version>
+    <version>0.12.0-SNAPSHOT</version>
     <name>Job and Node ownership plugin</name>
     <packaging>hpi</packaging>
     <description>Provides explicit ownership of jobs and nodes</description>

@@ -40,12 +40,16 @@
 
 import hudson.Extension;
 import hudson.model.Descriptor;
+import hudson.model.Items;
 import hudson.model.Job;
 import hudson.model.JobProperty;
 import hudson.model.JobPropertyDescriptor;
 import hudson.model.User;
+import hudson.util.XStream2;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Collection;
 import javax.annotation.CheckForNull;
 import javax.servlet.ServletException;
@@ -172,4 +176,14 @@ public void setItemSpecificSecurity(@CheckForNull ItemSpecificSecurity security)
         itemSpecificSecurity = security;
         owner.save();
     }
+
+    static {
+        // TODO: Remove reflection once baseline is updated past 2.85.
+        try {
+            Method m = XStream2.class.getMethod("addCriticalField", Class.class, String.class);
+            m.invoke(Items.XSTREAM2, JobOwnerJobProperty.class, "ownership");
+        } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
 }

@@ -35,6 +35,9 @@
 import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
 import hudson.slaves.SlaveComputer;
+import hudson.util.XStream2;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.List;
 import javax.annotation.CheckForNull;
 import jenkins.model.Jenkins;
@@ -147,4 +150,14 @@ public boolean isApplicable( Class<? extends Node> Type ) {
                 return true;
         }         
     }
+
+    static {
+        // TODO: Remove reflection once baseline is updated past 2.85.
+        try {
+            Method m = XStream2.class.getMethod("addCriticalField", Class.class, String.class);
+            m.invoke(Jenkins.XSTREAM2, OwnerNodeProperty.class, "ownership");
+        } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
 }

@@ -23,22 +23,37 @@
  */
 package com.synopsys.arc.jenkins.plugins.ownership;
 
+import com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerJobProperty;
 import com.synopsys.arc.jenkins.plugins.ownership.util.IdStrategyComparator;
 import com.synopsys.arc.jenkins.plugins.ownership.util.OwnershipDescriptionHelper;
 import com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Util;
+import hudson.model.Computer;
 import hudson.model.Descriptor;
+import hudson.model.Job;
+import hudson.model.Node;
 import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.AccessControlled;
+import hudson.security.Permission;
+import java.io.InvalidObjectException;
+import java.io.ObjectStreamException;
 import java.io.Serializable;
 import java.util.Collection;
+import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
+import org.acegisecurity.AccessDeniedException;
+import org.acegisecurity.Authentication;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
+import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerRequest;
 
 /**
  * Contains description of item's ownership. 
@@ -359,6 +374,62 @@ public boolean equals(Object obj) {
         return true;
     }
     
+    protected Object readResolve() throws ObjectStreamException {
+        checkUnsecuredConfiguration();
+        return this;
+    }
+
+    /**
+     * If the ownership is being deserialized because of a REST call or CLI command, we need to
+     * make sure that either the ownership is unchanged, or that the user performing the command
+     * has permission to manage ownership for the object attached to this {@link OwnershipDescription}.
+     */
+    private void checkUnsecuredConfiguration() throws ObjectStreamException {
+        Authentication authentication = Jenkins.getAuthentication();
+        if (authentication == ACL.SYSTEM) {
+            return;
+        }
+        StaplerRequest request = Stapler.getCurrentRequest();
+        if (request != null) {
+            AccessControlled context = request.findAncestorObject(AccessControlled.class);
+            if (context instanceof Job) {
+                Job<?, ?> job = (Job)context;
+                JobOwnerJobProperty existing = job.getProperty(JobOwnerJobProperty.class);
+                if (existing == null || !Objects.equals(existing.getOwnership(), this)) {
+                    throwIfMissingPermission(job, OwnershipPlugin.MANAGE_ITEMS_OWNERSHIP);
+                }
+                return;
+            } else if (context instanceof Computer) {
+                Node node = ((Computer)context).getNode();
+                if (node != null) {
+                    OwnerNodeProperty existing = node.getNodeProperties().get(OwnerNodeProperty.class);
+                    if (existing == null || !Objects.equals(existing.getOwnership(), this)) {
+                        throwIfMissingPermission(node, OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP);
+                    }
+                    return;
+                }
+            } else if (context instanceof Node) {
+                Node node = ((Node)context);
+                OwnerNodeProperty existing = node.getNodeProperties().get(OwnerNodeProperty.class);
+                if (existing == null || !Objects.equals(existing.getOwnership(), this)) {
+                    throwIfMissingPermission(node, OwnershipPlugin.MANAGE_SLAVES_OWNERSHIP);
+                }
+                return;
+            }
+        }
+        // We don't know what object this OwnershipDescription belongs to, so we require Overall/Administer permissions.
+        // CLI commands always use this check.
+        throwIfMissingPermission(Jenkins.getActiveInstance(), Jenkins.ADMINISTER);
+    }
+
+    private void throwIfMissingPermission(AccessControlled context, Permission permission) throws ObjectStreamException {
+        try {
+            context.checkPermission(permission);
+        } catch (AccessDeniedException e) {
+            throw new InvalidObjectException(e.getMessage());
+        }
+    }
+
     /**
      * Check if ownership is enabled.
      * @param descr Ownership description (can be {@code null})

@@ -0,0 +1,148 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 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 com.synopsys.arc.jenkins.plugins.ownership.jobs;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.synopsys.arc.jenkins.plugins.ownership.OwnershipDescription;
+import hudson.cli.CLICommandInvoker;
+import hudson.cli.UpdateJobCommand;
+import hudson.model.FreeStyleProject;
+import hudson.model.Item;
+import hudson.model.Job;
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import jenkins.model.Jenkins;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.JenkinsRule.WebClient;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+import static hudson.cli.CLICommandInvoker.Matcher.failedWith;
+import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+public class JobOwnerJobPropertyTest {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Before
+    public void setupSecurity() {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        MockAuthorizationStrategy mas = new MockAuthorizationStrategy();
+        mas.grant(Jenkins.ADMINISTER) // Implies MANAGE_ITEMS_OWNERSHIP.
+                .everywhere()
+                .to("admin");
+        mas.grant(Item.CONFIGURE, Item.READ, Jenkins.READ)
+                .everywhere()
+                .to("non-admin");
+        r.jenkins.setAuthorizationStrategy(mas);
+    }
+
+    @Test
+    public void changeOwnerViaPost() throws Exception {
+        FreeStyleProject p = r.createFreeStyleProject();
+        p.getProperty(JobOwnerJobProperty.class).setOwnershipDescription(new OwnershipDescription(true, "admin", null));
+
+        WebClient wc = r.createWebClient();
+        wc.login("non-admin", "non-admin");
+        WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%sconfig.xml", p.getUrl())), HttpMethod.POST);
+        req.setAdditionalHeader("Content-Type", "application/xml");
+        req.setRequestBody(getJobXml("admin"));
+        wc.getPage(req);
+        assertThat("Users should be able to configure jobs when ownership is unchanged",
+                getPrimaryOwner(p), is(equalTo("admin")));
+
+        try {
+            wc.login("non-admin", "non-admin");
+            req.setRequestBody(getJobXml("non-admin"));
+            wc.getPage(req);
+            fail("Users should not be able to configure job ownership without Manger Ownership/Jobs permissions");
+        } catch (FailingHttpStatusCodeException e) {
+            assertThat(getPrimaryOwner(p), is(equalTo("admin")));
+        }
+
+        wc.login("admin", "admin");
+        req.setRequestBody(getJobXml("non-admin"));
+        wc.getPage(req);
+        assertThat("Users with Manage Ownership/Jobs permissions should be able to change ownership",
+                getPrimaryOwner(p), is(equalTo("non-admin")));
+    }
+
+    @Test
+    public void changeOwnerViaCLI() throws Exception {
+        FreeStyleProject p = r.createFreeStyleProject();
+        p.getProperty(JobOwnerJobProperty.class).setOwnershipDescription(new OwnershipDescription(true, "admin", null));
+
+        CLICommandInvoker command = new CLICommandInvoker(r, new UpdateJobCommand())
+                .asUser("non-admin")
+                .withArgs(p.getFullName())
+                .withStdin(getJobXmlAsStream("admin"));
+        assertThat("Users without Overall/Administer permissions should not be able to configure jobs via CLI", 
+                command.invoke(), failedWith(1));
+        assertThat(getPrimaryOwner(p), is(equalTo("admin")));
+
+        command.asUser("admin")
+                .withArgs(p.getFullName())
+                .withStdin(getJobXmlAsStream("non-admin"));
+        assertThat("Users with Overall/Administer permissions should be able to configure jobs via CLI", 
+                command.invoke(), succeededSilently());
+        assertThat(getPrimaryOwner(p), is(equalTo("non-admin")));
+    }
+
+    private String getPrimaryOwner(Job<?,?> job) {
+        return job.getProperty(JobOwnerJobProperty.class).getOwnership().getPrimaryOwnerId();
+    }
+
+    private String getJobXml(String ownerSid) {
+        return String.format(JOB_XML_TEMPLATE, ownerSid);
+    }
+
+    private InputStream getJobXmlAsStream(String ownerSid) {
+        return new ByteArrayInputStream(getJobXml(ownerSid).getBytes(StandardCharsets.UTF_8));
+    }
+
+    private static final String JOB_XML_TEMPLATE =
+            "<?xml version='1.0' encoding='UTF-8'?>" +
+            "<project>" +
+            "<properties>" +
+            "    <com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerJobProperty plugin=\"ownership@0.10.1-SNAPSHOT\">" +
+            "        <ownership>" +
+            "           <ownershipEnabled>true</ownershipEnabled>" +
+            "           <primaryOwnerId>%s</primaryOwnerId>" +
+            "           <coownersIds class=\"sorted-set\"/>" +
+            "       </ownership>" +
+            "   </com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerJobProperty>" +
+            "</properties>" +
+            "</project>";
+}

@@ -0,0 +1,165 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 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 com.synopsys.arc.jenkins.plugins.ownership.nodes;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.synopsys.arc.jenkins.plugins.ownership.OwnershipDescription;
+import hudson.cli.CLICommandInvoker;
+import hudson.cli.UpdateNodeCommand;
+import hudson.model.Computer;
+import hudson.model.Node;
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import jenkins.model.Jenkins;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+import static hudson.cli.CLICommandInvoker.Matcher.failedWith;
+import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+public class OwnerNodePropertyTest {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Before
+    public void setupSecurity() {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        MockAuthorizationStrategy mas = new MockAuthorizationStrategy();
+        mas.grant(Jenkins.ADMINISTER) // Implies MANAGE_SLAVES_OWNERSHIP.
+                .everywhere()
+                .to("admin");
+        mas.grant(Computer.CONFIGURE, Jenkins.READ)
+                .everywhere()
+                .to("non-admin");
+        r.jenkins.setAuthorizationStrategy(mas);
+    }
+
+    @Test
+    public void changeOwnerViaPost() throws Exception {
+        String nodeName; // Computer#updateByXml replaces the existing node with a new instance, so we always need to look up the current instance.
+        String nodeUrl;
+        {
+            Node n = r.createSlave();
+            n.getNodeProperties().add(new OwnerNodeProperty(n, new OwnershipDescription(true, "admin", null)));
+            nodeName = n.getNodeName();
+            nodeUrl = n.toComputer().getUrl();
+        }
+
+        JenkinsRule.WebClient wc = r.createWebClient();
+        wc.login("non-admin", "non-admin");
+        WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%sconfig.xml", nodeUrl)), HttpMethod.POST);
+        req.setAdditionalHeader("Content-Type", "application/xml");
+        req.setRequestBody(getNodeXml(nodeName, "admin"));
+        wc.getPage(req);
+        assertThat("Users should be able to configure jobs when ownership is unchanged",
+                getPrimaryOwner(nodeName), is(equalTo("admin")));
+
+        try {
+            wc.login("non-admin", "non-admin");
+            req.setRequestBody(getNodeXml(nodeName, "non-admin"));
+            wc.getPage(req);
+            fail("Users should not be able to configure job ownership without Manger Ownership/Jobs permissions");
+        } catch (FailingHttpStatusCodeException e) {
+            assertThat(getPrimaryOwner(nodeName), is(equalTo("admin")));
+        }
+
+        wc.login("admin", "admin");
+        req.setRequestBody(getNodeXml(nodeName, "non-admin"));
+        wc.getPage(req);
+        assertThat("Users with Manage Ownership/Jobs permissions should be able to change ownership",
+                getPrimaryOwner(nodeName), is(equalTo("non-admin")));
+    }
+
+    @Test
+    public void changeOwnerViaCLI() throws Exception {
+        String nodeName;
+        {
+            Node n = r.createSlave();
+            n.getNodeProperties().add(new OwnerNodeProperty(n, new OwnershipDescription(true, "admin", null)));
+            nodeName = n.getNodeName();
+        }
+
+        CLICommandInvoker command = new CLICommandInvoker(r, new UpdateNodeCommand())
+                .asUser("non-admin")
+                .withArgs(nodeName)
+                .withStdin(getNodeXmlAsStream(nodeName, "admin"));
+        assertThat("Users without Overall/Administer permissions should not be able to configure nodes via CLI",
+                command.invoke(), failedWith(1));
+        assertThat(getPrimaryOwner(nodeName), is(equalTo("admin")));
+
+        command.asUser("admin")
+                .withArgs(nodeName)
+                .withStdin(getNodeXmlAsStream(nodeName, "non-admin"));
+        assertThat("Users with Overall/Administer permissions should be able to configure jobs via CLI", 
+                command.invoke(), succeededSilently());
+        assertThat(getPrimaryOwner(nodeName), is(equalTo("non-admin")));
+    }
+
+    private String getPrimaryOwner(String nodeName) {
+        return r.jenkins.getNode(nodeName).getNodeProperties().get(OwnerNodeProperty.class).getOwnership().getPrimaryOwnerId();
+    }
+
+    private String getNodeXml(String nodeName, String ownerSid) {
+        return String.format(NODE_XML_TEMPLATE, nodeName, ownerSid);
+    }
+
+    private InputStream getNodeXmlAsStream(String nodeName, String ownerSid) {
+        return new ByteArrayInputStream(getNodeXml(nodeName, ownerSid).getBytes(StandardCharsets.UTF_8));
+    }
+
+    private static final String NODE_XML_TEMPLATE =
+            "<?xml version='1.0' encoding='UTF-8'?>" +
+            "<slave>" +
+            "    <name>%s</name>" +
+            "    <description/>" +
+            "    <remoteFS>/tmp/dumbnode</remoteFS>" +
+            "    <numExecutors>1</numExecutors>" +
+            "    <mode>NORMAL</mode>" +
+            "    <retentionStrategy class=\"hudson.slaves.RetentionStrategy$Always\"/>" +
+            "    <launcher/>" +
+            "    <label/>" +
+            "    <nodeProperties>" +
+            "        <com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty plugin=\"ownership@0.10.1-SNAPSHOT\">" +
+            "            <ownership>" +
+            "                <ownershipEnabled>true</ownershipEnabled>" +
+            "                <primaryOwnerId>%s</primaryOwnerId>" +
+            "                <coownersIds class=\"sorted-set\"/>" +
+            "            </ownership>" +
+            "            <nodeName>DumbNode</nodeName>" +
+            "        </com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty>" +
+            "    </nodeProperties>" +
+            "</slave>";
+
+}