[SECURITY-3513]

Co-authored-by: Kevin-CB <kguerroudj@cloudbees.com>

Author: 2 people

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

@@ -61,6 +61,7 @@
 import jenkins.model.Jenkins;
 import jenkins.model.ModelObjectWithChildren;
 import jenkins.model.ModelObjectWithContextMenu.ContextMenu;
+import jenkins.security.ExtendedReadRedaction;
 import jenkins.util.Timer;
 import jenkins.widgets.HasWidgets;
 import net.sf.json.JSONObject;
@@ -75,6 +76,7 @@
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 import org.kohsuke.stapler.verb.POST;
+import org.springframework.security.access.AccessDeniedException;
 
 /**
  * Serves as the top of {@link Computer}s in the URL hierarchy.
@@ -295,6 +297,19 @@ public synchronized void doCreateItem(StaplerRequest2 req, StaplerResponse2 rsp,
 
             // copy through XStream
             String xml = Jenkins.XSTREAM.toXML(src);
+            if (!src.hasPermission(Computer.CONFIGURE)) {
+                final String redactedConfigDotXml = ExtendedReadRedaction.applyAll(xml);
+                if (!xml.equals(redactedConfigDotXml)) {
+                    // AccessDeniedException2 does not permit a custom message, and anyway redirecting the user to the login screen is obviously pointless.
+                    throw new AccessDeniedException(
+                            Messages.ComputerSet_may_not_copy_as_it_contains_secrets_and_(
+                                    src.getNodeName(),
+                                    Jenkins.getAuthentication2().getName(),
+                                    Computer.PERMISSIONS.title,
+                                    Computer.EXTENDED_READ.name,
+                                    Computer.CONFIGURE.name));
+                }
+            }
             Node result = (Node) Jenkins.XSTREAM.fromXML(xml);
             result.setNodeName(name);
             result.holdOffLaunchUntilSave = true;

core/src/main/resources/hudson/model/Messages.properties

@@ -112,6 +112,7 @@ ComputerSet.NoSuchSlave=No such agent: {0}
 ComputerSet.SlaveAlreadyExists=Agent called ‘{0}’ already exists
 ComputerSet.SpecifySlaveToCopy=Specify which agent to copy
 ComputerSet.DisplayName=Nodes
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=May not copy {0} as it contains secrets and {1} has {2}/{3} but not /{4}
 
 Descriptor.From=(from <a href="{1}" rel="noopener noreferrer" target="_blank">{0}</a>)
 

core/src/main/resources/hudson/model/Messages_bg.properties

@@ -155,6 +155,11 @@ CLI.wait-node-offline.shortDescription=\
 ComputerSet.DisplayName=\
  Компютри
 
+# May not copy {0} as it contains secrets and {1} has {2}/{3} but not /{4}
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=\
+ Не може да копирате „{0}“, защото на него има пароли, а „{1}“ има {2}/{3}, но\
+ не и /{4}
+
 Descriptor.From=\
  (от <a href="{1}">{0}</a>)
 

core/src/main/resources/hudson/model/Messages_de.properties

@@ -107,6 +107,7 @@ ComputerSet.DisplayName=Knoten
 ComputerSet.NoSuchSlave=Agent existiert nicht: „{0}“
 ComputerSet.SlaveAlreadyExists=Ein Agent mit dem Namen „{0}“ existiert bereits.
 ComputerSet.SpecifySlaveToCopy=Geben Sie an, welcher Agent kopiert werden soll
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Kann „{0}“ nicht kopieren, da es Geheimnisse enthält und „{1}“ nur {2}/{3} aber nicht /{4} hat.
 
 Descriptor.From=(aus <a href="{1}">{0}</a>)
 

core/src/main/resources/hudson/model/Messages_fr.properties

@@ -107,6 +107,7 @@ ComputerSet.NoSuchSlave=Aucun agent: {0}
 ComputerSet.SlaveAlreadyExists=L''agent ‘{0}’ existe déjà
 ComputerSet.SpecifySlaveToCopy=Quel agent à copier
 ComputerSet.DisplayName=Nœuds
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Copie impossible de {0} car il contient des secrets et {1} possède {2}/{3} mais pas /{4}
 
 Descriptor.From=(de <a href="{1}" rel="noopener noreferrer" target="_blank">{0}</a>)
 

core/src/main/resources/hudson/model/Messages_it.properties

@@ -156,6 +156,8 @@ ComputerSet.DisplayName=Nodi
 ComputerSet.NoSuchSlave=L''agente {0} non esiste
 ComputerSet.SlaveAlreadyExists=Un agente di nome "{0}" esiste già
 ComputerSet.SpecifySlaveToCopy=Specificare l''agente da copiare
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Impossibile copiare \
+  {0} perché contiene segreti e {1} ha {2}/{3} ma non /{4}
 Descriptor.From=(da <a href="{1}">{0}</a>)
 Executor.NotAvailable=N/D
 FileParameterDefinition.DisplayName=Parametro file

core/src/main/resources/hudson/model/Messages_pt_BR.properties

@@ -105,6 +105,8 @@ UpdateCenter.Status.Success=Sucesso
 UpdateCenter.init=Iniciando o centro de atualização
 ParameterAction.DisplayName=Parâmetros
 ComputerSet.DisplayName=Nós
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Pode não conseguir copiar {0} já que contem segredos e {1} \
+  tem {2}/{3} mas não /{4}
 Run.DeletePermission.Description=\
   Esta permissão concede aos usuários o direito de manualmente apagarem construções do histórico de construções.
 Run.Summary.BrokenForALongTime=Quebrado há muito tempo

core/src/main/resources/hudson/model/Messages_sr.properties

@@ -85,6 +85,7 @@ Computer.BadChannel=Машина агента није повезана или 
 ComputerSet.SlaveAlreadyExists=Већ постоји агент са именом {0}
 ComputerSet.SpecifySlaveToCopy=Одредите агент за копирање
 ComputerSet.DisplayName=Рачунари
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Неможе се копирати {0} пошто садржи тајне и {1} има {2}/{3} а не /{4}
 Descriptor.From=(од <a href="{1}">{0}</a>)
 Executor.NotAvailable=Н/Д
 FreeStyleProject.DisplayName=Независни задатак

core/src/main/resources/hudson/model/Messages_sv_SE.properties

@@ -112,6 +112,7 @@ ComputerSet.NoSuchSlave=Det finns ingen sådan agent: {0}
 ComputerSet.SlaveAlreadyExists=En agent med namnet "{0}" finns redan
 ComputerSet.SpecifySlaveToCopy=Ange vilken agent som ska kopieras
 ComputerSet.DisplayName=Noder
+ComputerSet.may_not_copy_as_it_contains_secrets_and_=Kan inte kopiera {0} eftersom den innehåller hemligheter och {1} har {2}/{3} men inte /{4}
 
 Descriptor.From=(från <a href="{1}" rel="noopener noreferrer" target="_blank">{0}</a>)
 

test/src/test/java/lib/form/PasswordTest.java

@@ -37,6 +37,7 @@
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
 import hudson.FilePath;
 import hudson.Launcher;
 import hudson.cli.CopyJobCommand;
@@ -62,6 +63,7 @@
 import hudson.security.ACL;
 import hudson.slaves.DumbSlave;
 import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
 import hudson.tasks.BuildStepDescriptor;
 import hudson.tasks.Builder;
 import hudson.util.FormValidation;
@@ -70,6 +72,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.net.URL;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
@@ -82,7 +85,10 @@
 import jenkins.security.ExtendedReadRedaction;
 import jenkins.security.ExtendedReadSecretRedaction;
 import jenkins.tasks.SimpleBuildStep;
+import org.htmlunit.HttpMethod;
 import org.htmlunit.Page;
+import org.htmlunit.WebRequest;
+import org.htmlunit.WebResponse;
 import org.htmlunit.html.DomElement;
 import org.htmlunit.html.HtmlHiddenInput;
 import org.htmlunit.html.HtmlInput;
@@ -195,16 +201,74 @@ public void testNodeSecrets() throws Exception {
         }
     }
 
+    @For({ExtendedReadRedaction.class, ExtendedReadSecretRedaction.class})
+    @Issue("SECURITY-3513")
+    @Test
+    public void testCopyNodeSecrets() throws Exception {
+        Computer.EXTENDED_READ.setEnabled(true);
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        MockAuthorizationStrategy mockAuthorizationStrategy = new MockAuthorizationStrategy();
+        mockAuthorizationStrategy.grant(Jenkins.READ, Computer.CREATE, Computer.CONFIGURE).everywhere().to("alice");
+        mockAuthorizationStrategy.grant(Jenkins.READ, Computer.CREATE, Computer.EXTENDED_READ).everywhere().to("bob");
+        j.jenkins.setAuthorizationStrategy(mockAuthorizationStrategy);
+
+        final DumbSlave onlineSlave = j.createOnlineSlave();
+        final String secretText = "t0ps3cr3td4t4_node";
+        final Secret encryptedSecret = Secret.fromString(secretText);
+        final String encryptedSecretText = encryptedSecret.getEncryptedValue();
+
+        onlineSlave.getNodeProperties().add(new NodePropertyWithSecret(encryptedSecret));
+        onlineSlave.save();
+
+        assertThat(readString(new File(onlineSlave.getRootDir(), "config.xml").toPath()), containsString(encryptedSecretText));
+        assertEquals(2, j.getInstance().getComputers().length);
+
+        String agentCopyURL = j.getURL() + "/computer/createItem?mode=copy&from=" + onlineSlave.getNodeName() + "&name=";
+
+        { // with configure, you can copy a node containing secrets
+            try (JenkinsRule.WebClient wc = j.createWebClient().login("alice")) {
+                WebResponse rsp = wc.getPage(wc.addCrumb(new WebRequest(new URL(agentCopyURL + "aliceAgent"),
+                        HttpMethod.POST))).getWebResponse();
+                assertEquals(200, rsp.getStatusCode());
+                assertEquals(3, j.getInstance().getComputers().length);
+
+                final Page page = wc.goTo("computer/aliceAgent/config.xml", "application/xml");
+                final String content = page.getWebResponse().getContentAsString();
+
+                assertThat(content, not(containsString(secretText)));
+                assertThat(content, containsString(encryptedSecretText));
+                assertThat(content, containsString("<secret>" + encryptedSecretText + "</secret>"));
+            }
+        }
+
+        { // without configure, you cannot copy a node containing secrets
+            try (JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false).login("bob")) {
+                WebResponse rsp = wc.getPage(wc.addCrumb(new WebRequest(new URL(agentCopyURL + "bobAgent"),
+                        HttpMethod.POST))).getWebResponse();
+
+                assertEquals(403, rsp.getStatusCode());
+                assertThat(rsp.getContentAsString(), containsString("May not copy " + onlineSlave.getNodeName() + " as it contains secrets"));
+                assertEquals(3, j.getInstance().getComputers().length);
+            }
+        }
+    }
+
     public static class NodePropertyWithSecret extends NodeProperty<Node> {
         private final Secret secret;
 
+        @DataBoundConstructor
         public NodePropertyWithSecret(Secret secret) {
             this.secret = secret;
         }
 
         public Secret getSecret() {
             return secret;
         }
+
+        @Extension
+        public static class DescriptorImpl extends NodePropertyDescriptor {
+
+        }
     }
 
     @For({ExtendedReadRedaction.class, ExtendedReadSecretRedaction.class})