Merge branch 'master' into new-weather-icons

.github/dependabot.yml

@@ -34,10 +34,6 @@ updates:
       # Fails test automation; needs further investigation.
       - dependency-name: "com.google.inject:guice-bom"
 
-      # Requires Java 11 starting with version 10.0.
-      - dependency-name: "com.puppycrawl.tools:checkstyle"
-        versions: [">=10.0"]
-
       # Contains incompatible API changes and needs compatibility work.
       - dependency-name: "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api"
 

Jenkinsfile

@@ -13,22 +13,19 @@ properties([
 ])
 
 def buildTypes = ['Linux', 'Windows']
-def jdks = [8, 11, 17]
+def jdks = [11, 17]
 
 def builds = [:]
 for (i = 0; i < buildTypes.size(); i++) {
   for (j = 0; j < jdks.size(); j++) {
     def buildType = buildTypes[i]
     def jdk = jdks[j]
-    if (buildType == 'Windows' && jdk == 8) {
-      continue // unnecessary use of hardware
-    }
     if (buildType == 'Windows' && jdk == 17) {
       continue // TODO pending jenkins-infra/helpdesk#2822
     }
     builds["${buildType}-jdk${jdk}"] = {
       // see https://github.com/jenkins-infra/documentation/blob/master/ci.adoc#node-labels for information on what node types are available
-      def agentContainerLabel = jdk == 8 ? 'maven' : 'maven-' + jdk
+      def agentContainerLabel = 'maven-' + jdk
       if (buildType == 'Windows') {
         agentContainerLabel += '-windows'
       }

bom/pom.xml

@@ -56,7 +56,7 @@ THE SOFTWARE.
       <dependency>
         <groupId>org.springframework</groupId>
         <artifactId>spring-framework-bom</artifactId>
-        <version>5.3.20</version>
+        <version>5.3.21</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
@@ -256,11 +256,6 @@ THE SOFTWARE.
         <artifactId>remoting</artifactId>
         <version>${remoting.version}</version>
       </dependency>
-      <dependency>
-        <groupId>org.jenkins-ci.modules</groupId>
-        <artifactId>instance-identity</artifactId>
-        <version>2.2</version>
-      </dependency>
       <dependency>
         <groupId>org.jfree</groupId>
         <artifactId>jfreechart</artifactId>

core/pom.xml

@@ -719,7 +719,8 @@ THE SOFTWARE.
         <artifactId>maven-surefire-plugin</artifactId>
         <!-- Version specified in grandparent POM -->
         <configuration>
-          <argLine>@{jacocoSurefireArgs}</argLine>
+          <!-- Make sure to keep the directives in test/pom.xml and war/pom.xml in sync with these. -->
+          <argLine>@{jacocoSurefireArgs} --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.desktop/com.sun.beans.introspect=ALL-UNNAMED</argLine>
           <forkCount>0.5C</forkCount>
           <reuseForks>false</reuseForks>
         </configuration>
@@ -821,23 +822,6 @@ THE SOFTWARE.
         <maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
       </properties>
     </profile>
-    <profile>
-      <id>jdk-9-and-above</id>
-      <activation>
-        <jdk>[9,)</jdk>
-      </activation>
-      <build>
-        <plugins>
-          <plugin>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <configuration>
-              <!-- Make sure to keep the directives in test/pom.xml and war/pom.xml in sync with these. -->
-              <argLine>@{jacocoSurefireArgs} --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.desktop/com.sun.beans.introspect=ALL-UNNAMED</argLine>
-            </configuration>
-          </plugin>
-        </plugins>
-      </build>
-    </profile>
     <profile>
       <id>enable-jacoco</id>
       <build>

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

@@ -2360,33 +2360,34 @@ public static String tryGetIconPath(String iconGuess, JellyContext context) {
         }
 
         StaplerRequest currentRequest = Stapler.getCurrentRequest();
-        currentRequest.getWebApp().getDispatchValidator().allowDispatch(currentRequest, Stapler.getCurrentResponse());
         String rootURL = currentRequest.getContextPath();
         Icon iconMetadata = tryGetIcon(iconGuess);
-        String iconSource = null;
 
+        String iconSource;
         if (iconMetadata != null) {
-            iconSource = iconMetadata.getQualifiedUrl(context);
+            iconSource = IconSet.tryTranslateTangoIconToSymbol(iconMetadata.getClassSpec(), () -> iconMetadata.getQualifiedUrl(context));
+        } else {
+            iconSource = guessIcon(iconGuess, rootURL);
         }
+        return iconSource;
+    }
 
-        if (iconMetadata == null) {
-            //noinspection HttpUrlsUsage
-            if (iconGuess.startsWith("http://") || iconGuess.startsWith("https://")) {
-                return iconGuess;
-            }
+    static String guessIcon(String iconGuess, String rootURL) {
+        String iconSource;
+        //noinspection HttpUrlsUsage
+        if (iconGuess.startsWith("http://") || iconGuess.startsWith("https://")) {
+            iconSource = iconGuess;
+        } else {
             if (!iconGuess.startsWith("/")) {
                 iconGuess = "/" + iconGuess;
             }
-            iconSource = rootURL + (iconGuess.startsWith("/images/") || iconGuess.startsWith("/plugin/") ? getResourcePath() : "") + iconGuess;
-        }
-
-        if (iconMetadata != null && iconMetadata.getClassSpec() != null) {
-            String translatedIcon = IconSet.tryTranslateTangoIconToSymbol(iconMetadata.getClassSpec());
-            if (translatedIcon != null) {
-                return translatedIcon;
+            if (iconGuess.startsWith(rootURL)) {
+                if ((!rootURL.equals("/images") && !rootURL.equals("/plugin")) || iconGuess.startsWith(rootURL + rootURL)) {
+                    iconGuess = iconGuess.substring(rootURL.length());
+                }
             }
+            iconSource = rootURL + (iconGuess.startsWith("/images/") || iconGuess.startsWith("/plugin/") ? getResourcePath() : "") + iconGuess;
         }
-
         return iconSource;
     }
 

core/src/main/java/hudson/PluginManager.java

@@ -1437,7 +1437,7 @@ public HttpResponse doPluginsSearch(@QueryParameter String query, @QueryParamete
                         jsonObject.put("releaseTimestamp", releaseTimestamp);
                     }
                     if (hasLatestVersionNewerThanOffered(plugin)) {
-                        jsonObject.put("newerVersionAvailableNotOffered", Messages.PluginManager_newerVersionExists(plugin.latest));
+                        jsonObject.put("newerVersionAvailableNotOffered", Messages.PluginManager_newerVersionExists(plugin.latest, plugin.wiki));
                     }
                     return jsonObject;
                 })

core/src/main/java/hudson/logging/LogRecorderManager.java

@@ -78,6 +78,9 @@
  * @author Kohsuke Kawaguchi
  */
 public class LogRecorderManager extends AbstractModelObject implements ModelObjectWithChildren, StaplerProxy {
+
+    private static final Logger LOGGER = Logger.getLogger(LogRecorderManager.class.getName());
+
     /**
      * {@link LogRecorder}s keyed by their {@linkplain LogRecorder#getName()}  name}.
      *
@@ -104,7 +107,14 @@ public void setRecorders(List<LogRecorder> recorders) {
         this.recorders = recorders;
 
         Map<String, LogRecorder> values = recorders.stream()
-                .collect(toMap(LogRecorder::getName, Function.identity()));
+                .collect(toMap(
+                        LogRecorder::getName,
+                        Function.identity(),
+                        // see JENKINS-68752, ignore duplicates
+                        (recorder1, recorder2) -> {
+                            LOGGER.warning(String.format("Ignoring duplicate log recorder '%s', check $JENKINS_HOME/log and remove the duplicate recorder", recorder2.getName()));
+                            return recorder1;
+                        }));
         ((CopyOnWriteMap<String, LogRecorder>) logRecorders).replaceBy(values);
     }
 

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

@@ -114,7 +114,6 @@
 import net.sf.json.JSONObject;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.input.CountingInputStream;
-import org.apache.commons.io.output.NullOutputStream;
 import org.jenkinsci.Symbol;
 import org.jvnet.localizer.Localizable;
 import org.kohsuke.accmod.Restricted;
@@ -1422,8 +1421,8 @@ private void testConnection(URL url) throws IOException {
                         throw new HttpRetryException("Invalid response code (" + responseCode + ") from URL: " + url, responseCode);
                     }
                 } else {
-                    try (InputStream is = connection.getInputStream()) {
-                        IOUtils.copy(is, NullOutputStream.NULL_OUTPUT_STREAM);
+                    try (InputStream is = connection.getInputStream(); OutputStream os = OutputStream.nullOutputStream()) {
+                        IOUtils.copy(is, os);
                     }
                 }
             } catch (SSLHandshakeException e) {

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

@@ -55,6 +55,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Random;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -205,7 +206,14 @@ public Details load(String username) throws UsernameNotFoundException {
 
     @Override
     protected UserDetails authenticate2(String username, String password) throws AuthenticationException {
-        Details u = load(username);
+        Details u;
+        try {
+            u = load(username);
+        } catch (UsernameNotFoundException ex) {
+            // Waste time to prevent timing attacks distinguishing existing and non-existing user
+            PASSWORD_ENCODER.matches(password, ENCODED_INVALID_USER_PASSWORD);
+            throw ex;
+        }
         if (!u.isPasswordCorrect(password)) {
             throw new BadCredentialsException("Bad credentials");
         }
@@ -963,6 +971,19 @@ public boolean isPasswordHashed(String password) {
 
     public static final MultiPasswordEncoder PASSWORD_ENCODER = new MultiPasswordEncoder();
 
+    /**
+     * This value is used to prevent timing discrepancies when trying to authenticate with an invalid username
+     * compared to just a wrong password. If the user doesn't exist, compare the provided password with this value.
+     */
+    private static final String ENCODED_INVALID_USER_PASSWORD = PASSWORD_ENCODER.encode(generatePassword());
+
+    @SuppressFBWarnings(value = {"DMI_RANDOM_USED_ONLY_ONCE", "PREDICTABLE_RANDOM"}, justification = "https://github.com/spotbugs/spotbugs/issues/1539 and doesn't need to be secure, we're just not hardcoding a 'wrong' password")
+    private static String generatePassword() {
+        String password = new Random().ints(20, 33, 127).mapToObj(i -> (char) i)
+                .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append).toString();
+        return password;
+    }
+
     @Extension @Symbol("local")
     public static final class DescriptorImpl extends Descriptor<SecurityRealm> {
         @NonNull

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

@@ -34,6 +34,7 @@
 import hudson.model.TaskListener;
 import hudson.util.FormValidation;
 import jenkins.model.Jenkins;
+import jenkins.model.identity.InstanceIdentityProvider;
 import jenkins.slaves.RemotingWorkDirSettings;
 import jenkins.util.SystemProperties;
 import jenkins.websocket.WebSockets;
@@ -251,6 +252,9 @@ public FormValidation doCheckWebSocket(@QueryParameter boolean webSocket, @Query
                 if (Jenkins.get().getTcpSlaveAgentListener() == null) {
                     return FormValidation.error("Either WebSocket mode is selected, or the TCP port for inbound agents must be enabled");
                 }
+                if (InstanceIdentityProvider.RSA.getCertificate() == null || InstanceIdentityProvider.RSA.getPrivateKey() == null) {
+                    return FormValidation.error("You must install the instance-identity plugin to use inbound agents in TCP mode");
+                }
             }
             return FormValidation.ok();
         }

core/src/main/java/jenkins/model/identity/IdentityRootAction.java

@@ -1,5 +1,7 @@
 package jenkins.model.identity;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.model.UnprotectedRootAction;
 import java.nio.charset.StandardCharsets;
@@ -11,7 +13,7 @@
  * A simple root action that exposes the public key to users so that they do not need to search for the
  * {@code X-Instance-Identity} response header, also exposes the fingerprint of the public key so that people
  * can verify a fingerprint of a master before connecting to it.
- *
+ * <p>Do not use this class from plugins. Depend on {@code instance-identity} directly instead.
  * @since 2.16
  */
 @Extension
@@ -35,7 +37,9 @@ public String getUrlName() {
      * Returns the PEM encoded public key.
      *
      * @return the PEM encoded public key.
+     *         Null if the {@code instance-identity} plugin is not enabled.
      */
+    @CheckForNull
     public String getPublicKey() {
         RSAPublicKey key = InstanceIdentityProvider.RSA.getPublicKey();
         if (key == null) {
@@ -60,6 +64,7 @@ public String getPublicKey() {
      *
      * @return the fingerprint of the public key.
      */
+    @NonNull
     public String getFingerprint() {
         return KeyUtils.fingerprint(InstanceIdentityProvider.RSA.getPublicKey());
     }

core/src/main/java/jenkins/model/identity/InstanceIdentityProvider.java

@@ -40,10 +40,13 @@
 import java.security.interfaces.RSAPublicKey;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * A source of instance identity.
- *
+ * <p>Should not be used from plugins, except to be implemented by {@code instance-identity}.
+ * Other plugins wishing to get the RSA key may depend on {@code instance-identity} directly.
  * @param <PUB>  the type of public key.
  * @param <PRIV> the type of private key.
  * @since 2.16
@@ -58,16 +61,23 @@ public abstract class InstanceIdentityProvider<PUB extends PublicKey, PRIV exten
     /**
      * RSA keys.
      */
+    @Restricted(NoExternalUse.class)
     public static final KeyTypes<RSAPublicKey, RSAPrivateKey> RSA =
             new KeyTypes<>(RSAPublicKey.class, RSAPrivateKey.class);
     /**
      * DSA keys.
+     * @deprecated unused
      */
+    @Restricted(NoExternalUse.class)
+    @Deprecated
     public static final KeyTypes<DSAPublicKey, DSAPrivateKey> DSA =
             new KeyTypes<>(DSAPublicKey.class, DSAPrivateKey.class);
     /**
      * EC keys
+     * @deprecated unused
      */
+    @Restricted(NoExternalUse.class)
+    @Deprecated
     public static final KeyTypes<ECPublicKey, ECPrivateKey> EC =
             new KeyTypes<>(ECPublicKey.class, ECPrivateKey.class);
 
@@ -77,6 +87,7 @@ public abstract class InstanceIdentityProvider<PUB extends PublicKey, PRIV exten
      * @return the {@link KeyPair} that comprises the instance identity. {@code null} could technically be returned in
      * the event that a keypair could not be generated, for example if the specific key type of this provider
      * is not permitted at the required length by the JCA policy.
+     * More commonly it just means that the {@code instance-identity} plugin needs to be installed.
      */
     @CheckForNull
     protected abstract KeyPair getKeyPair();
@@ -120,6 +131,7 @@ protected PRIV getPrivateKey() {
      * @param <PUB>  the type of public key.
      * @param <PRIV> the type of private key.
      */
+    @Restricted(NoExternalUse.class)
     public static final class KeyTypes<PUB extends PublicKey, PRIV extends PrivateKey> {
         /**
          * The interface for the public key.
@@ -154,6 +166,7 @@ private KeyTypes(Class<PUB> pubKeyType, Class<PRIV> privKeyType) {
         private static <PUB extends PublicKey, PRIV extends PrivateKey> InstanceIdentityProvider<PUB, PRIV> get(
                 @NonNull KeyTypes<PUB, PRIV> type) {
             for (InstanceIdentityProvider provider : ExtensionList.lookup(InstanceIdentityProvider.class)) {
+                LOGGER.fine(() -> "loaded " + provider + " from " + provider.getClass().getProtectionDomain().getCodeSource().getLocation());
                 try {
                     KeyPair keyPair = provider.getKeyPair();
                     if (keyPair != null
@@ -173,6 +186,7 @@ private static <PUB extends PublicKey, PRIV extends PrivateKey> InstanceIdentity
                             "Instance identity provider " + provider + " propagated an uncaught exception", e);
                 }
             }
+            LOGGER.fine("no providers");
             return null;
         }
 

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

@@ -150,14 +150,14 @@ public void afterProperties(@NonNull JnlpConnectionState event) {
     }
 
     @Override
+    @SuppressFBWarnings(value = "OS_OPEN_STREAM", justification = "Closed by hudson.slaves.SlaveComputer#kill")
     public void beforeChannel(@NonNull JnlpConnectionState event) {
         DefaultJnlpSlaveReceiver.State state = event.getStash(DefaultJnlpSlaveReceiver.State.class);
         final SlaveComputer computer = state.getNode();
         final OutputStream log = computer.openLogFile();
         state.setLog(log);
-        try (PrintWriter logw = new PrintWriter(new OutputStreamWriter(log, /* TODO switch agent logs to UTF-8 */ Charset.defaultCharset()), true)) {
-            logw.println("Inbound agent connected from " + event.getRemoteEndpointDescription());
-        }
+        PrintWriter logw = new PrintWriter(new OutputStreamWriter(log, /* TODO switch agent logs to UTF-8 */ Charset.defaultCharset()), true); // Closed by hudson.slaves.SlaveComputer#kill
+        logw.println("Inbound agent connected from " + event.getRemoteEndpointDescription());
         for (ChannelConfigurator cc : ChannelConfigurator.all()) {
             cc.onChannelBuilding(event.getChannelBuilder(), computer);
         }

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

@@ -103,11 +103,11 @@ public JnlpSlaveAgentProtocol4() throws KeyStoreException, KeyManagementExceptio
         // prepare our local identity and certificate
         X509Certificate identityCertificate = InstanceIdentityProvider.RSA.getCertificate();
         if (identityCertificate == null) {
-            throw new KeyStoreException("JENKINS-41987: no X509Certificate found; perhaps instance-identity module is missing or too old");
+            throw new KeyStoreException("JENKINS-41987: no X509Certificate found; perhaps instance-identity plugin is not installed");
         }
         RSAPrivateKey privateKey = InstanceIdentityProvider.RSA.getPrivateKey();
         if (privateKey == null) {
-            throw new KeyStoreException("JENKINS-41987: no RSAPrivateKey found; perhaps instance-identity module is missing or too old");
+            throw new KeyStoreException("JENKINS-41987: no RSAPrivateKey found; perhaps instance-identity plugin is not installed");
         }
 
         // prepare our keyStore so we can provide our authentication