diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java index 1b9b4856ad7e..9c33baa35841 100644 --- a/cli/src/main/java/hudson/cli/CLI.java +++ b/cli/src/main/java/hudson/cli/CLI.java @@ -23,6 +23,7 @@ */ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.cli.client.Messages; import java.io.DataInputStream; import javax.net.ssl.HostnameVerifier; @@ -175,6 +176,7 @@ public static int _main(String[] _args) throws Exception { HttpsURLConnection.setDefaultSSLSocketFactory(context.getSocketFactory()); // bypass host name check, too. HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() { + @SuppressFBWarnings(value = "WEAK_HOSTNAME_VERIFIER", justification = "User set parameter to skip verifier.") public boolean verify(String s, SSLSession sslSession) { return true; } @@ -188,7 +190,7 @@ public boolean verify(String s, SSLSession sslSession) { continue; } if(head.equals("-i") && args.size()>=2) { - File f = new File(args.get(1)); + File f = getFileFromArguments(args); if (!f.exists()) { printUsage(Messages.CLI_NoSuchFileExists(f)); return -1; @@ -290,7 +292,7 @@ public boolean verify(String s, SSLSession sslSession) { if (userInfo != null) { factory = factory.basicAuth(userInfo); } else if (auth != null) { - factory = factory.basicAuth(auth.startsWith("@") ? FileUtils.readFileToString(new File(auth.substring(1)), Charset.defaultCharset()).trim() : auth); + factory = factory.basicAuth(auth.startsWith("@") ? readAuthFromFile(auth).trim() : auth); } if (mode == Mode.HTTP) { @@ -304,6 +306,16 @@ public boolean verify(String s, SSLSession sslSession) { throw new AssertionError(); } + @SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "URLCONNECTION_SSRF_FD"}, justification = "User provided values for running the program.") + private static String readAuthFromFile(String auth) throws IOException { + return FileUtils.readFileToString(new File(auth.substring(1)), Charset.defaultCharset()); + } + + @SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "URLCONNECTION_SSRF_FD"}, justification = "User provided values for running the program.") + private static File getFileFromArguments(List args) { + return new File(args.get(1)); + } + private static int webSocketConnection(String url, List args, CLIConnectionFactory factory) throws Exception { LOGGER.fine(() -> "Trying to connect to " + url + " via plain protocol over WebSocket"); class CLIEndpoint extends Endpoint { diff --git a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java index 723e97f16c26..fe8db4511691 100644 --- a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java +++ b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java @@ -1,5 +1,7 @@ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -60,7 +62,7 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio // server->client LOGGER.fine("establishing download side"); - HttpURLConnection con = (HttpURLConnection) target.openConnection(); + HttpURLConnection con = openHttpConnection(target); con.setDoOutput(true); // request POST to avoid caching con.setRequestMethod("POST"); con.addRequestProperty("Session", uuid.toString()); @@ -78,7 +80,7 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio // client->server uses chunked encoded POST for unlimited capacity. LOGGER.fine("establishing upload side"); - con = (HttpURLConnection) target.openConnection(); + con = openHttpConnection(target); con.setDoOutput(true); // request POST con.setRequestMethod("POST"); con.setChunkedStreamingMode(0); @@ -92,10 +94,15 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio LOGGER.fine("established upload side"); } - // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first. + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") + private HttpURLConnection openHttpConnection(URL target) throws IOException { + return (HttpURLConnection) target.openConnection(); + } + + // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first. @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") private URL tryToResolveRedirects(URL base, String authorization) { try { - HttpURLConnection con = (HttpURLConnection) base.openConnection(); + HttpURLConnection con = openHttpConnection(base); if (authorization != null) { con.addRequestProperty("Authorization", authorization); } diff --git a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java index c69b3330c210..cceacf2b21cb 100644 --- a/cli/src/main/java/hudson/cli/NoCheckTrustManager.java +++ b/cli/src/main/java/hudson/cli/NoCheckTrustManager.java @@ -1,5 +1,7 @@ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import javax.net.ssl.X509TrustManager; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -8,12 +10,15 @@ * @author Kohsuke Kawaguchi */ public class NoCheckTrustManager implements X509TrustManager { + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } + @SuppressFBWarnings(value = "WEAK_TRUST_MANAGER", justification = "User set parameter to skip verifier.") public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; } diff --git a/cli/src/main/java/hudson/cli/SSHCLI.java b/cli/src/main/java/hudson/cli/SSHCLI.java index 39dc9210a3e1..5321536c5c62 100644 --- a/cli/src/main/java/hudson/cli/SSHCLI.java +++ b/cli/src/main/java/hudson/cli/SSHCLI.java @@ -61,11 +61,10 @@ */ class SSHCLI { - @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE", justification = "Due to whatever reason FindBugs reports it fot try-with-resources") static int sshConnection(String jenkinsUrl, String user, List args, PrivateKeyProvider provider, final boolean strictHostKey) throws IOException { Logger.getLogger(SecurityUtils.class.getName()).setLevel(Level.WARNING); // suppress: BouncyCastle not registered, using the default JCE provider URL url = new URL(jenkinsUrl + "login"); - URLConnection conn = url.openConnection(); + URLConnection conn = openConnection(url); CLI.verifyJenkinsConnection(conn); String endpointDescription = conn.getHeaderField("X-SSH-Endpoint"); @@ -131,6 +130,11 @@ public boolean verifyServerKey(ClientSession clientSession, SocketAddress remote } } + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Client-side code doesn't involve SSRF.") + private static URLConnection openConnection(URL url) throws IOException { + return url.openConnection(); + } + private SSHCLI() {} } diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index c5369614de87..e3af9fc07cc0 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -431,6 +431,7 @@ private DependencyClassLoader findAncestorDependencyClassLoader(ClassLoader clas return null; } + @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "Administrator action installing a plugin, which could do far worse.") private static File resolve(File base, String relative) { File rel = new File(relative); if(rel.isAbsolute()) diff --git a/core/src/main/java/hudson/Proc.java b/core/src/main/java/hudson/Proc.java index 6acb1ceebaf5..f387d6abcc48 100644 --- a/core/src/main/java/hudson/Proc.java +++ b/core/src/main/java/hudson/Proc.java @@ -23,6 +23,7 @@ */ package hudson; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Launcher.ProcStarter; import hudson.model.TaskListener; import hudson.remoting.Channel; @@ -215,6 +216,7 @@ public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out, File * @param err * null to redirect stderr to stdout. */ + @SuppressFBWarnings(value = "COMMAND_INJECTION", justification = "Command injection is the point of this old, barely used class.") public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir) throws IOException { this( calcName(cmd), stderr(environment(new ProcessBuilder(cmd),env).directory(workDir), err==null || err== SELFPUMP_OUTPUT), diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 7775b4ca6afe..776fea2c4162 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -589,6 +589,8 @@ public static String ensureEndsWith(@CheckForNull String subject, @CheckForNull /** * Computes MD5 digest of the given input stream. * + * This method should only be used for non-security applications where the MD5 weakness is not a problem. + * * @param source * The stream will be closed by this method at the end of this method. * @return @@ -598,7 +600,7 @@ public static String ensureEndsWith(@CheckForNull String subject, @CheckForNull @Nonnull public static String getDigestOf(@Nonnull InputStream source) throws IOException { try { - MessageDigest md5 = MessageDigest.getInstance("MD5"); + MessageDigest md5 = getMd5(); DigestInputStream in = new DigestInputStream(source, md5); // Note: IOUtils.copy() buffers the input internally, so there is no // need to use a BufferedInputStream. @@ -618,6 +620,14 @@ public static String getDigestOf(@Nonnull InputStream source) throws IOException */ } + // TODO JENKINS-60563 remove MD5 from all usages in Jenkins + @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = + "This method should only be used for non-security applications where the MD5 weakness is not a problem.") + @Deprecated + private static MessageDigest getMd5() throws NoSuchAlgorithmException { + return MessageDigest.getInstance("MD5"); + } + @Nonnull public static String getDigestOf(@Nonnull String text) { try { diff --git a/core/src/main/java/hudson/cli/Connection.java b/core/src/main/java/hudson/cli/Connection.java index 1961447778d0..bdeb192fd3ee 100644 --- a/core/src/main/java/hudson/cli/Connection.java +++ b/core/src/main/java/hudson/cli/Connection.java @@ -23,6 +23,7 @@ */ package hudson.cli; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.remoting.ClassFilter; import hudson.remoting.ObjectInputStreamEx; import hudson.remoting.SocketChannelStream; @@ -113,6 +114,8 @@ public void writeObject(Object o) throws IOException { /** * Receives an object sent by {@link #writeObject(Object)} */ + // TODO JENKINS-60562 remove this class + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Not used. We should just remove it. Class is deprecated.") public T readObject() throws IOException, ClassNotFoundException { ObjectInputStream ois = new ObjectInputStreamEx(in, getClass().getClassLoader(), ClassFilter.DEFAULT); @@ -194,16 +197,20 @@ public KeyAgreement diffieHellman(boolean side, int keySize) throws IOException, */ public Connection encryptConnection(SecretKey sessionKey, String algorithm) throws IOException, GeneralSecurityException { Cipher cout = Cipher.getInstance(algorithm); - cout.init(Cipher.ENCRYPT_MODE, sessionKey, new IvParameterSpec(sessionKey.getEncoded())); + cout.init(Cipher.ENCRYPT_MODE, sessionKey, createIv(sessionKey)); CipherOutputStream o = new CipherOutputStream(out, cout); Cipher cin = Cipher.getInstance(algorithm); - cin.init(Cipher.DECRYPT_MODE, sessionKey, new IvParameterSpec(sessionKey.getEncoded())); + cin.init(Cipher.DECRYPT_MODE, sessionKey, createIv(sessionKey)); CipherInputStream i = new CipherInputStream(in, cin); return new Connection(i,o); } + private IvParameterSpec createIv(SecretKey sessionKey) { + return new IvParameterSpec(sessionKey.getEncoded()); + } + /** * Given a byte array that contains arbitrary number of bytes, digests or expands those bits into the specified * number of bytes without loss of entropy. diff --git a/core/src/main/java/hudson/console/AnnotatedLargeText.java b/core/src/main/java/hudson/console/AnnotatedLargeText.java index c862d0c61b18..36d2664e9208 100644 --- a/core/src/main/java/hudson/console/AnnotatedLargeText.java +++ b/core/src/main/java/hudson/console/AnnotatedLargeText.java @@ -25,6 +25,7 @@ */ package hudson.console; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import jenkins.model.Jenkins; import hudson.remoting.ObjectInputStreamEx; import java.util.concurrent.TimeUnit; @@ -128,7 +129,7 @@ private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOExcepti long timestamp = ois.readLong(); if (TimeUnit.HOURS.toMillis(1) > abs(System.currentTimeMillis()-timestamp)) // don't deserialize something too old to prevent a replay attack - return (ConsoleAnnotator) ois.readObject(); + return getConsoleAnnotator(ois); } catch (RuntimeException ex) { throw new IOException("Could not decode input", ex); } @@ -140,6 +141,11 @@ private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOExcepti return ConsoleAnnotator.initial(context); } + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Deserialization is protected by logic.") + private ConsoleAnnotator getConsoleAnnotator(ObjectInputStream ois) throws IOException, ClassNotFoundException { + return (ConsoleAnnotator) ois.readObject(); + } + @CheckReturnValue @Override public long writeLogTo(long start, Writer w) throws IOException { diff --git a/core/src/main/java/hudson/console/ConsoleNote.java b/core/src/main/java/hudson/console/ConsoleNote.java index 4d2e2c2530cb..e55f7c2629eb 100644 --- a/core/src/main/java/hudson/console/ConsoleNote.java +++ b/core/src/main/java/hudson/console/ConsoleNote.java @@ -23,6 +23,7 @@ */ package hudson.console; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.ExtensionPoint; import hudson.Functions; import hudson.MarkupText; @@ -275,7 +276,7 @@ public static ConsoleNote readFrom(DataInputStream in) throws IOException, Class try (ObjectInputStream ois = new ObjectInputStreamEx(new GZIPInputStream(new ByteArrayInputStream(buf)), jenkins != null ? jenkins.pluginManager.uberClassLoader : ConsoleNote.class.getClassLoader(), ClassFilter.DEFAULT)) { - return (ConsoleNote) ois.readObject(); + return getConsoleNote(ois); } } catch (Error e) { // for example, bogus 'sz' can result in OutOfMemoryError. @@ -284,6 +285,11 @@ public static ConsoleNote readFrom(DataInputStream in) throws IOException, Class } } + @SuppressFBWarnings(value = "OBJECT_DESERIALIZATION", justification = "Deserialization is protected by logic.") + private static ConsoleNote getConsoleNote(ObjectInputStream ois) throws IOException, ClassNotFoundException { + return (ConsoleNote) ois.readObject(); + } + /** * Skips the encoded console note. */ diff --git a/core/src/main/java/hudson/scheduler/Hash.java b/core/src/main/java/hudson/scheduler/Hash.java index 51592e6629de..833ee87e1c19 100644 --- a/core/src/main/java/hudson/scheduler/Hash.java +++ b/core/src/main/java/hudson/scheduler/Hash.java @@ -23,6 +23,8 @@ */ package hudson.scheduler; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -51,10 +53,10 @@ public abstract class Hash { * Produces an integer in [0,n) */ public abstract int next(int n); - + public static Hash from(String seed) { try { - MessageDigest md5 = MessageDigest.getInstance("MD5"); + MessageDigest md5 = getMd5(); md5.update(seed.getBytes(StandardCharsets.UTF_8)); byte[] digest = md5.digest(); @@ -77,6 +79,12 @@ public int next(int n) { } } + // TODO JENKINS-60563 remove MD5 from all usages in Jenkins + @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = "Should not be used for security.") + private static MessageDigest getMd5() throws NoSuchAlgorithmException { + return MessageDigest.getInstance("MD5"); + } + /** * Creates a hash that always return 0. */ diff --git a/core/src/main/java/hudson/security/BasicAuthenticationFilter.java b/core/src/main/java/hudson/security/BasicAuthenticationFilter.java index 62df185d1610..2c1c379d7d6f 100644 --- a/core/src/main/java/hudson/security/BasicAuthenticationFilter.java +++ b/core/src/main/java/hudson/security/BasicAuthenticationFilter.java @@ -23,6 +23,7 @@ */ package hudson.security; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.model.User; import jenkins.model.Jenkins; import hudson.util.Scrambler; @@ -162,8 +163,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha path += '?'+q; // prepare a redirect - rsp.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY); - rsp.setHeader("Location",path); + prepareRedirect(rsp, path); // ... but first let the container authenticate this request RequestDispatcher d = servletContext.getRequestDispatcher("/j_security_check?j_username="+ @@ -171,6 +171,12 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha d.include(req,rsp); } + @SuppressFBWarnings(value = "UNVALIDATED_REDIRECT", justification = "Redirect is validated as processed.") + private void prepareRedirect(HttpServletResponse rsp, String path) { + rsp.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY); + rsp.setHeader("Location",path); + } + //public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { // HttpServletRequest req = (HttpServletRequest) request; // String authorization = req.getHeader("Authorization"); diff --git a/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java b/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java index 4b86333f62d2..43798635ad74 100644 --- a/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java +++ b/core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java @@ -23,6 +23,7 @@ */ package hudson.security; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Functions; import com.google.common.base.Strings; @@ -100,15 +101,7 @@ public void commence(ServletRequest request, ServletResponse response, Authentic } catch (IllegalStateException e) { out = rsp.getWriter(); } - out.printf( - "" + - "" + - "" + - "" + - "%n" + - "%n%n"+ - "Authentication required%n"+ - " + + + + + +