Make AvatarContributorallow more targeted

Author: d1vyanshu-kumar

core/src/main/java/jenkins/security/csp/AvatarContributor.java

@@ -47,43 +47,114 @@
 public class AvatarContributor implements Contributor {
     private static final Logger LOGGER = Logger.getLogger(AvatarContributor.class.getName());
 
-    private final Set<String> domains = ConcurrentHashMap.newKeySet();
+   private final Set<String> allowedSources = ConcurrentHashMap.newKeySet();
 
     @Override
     public void apply(CspBuilder cspBuilder) {
-        domains.forEach(d -> cspBuilder.add("img-src", d));
+        allowedSources.forEach(source -> cspBuilder.add("img-src", source));
     }
 
     /**
-     * Request addition of the domain of the specified URL to the allowed set of avatar image domains.
+     * Request addition of the complete URL to the allowed set of avatar image sources.
      * <p>
-     *     This is a utility method intended to accept any avatar URL from an undetermined, but trusted (for images) domain.
-     *     If the specified URL is not {@code null}, has a host, and {@code http} or {@code https} scheme, its domain will
-     *     be added to the set of allowed domains.
+     *     Unlike the previous implementation that extracted only the domain, this now
+     *     allowlists the exact URL, providing more granular security control.
      * </p>
      * <p>
      *     <strong>Important:</strong> Only implementations restricting specification of avatar URLs to at least somewhat
-     *     privileged users to should invoke this method, for example users with at least {@link hudson.model.Item#CONFIGURE}
+     *     privileged users should invoke this method, for example users with at least {@link hudson.model.Item#CONFIGURE}
      *     permission. Note that this guidance may change over time and require implementation changes.
      * </p>
+     * <p>
+     *     <strong>Note:</strong> Due to CSP header length limitations (approximately 8KB), administrators should
+     *     monitor the total number of allowlisted URLs. Issue #23887 provides mechanisms to handle extremely long
+     *     CSP headers.
+     * </p>
      *
-     * @param url The avatar image URL whose domain should be added to the list of allowed domains
+     * @param url The complete avatar image URL that should be allowlisted
      */
     public static void allow(@CheckForNull String url) {
-        String domain = extractDomainFromUrl(url);
+        String normalizedUrl = normalizeUrl(url);
 
-        if (domain == null) {
-            LOGGER.log(Level.FINE, "Skipping null domain in avatar URL: " + url);
+        if (normalizedUrl == null) {
+            LOGGER.log(Level.FINE, "Skipping invalid or null URL: " + url);
             return;
         }
 
-        if (ExtensionList.lookupSingleton(AvatarContributor.class).domains.add(domain)) {
-            LOGGER.log(Level.CONFIG, "Adding domain '" + domain + "' from avatar URL: " + url);
+        if (ExtensionList.lookupSingleton(AvatarContributor.class).allowedSources.add(normalizedUrl)) {
+            LOGGER.log(Level.CONFIG, "Adding URL '" + normalizedUrl + "' to avatar allowlist");
         } else {
-            LOGGER.log(Level.FINEST, "Skipped adding duplicate domain '" + domain + "' from avatar URL: " + url);
+            LOGGER.log(Level.FINEST, "Skipped adding duplicate URL '" + normalizedUrl + "'");
         }
     }
 
+    /**
+     * Normalizes and validates a URL for CSP inclusion.
+     * Only http and https URLs with a valid host are accepted.
+     *
+     * @param url the URL to normalize
+     * @return the normalized URL suitable for CSP, or null if invalid
+     */
+    @CheckForNull
+    private static String normalizeUrl(@CheckForNull String url) {
+        if (url == null) {
+            return null;
+        }
+        try {
+            final URI uri = new URI(url);
+            final String host = uri.getHost();
+            if (host == null) {
+                LOGGER.log(Level.FINER, "Ignoring URI without host: " + url);
+                return null;
+            }
+            // Reject URLs with embedded credentials
+            if (uri.getUserInfo() != null) {
+                LOGGER.log(Level.FINER, "Ignoring URI with user info: " + url);
+                return null;
+            }
+            String scheme = uri.getScheme();
+            if (scheme == null) {
+                LOGGER.log(Level.FINER, "Ignoring URI without scheme: " + url);
+                return null;
+            }
+            scheme = scheme.toLowerCase(Locale.ROOT);
+            if (!scheme.equals("http") && !scheme.equals("https")) {
+                LOGGER.log(Level.FINER, "Ignoring URI with unsupported scheme: " + url);
+                return null;
+            }
+
+            // ToASCII for IDNs
+            final String asciiHost = java.net.IDN.toASCII(host.toLowerCase(Locale.ROOT));
+
+            StringBuilder normalized = new StringBuilder(scheme).append("://");
+            // IPv6 needs brackets when serialized
+            if (asciiHost.contains(":") && !asciiHost.startsWith("[")) {
+                normalized.append("[").append(asciiHost).append("]");
+            } else {
+                normalized.append(asciiHost);
+            }
+
+            final int port = uri.getPort();
+            // omit default ports
+            if (port != -1) {
+                if (!(scheme.equals("http") && port == 80) && !(scheme.equals("https") && port == 443)) {
+                    normalized.append(":").append(port);
+                }
+            }
+
+            // preserve encoded path (raw) to avoid surprising decoding differences
+            final String rawPath = uri.getRawPath();
+            if (rawPath != null && !rawPath.isEmpty()) {
+                normalized.append(rawPath);
+            }
+            return normalized.toString();
+        } catch (URISyntaxException | IllegalArgumentException e) {
+            LOGGER.log(Level.FINE, "Failed to parse avatar URI: " + url, e);
+            return null;
+        }
+    }
+
+
     /**
      * Utility method extracting the domain specification for CSP fetch directives from a specified URL.
      * If the specified URL is not {@code null}, has a host, and {@code http} or {@code https} scheme, this method
@@ -93,7 +164,9 @@ public static void allow(@CheckForNull String url) {
      *
      * @param url the URL
      * @return the domain from the specified URL, or {@code null} if the URL does not satisfy the stated conditions
+     * @deprecated Use domain-based allowlisting only when necessary. Prefer URL-specific allowlisting via {@link #allow(String)}
      */
+    @Deprecated
     @CheckForNull
     public static String extractDomainFromUrl(@CheckForNull String url) {
         if (url == null) {