Skip to content

Commit

Permalink
[JENKINS-73506] Enforce use of TLS in FIPS mode (jenkinsci#1611)
Browse files Browse the repository at this point in the history
* [JENKINS-73506] Enforce use of TLS in FIPS mode

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

* fix test

* fix ssh keys to be FIPS compliant

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

* add test for checkout step

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

* extract to one single method

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

* test the extract method

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

* format magnify :)

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

* windauze...

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

* improve namimg

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

* control not used of git protocol with credentials in FIPS mode

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

* Use commons.lang.StringUtils instead of commons.lang3.StringUtils

Remain consistent with other usages in plugin.  These would be the first
imports fo commons.lang3 in the plugin and a method of the same name is
already provided by commons.lang.

* Make javadoc more consistent

* Use git 2.45 in all test containers

* Intentionally test modern scmGit checkout syntax

* Consistent indentation for readability

* Format new test with spotless

Eventually the whole repo will use spotless

Use spotless for this new file

* fix typo

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

---------

Signed-off-by: Olivier Lamy <[email protected]>
Co-authored-by: Mark Waite <[email protected]>
  • Loading branch information
olamy and MarkEWaite authored Jul 30, 2024
1 parent 899c4f6 commit 43a56cb
Show file tree
Hide file tree
Showing 8 changed files with 400 additions and 4 deletions.
18 changes: 18 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,24 @@
<version>1.7.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>testcontainers</artifactId>
<version>1.20.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.github.sparsick.testcontainers.gitserver</groupId>
<artifactId>testcontainers-gitserver</artifactId>
<version>0.8.0</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>ch.qos.logback</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
<!-- JCasC compatibility -->
<dependency>
<groupId>io.jenkins</groupId>
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/hudson/plugins/git/GitSCM.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package hudson.plugins.git;

import com.cloudbees.plugins.credentials.CredentialsMatcher;
import com.cloudbees.plugins.credentials.CredentialsMatchers;
import com.cloudbees.plugins.credentials.CredentialsProvider;
import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import com.google.common.collect.Iterables;

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/hudson/plugins/git/UserRemoteConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import jenkins.model.Jenkins;
import jenkins.plugins.git.GitSCMSource;
import jenkins.security.FIPS140;
import org.apache.commons.lang.StringUtils;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.transport.RemoteConfig;
import org.jenkinsci.plugins.gitclient.Git;
Expand Down Expand Up @@ -58,6 +61,9 @@ public UserRemoteConfig(String url, String name, String refspec, @CheckForNull S
this.name = fixEmpty(name);
this.refspec = fixEmpty(refspec);
this.credentialsId = fixEmpty(credentialsId);
if (FIPS140.useCompliantAlgorithms() && StringUtils.isNotEmpty(this.credentialsId) && StringUtils.startsWith(this.url, "http:")) {
throw new IllegalArgumentException(Messages.git_fips_url_notsecured());
}
}

@Exported
Expand Down Expand Up @@ -171,6 +177,10 @@ public FormValidation doCheckUrl(@AncestorInPath Item item,
@QueryParameter String credentialsId,
@QueryParameter String value) throws IOException, InterruptedException {

if (!GitSCMSource.isFIPSCompliantTLS(credentialsId, value)) {
return FormValidation.error(hudson.plugins.git.Messages.git_fips_url_notsecured());
}

// Normally this permission is hidden and implied by Item.CONFIGURE, so from a view-only form you will not be able to use this check.
// (TODO under certain circumstances being granted only USE_OWN might suffice, though this presumes a fix of JENKINS-31870.)
if (item == null && !Jenkins.get().hasPermission(Jenkins.ADMINISTER) ||
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/jenkins/plugins/git/GitSCMSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import hudson.security.ACLContext;
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;

import java.io.IOException;
import java.io.ObjectStreamException;
import java.io.PrintWriter;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -92,6 +94,8 @@
import jenkins.scm.impl.trait.Discovery;
import jenkins.scm.impl.trait.Selection;
import jenkins.scm.impl.trait.WildcardSCMHeadFilterTrait;
import jenkins.security.FIPS140;
import org.apache.commons.lang.StringUtils;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.URIish;
import org.jenkinsci.Symbol;
Expand All @@ -105,6 +109,7 @@
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.interceptor.RequirePOST;

/**
* A {@link SCMSource} that discovers branches in a git repository.
Expand Down Expand Up @@ -404,6 +409,17 @@ 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 {
Expand Down Expand Up @@ -431,6 +447,15 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context,
.includeCurrentValue(credentialsId);
}

@RequirePOST
public FormValidation doCheckRemote(@AncestorInPath Item item,
@QueryParameter String credentialsId,
@QueryParameter String remote) throws IOException, InterruptedException {
Jenkins.get().checkPermission(Jenkins.MANAGE);
return isFIPSCompliantTLS(credentialsId, remote) ? FormValidation.ok() : FormValidation.error(hudson.plugins.git.Messages.git_fips_url_notsecured());
}

@RequirePOST
public FormValidation doCheckCredentialsId(@AncestorInPath Item context,
@QueryParameter String remote,
@QueryParameter String value) {
Expand Down
4 changes: 3 additions & 1 deletion src/main/resources/hudson/plugins/git/Messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ GitPublisher.Check.Required={0} is required.
check.out.to.specific.local.branch=Check out to specific local branch
advanced.sub_modules.behaviours=Advanced sub-modules behaviours
check.out.to.a.sub_directory=Check out to a sub-directory
advanced.checkout.behaviours=Advanced checkout behaviours
advanced.checkout.behaviours=Advanced checkout behaviours

git.fips.url.notsecured=FIPS requires to use git url with TLS.
Loading

0 comments on commit 43a56cb

Please sign in to comment.