Merge branch 'security-stable-2.204' into security-master

core/src/main/java/hudson/DNSMultiCast.java

@@ -93,8 +93,6 @@ public void close() {
 
     private static final Logger LOGGER = Logger.getLogger(DNSMultiCast.class.getName());
 
-    public static boolean disabled = SystemProperties.getBoolean(DNSMultiCast.class.getName()+".disabled");
-
     /**
      * Class that extends {@link JmDNSImpl} to add an abort method. Since {@link javax.jmdns.JmDNS#close()} might
      * make the instance hang during the shutdown, the abort method terminate uncleanly, but rapidly and
@@ -203,4 +201,5 @@ private void executePrivateParentMethod(String method) throws IOException {
         }
 
     }
+    public static boolean disabled = SystemProperties.getBoolean(DNSMultiCast.class.getName()+".disabled", true);
 }

core/src/main/java/hudson/Plugin.java

@@ -30,6 +30,9 @@
 import hudson.model.listeners.ItemListener;
 import hudson.model.listeners.SaveableListener;
 import hudson.model.Descriptor.FormException;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.stapler.StaplerProxy;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -80,7 +83,7 @@
  * @author Kohsuke Kawaguchi
  * @since 1.42
  */
-public abstract class Plugin implements Saveable {
+public abstract class Plugin implements Saveable, StaplerProxy {
 
     private static final Logger LOGGER = Logger.getLogger(Plugin.class.getName());
 
@@ -293,6 +296,20 @@ protected XmlFile getConfigXml() {
                 new File(Jenkins.get().getRootDir(),wrapper.getShortName()+".xml"));
     }
 
+    @Override
+    @Restricted(NoExternalUse.class)
+    public Object getTarget() {
+        if (!SKIP_PERMISSION_CHECK) {
+            Jenkins.get().checkPermission(Jenkins.READ);
+        }
+        return this;
+    }
+
+    /**
+     * Escape hatch for StaplerProxy-based access control
+     */
+    @Restricted(NoExternalUse.class)
+    public static /* Script Console modifiable */ boolean SKIP_PERMISSION_CHECK = Boolean.getBoolean(Plugin.class.getName() + ".skipPermissionCheck");
 
     /**
      * Dummy instance of {@link Plugin} to be used when a plugin didn't

core/src/main/java/hudson/UDPBroadcastThread.java

@@ -131,7 +131,8 @@ public void shutdown() {
         interrupt();
     }
 
-    public static final int PORT = SystemProperties.getInteger("hudson.udp",33848);
+    // The previous default port was 33848, before the "disabled by default" change
+    public static final int PORT = SystemProperties.getInteger("hudson.udp", -1);
 
     private static final Logger LOGGER = Logger.getLogger(UDPBroadcastThread.class.getName());
 

core/src/main/java/hudson/diagnosis/MemoryUsageMonitor.java

@@ -39,6 +39,7 @@
 import java.util.ArrayList;
 import java.io.IOException;
 
+import jenkins.model.Jenkins;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.QueryParameter;
 
@@ -102,6 +103,7 @@ private void update() {
          * Generates the memory usage statistics graph.
          */
         public TrendChart doGraph(@QueryParameter String type) throws IOException {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
             return MultiStageTimeSeries.createTrendChart(TimeScale.parse(type),used,max);
         }
     }

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

@@ -247,6 +247,10 @@ private boolean permit(StaplerRequest req) {
     protected void setHeaders(StaplerResponse rsp) {
         rsp.setHeader("X-Jenkins", Jenkins.VERSION);
         rsp.setHeader("X-Jenkins-Session", Jenkins.SESSION_HASH);
+        // to be really defensive against dumb browsers not taking into consideration the content-type being set
+        rsp.setHeader("X-Content-Type-Options", "nosniff");
+        // recommended by OWASP: https://cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html#security-headers
+        rsp.setHeader("X-Frame-Options", "deny");
     }
 
     private static final Logger LOGGER = Logger.getLogger(Api.class.getName());

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

@@ -79,6 +79,7 @@
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerProxy;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.QueryParameter;
@@ -150,7 +151,7 @@
  * @author Kohsuke Kawaguchi
  */
 @ExportedBean
-public /*transient*/ abstract class Computer extends Actionable implements AccessControlled, ExecutorListener, DescriptorByNameOwner {
+public /*transient*/ abstract class Computer extends Actionable implements AccessControlled, ExecutorListener, DescriptorByNameOwner, StaplerProxy {
 
     private final CopyOnWriteArrayList<Executor> executors = new CopyOnWriteArrayList<>();
     // TODO:
@@ -1570,6 +1571,21 @@ public void doProgressiveLog( StaplerRequest req, StaplerResponse rsp) throws IO
         getLogText().doProgressText(req, rsp);
     }
 
+    @Override
+    @Restricted(NoExternalUse.class)
+    public Object getTarget() {
+        if (!SKIP_PERMISSION_CHECK) {
+            Jenkins.get().checkPermission(Jenkins.READ);
+        }
+        return this;
+    }
+
+    /**
+     * Escape hatch for StaplerProxy-based access control
+     */
+    @Restricted(NoExternalUse.class)
+    public static /* Script Console modifiable */ boolean SKIP_PERMISSION_CHECK = Boolean.getBoolean(Computer.class.getName() + ".skipPermissionCheck");
+
     /**
      * Gets the current {@link Computer} that the build is running.
      * This method only works when called during a build, such as by

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

@@ -6,17 +6,26 @@
 import hudson.model.UnprotectedRootAction;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
+import java.util.Set;
 
 import jenkins.util.MemoryReductionUtil;
 import jenkins.model.Jenkins;
 
 import org.acegisecurity.Authentication;
 import org.acegisecurity.GrantedAuthority;
 import org.jenkinsci.Symbol;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
+import javax.annotation.Nonnull;
+
 /**
  * Expose the data needed for /whoAmI, so it can be exposed by Api.
  * 
@@ -26,6 +35,11 @@
 @Extension @Symbol("whoAmI")
 @ExportedBean
 public class WhoAmI implements UnprotectedRootAction {
+    private static final Set<String> dangerousHeaders = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
+            "cookie",
+            // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Authentication
+            "authorization", "www-authenticate", "proxy-authenticate", "proxy-authorization"
+    )));
 
     public Api getApi() {
         return new Api(this);
@@ -46,17 +60,17 @@ public boolean isAnonymous() {
         return Functions.isAnonymous();
     }
 
-    @Exported
+    // @Exported removed due to leak of sessionId with some SecurityRealm
     public String getDetails() {
         return auth().getDetails() != null ? auth().getDetails().toString() : null;
     }
 
-    @Exported
+    // @Exported removed due to leak of sessionId with some SecurityRealm
     public String getToString() {
         return auth().toString();
     }
 
-    private Authentication auth() {
+    private @Nonnull Authentication auth() {
         return Jenkins.getAuthentication();
     }
 
@@ -72,6 +86,12 @@ public String[] getAuthorities() {
         return authorities.toArray(new String[0]);
     }
 
+    // Used by Jelly
+    @Restricted(NoExternalUse.class)
+    public boolean isHeaderDangerous(@Nonnull String name) {
+        return dangerousHeaders.contains(name.toLowerCase(Locale.ENGLISH));
+    }
+
     @Override
     public String getIconFileName() {
         return null;

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -65,6 +65,7 @@
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.Beta;
 import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.QueryParameter;
@@ -785,6 +786,24 @@ public HttpResponse doSlaveAgentJnlp(StaplerRequest req, StaplerResponse res) {
         return new EncryptedSlaveAgentJnlpFile(this, "slave-agent.jnlp.jelly", getName(), CONNECT);
     }
 
+    class LowPermissionResponse {
+        @WebMethod(name="slave-agent.jnlp")
+        public HttpResponse doSlaveAgentJnlp(StaplerRequest req, StaplerResponse res) {
+            return SlaveComputer.this.doSlaveAgentJnlp(req, res);
+        }
+    }
+
+    @Override
+    @Restricted(NoExternalUse.class)
+    public Object getTarget() {
+        if (!SKIP_PERMISSION_CHECK) {
+            if (!Jenkins.get().hasPermission(Jenkins.READ)) {
+                return new LowPermissionResponse();
+            }
+        }
+        return this;
+    }
+
     @Override
     protected void kill() {
         super.kill();

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

@@ -9,6 +9,7 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
+import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 
@@ -80,7 +81,7 @@ public synchronized byte[] mac(byte[] message) {
      * Convenience method for verifying the MAC code.
      */
     public boolean checkMac(byte[] message, byte[] mac) {
-        return Arrays.equals(mac(message),mac);
+        return MessageDigest.isEqual(mac(message),mac);
     }
 
     /**
@@ -95,7 +96,7 @@ public String mac(String message) {
      * Verifies MAC constructed from {@link #mac(String)}
      */
     public boolean checkMac(String message, String mac) {
-        return mac(message).equals(mac);
+        return MessageDigest.isEqual(mac(message).getBytes(StandardCharsets.UTF_8), mac.getBytes(StandardCharsets.UTF_8));
     }
 
     private byte[] chop(byte[] mac) {

core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java

@@ -24,6 +24,8 @@
 
 import java.io.IOException;
 import java.nio.channels.ClosedChannelException;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -122,7 +124,7 @@ public void afterProperties(@Nonnull JnlpConnectionState event) {
         Channel ch = computer.getChannel();
         if (ch != null) {
             String cookie = event.getProperty(JnlpConnectionState.COOKIE_KEY);
-            if (cookie != null && cookie.equals(ch.getProperty(COOKIE_NAME))) {
+            if (cookie != null && MessageDigest.isEqual(cookie.getBytes(StandardCharsets.UTF_8), ch.getProperty(COOKIE_NAME).toString().getBytes(StandardCharsets.UTF_8))) {
                 // we think we are currently connected, but this request proves that it's from the party
                 // we are supposed to be communicating to. so let the current one get disconnected
                 LOGGER.log(Level.INFO, "Disconnecting {0} as we are reconnected from the current peer", clientName);

core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol3.java

@@ -1,16 +1,22 @@
 package jenkins.slaves;
 
 import hudson.Extension;
+import hudson.ExtensionComponent;
 import hudson.ExtensionList;
 import hudson.model.Computer;
 import java.io.IOException;
 import java.net.Socket;
 import java.util.Collections;
+import java.util.logging.Logger;
 import javax.inject.Inject;
+
 import jenkins.AgentProtocol;
+import jenkins.ExtensionFilter;
 import org.jenkinsci.Symbol;
 import org.jenkinsci.remoting.engine.JnlpConnectionState;
 import org.jenkinsci.remoting.engine.JnlpProtocol3Handler;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * Master-side implementation for JNLP3-connect protocol.
@@ -23,6 +29,10 @@
 @Extension
 @Symbol("jnlp3")
 public class JnlpSlaveAgentProtocol3 extends AgentProtocol {
+
+    private static final Logger logger = Logger.getLogger(JnlpSlaveAgentProtocol3.class.getName());
+    @Restricted(NoExternalUse.class)
+    public static final String ALLOW_UNSAFE = JnlpSlaveAgentProtocol3.class.getName() + ".ALLOW_UNSAFE";
     private NioChannelSelector hub;
 
     private JnlpProtocol3Handler handler;
@@ -61,4 +71,23 @@ public void handle(Socket socket) throws IOException, InterruptedException {
                 ExtensionList.lookup(JnlpAgentReceiver.class));
     }
 
+    @Extension
+    @Restricted(NoExternalUse.class)
+    public static class Impl extends ExtensionFilter {
+        @Override
+        public <T> boolean allows(Class<T> type, ExtensionComponent<T> component) {
+            if (Boolean.getBoolean(ALLOW_UNSAFE)) {
+                return true;
+            }
+            if (!AgentProtocol.class.isAssignableFrom(type)) {
+                return true;
+            }
+            boolean isJnlp3 = component.getInstance().getClass().isAssignableFrom(JnlpSlaveAgentProtocol3.class);
+            if (isJnlp3) {
+                logger.info("Inbound TCP Agent Protocol/3 has been forcibly disabled for additional security reasons. To enable it yet again set the system property " + ALLOW_UNSAFE);
+            }
+            return !isJnlp3;
+        }
+    }
+
 }