[JENKINS-73506] Reject fetch requests that use credentials with HTTP …

…in FIPS mode (#1615)

* Launching exception on remote fetch

* reject wrong setup for mbp scan and reject saving data

Signed-off-by: Olivier Lamy <[email protected]>

* Update src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java

Co-authored-by: Mark Waite <[email protected]>

* Update src/main/resources/jenkins/plugins/git/Messages.properties

Co-authored-by: Mark Waite <[email protected]>

* updating test

---------

Signed-off-by: Olivier Lamy <[email protected]>
Co-authored-by: Olivier Lamy <[email protected]>
Co-authored-by: Mark Waite <[email protected]>

src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java

@@ -101,6 +101,7 @@
 import jenkins.scm.api.trait.SCMTrait;
 import jenkins.scm.impl.trait.WildcardSCMHeadFilterTrait;
 import jenkins.scm.impl.trait.WildcardSCMSourceFilterTrait;
+import jenkins.security.FIPS140;
 import jenkins.util.SystemProperties;
 import net.jcip.annotations.GuardedBy;
 import org.apache.commons.lang.StringUtils;
@@ -351,6 +352,17 @@ private <T, C extends GitSCMSourceContext<C, R>, R extends GitSCMSourceRequest>
         return doRetrieve(retriever, context, listener, prune, getOwner(), delayFetch);
     }
 
+    /**
+     * Returns false if a non-TLS protocol is used when FIPS mode is enabled.
+     * @param credentialsId any credentials (can be {@code null})
+     * @param remoteUrl the git remote url
+     * @return {@code false} if using any credentials with a non TLS protocol with FIPS mode activated
+     * @see FIPS140#useCompliantAlgorithms()
+     */
+    public static boolean isFIPSCompliantTLS(String credentialsId, String remoteUrl) {
+        return !FIPS140.useCompliantAlgorithms() || StringUtils.isEmpty(credentialsId) || (!StringUtils.startsWith(remoteUrl, "http:") && !StringUtils.startsWith(remoteUrl, "git:"));
+    }
+
     @NonNull
     private <T, C extends GitSCMSourceContext<C, R>, R extends GitSCMSourceRequest> T doRetrieve(Retriever<T> retriever,
                                                                                                  @NonNull C context,
@@ -359,6 +371,12 @@ private <T, C extends GitSCMSourceContext<C, R>, R extends GitSCMSourceRequest>
                                                                                                  @CheckForNull Item retrieveContext,
                                                                                                  boolean delayFetch)
             throws IOException, InterruptedException {
+        if (!isFIPSCompliantTLS(this.getCredentialsId(), this.getRemote())) {
+            listener.fatalError(Messages.git_fips_url_notsecured());
+            LOGGER.log(Level.SEVERE, Messages.git_fips_url_notsecured());
+            throw new IllegalArgumentException(Messages.git_fips_url_notsecured());
+        }
+
         String cacheEntry = getCacheEntry();
         Lock cacheLock = getCacheLock(cacheEntry);
         cacheLock.lock();
@@ -1126,6 +1144,11 @@ protected Set<String> retrieveRevisions(@NonNull final TaskListener listener, @C
     protected List<Action> retrieveActions(@CheckForNull SCMSourceEvent event, @NonNull TaskListener listener)
             throws IOException, InterruptedException {
         final GitSCMTelescope telescope = GitSCMTelescope.of(this);
+        if (!isFIPSCompliantTLS(this.getCredentialsId(), this.getRemote())) {
+            listener.fatalError(Messages.git_fips_url_notsecured());
+            LOGGER.log(Level.SEVERE, Messages.git_fips_url_notsecured());
+            throw new IllegalArgumentException(Messages.git_fips_url_notsecured());
+        }
         if (telescope != null) {
             final String remote = getRemote();
             final StandardUsernameCredentials credentials = getCredentials();

src/main/java/jenkins/plugins/git/GitSCMSource.java

@@ -164,6 +164,10 @@ public GitSCMSource(String remote) {
 
     @DataBoundSetter
     public void setCredentialsId(@CheckForNull String credentialsId) {
+        if (!isFIPSCompliantTLS(credentialsId, this.remote)) {
+            LOGGER.log(Level.SEVERE, Messages.git_fips_url_notsecured());
+            throw new IllegalArgumentException(Messages.git_fips_url_notsecured());
+        }
         this.credentialsId = credentialsId;
     }
 
@@ -409,17 +413,6 @@ public List<SCMSourceTrait> getTraits() {
         return traits;
     }
 
-    /**
-     * Returns false if a non-TLS protocol is used when FIPS mode is enabled.
-     * @param credentialsId any credentials (can be {@code null})
-     * @param remoteUrl the git remote url
-     * @return {@code false} if using any credentials with a non TLS protocol with FIPS mode activated
-     * @see FIPS140#useCompliantAlgorithms()
-     */
-    public static boolean isFIPSCompliantTLS(String credentialsId, String remoteUrl) {
-        return !FIPS140.useCompliantAlgorithms() || !StringUtils.isNotEmpty(credentialsId) || (!StringUtils.startsWith(remoteUrl, "http:") && !StringUtils.startsWith(remoteUrl, "git:"));
-    }
-
     @Symbol("git")
     @Extension
     public static class DescriptorImpl extends SCMSourceDescriptor {

src/main/resources/jenkins/plugins/git/Messages.properties

@@ -26,3 +26,5 @@ GitStep.git=Git
 within.Repository=Within Repository
 additional=Additional
 GitUsernamePasswordBinding.DisplayName=Git Username and Password
+
+git.fips.url.notsecured=Invalid configuration will not fetch any remote. FIPS requires a secure channel for git fetch with credentials.

src/test/java/jenkins/plugins/git/FIPSModeSCMSourceTest.java

@@ -0,0 +1,58 @@
+package jenkins.plugins.git;
+
+import hudson.model.TaskListener;
+import hudson.plugins.git.GitException;
+import hudson.util.StreamTaskListener;
+import jenkins.security.FIPS140;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.FlagRule;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
+
+import java.io.IOException;
+import java.util.logging.Level;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
+public class FIPSModeSCMSourceTest {
+
+    @ClassRule
+    public static final FlagRule<String> FIPS_FLAG =
+            FlagRule.systemProperty(FIPS140.class.getName() + ".COMPLIANCE", "true");
+
+    @Rule
+    public JenkinsRule rule = new JenkinsRule();
+
+    @Rule
+    public LoggerRule logger = new LoggerRule();
+
+    @Test
+    @SuppressWarnings("deprecation")
+    public void remotesAreNotFetchedTest() throws IOException, InterruptedException {
+        GitSCMSource source = new GitSCMSource("http://insecure-repo");
+        // Credentials are null, so we should have no FIPS error
+        logger.record(AbstractGitSCMSource.class, Level.SEVERE);
+        logger.capture(10);
+        TaskListener listener = StreamTaskListener.fromStderr();
+        assertThrows("expected exception as repo doesn't exist", GitException.class, () ->source.fetch(listener));
+        assertThat("We should no see the error in the logs", logger.getMessages().size(), is(0));
+
+        // Using creds we should be getting an exception
+        Throwable exception = assertThrows("We're not saving creds", IllegalArgumentException.class, () -> source.setCredentialsId("cred-id"));
+        assertThat(exception.getMessage(), containsString("FIPS requires a secure channel"));
+        assertThat("credentials are not saved", source.getCredentialsId(), nullValue());
+
+        // Using old constructor (restricted since 3.4.0) to simulate credentials are being set with unsecure connection
+        // This would be equivalent to a user manually adding credentials to config.xml
+        GitSCMSource anotherSource = new GitSCMSource("fake", "http://insecure", "credentialsId", "", "", true);
+        exception = assertThrows("fetch was interrupted so no credential was leaked", IllegalArgumentException.class, () -> anotherSource.fetch(listener));
+        assertThat("We should have a severe log indicating the error", logger.getMessages().size(), is(1));
+        assertThat("Exception indicates problem", exception.getMessage(), containsString("FIPS requires a secure channel"));
+    }
+}