From 81ef4269d76477f11c4cbd9fb536de81e3a53060 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Tue, 17 Sep 2024 18:55:27 +1000 Subject: [PATCH] In FIPS environment password must be at least 14 characters (#558) Enforce minimum password length or 14 characters when running in a FIPS environment --------- Signed-off-by: Olivier Lamy Co-authored-by: James Nord --- pom.xml | 1 + .../impl/UsernamePasswordCredentialsImpl.java | 22 ++++- ...rnamePasswordCredentialsSnapshotTaker.java | 12 ++- .../credentials/impl/Messages.properties | 1 + .../CredentialsParameterBinderTest.java | 2 +- .../credentials/cli/CLICommandsTest.java | 2 +- .../impl/BaseStandardCredentialsTest.java | 6 +- ...ernamePasswordCredentialsImplFIPSTest.java | 91 +++++++++++++++++++ .../UsernamePasswordCredentialsImplTest.java | 6 +- 9 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplFIPSTest.java diff --git a/pom.xml b/pom.xml index 0570b2c39..113fb88ea 100644 --- a/pom.xml +++ b/pom.xml @@ -68,6 +68,7 @@ 999999-SNAPSHOT jenkinsci/${project.artifactId}-plugin 2.426.3 + 1372 diff --git a/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java b/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java index bb947482c..34321f8fa 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java @@ -30,11 +30,19 @@ import edu.umd.cs.findbugs.annotations.Nullable; import hudson.Extension; import hudson.Util; +import hudson.model.Descriptor; +import hudson.util.FormValidation; import hudson.util.Secret; +import jenkins.security.FIPS140; +import org.apache.commons.lang.StringUtils; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.interceptor.RequirePOST; + +import java.util.Objects; /** * Concrete implementation of {@link StandardUsernamePasswordCredentials}. @@ -73,9 +81,13 @@ public class UsernamePasswordCredentialsImpl extends BaseStandardCredentials imp @SuppressWarnings("unused") // by stapler public UsernamePasswordCredentialsImpl(@CheckForNull CredentialsScope scope, @CheckForNull String id, @CheckForNull String description, - @CheckForNull String username, @CheckForNull String password) { + @CheckForNull String username, @CheckForNull String password) + throws Descriptor.FormException { super(scope, id, description); this.username = Util.fixNull(username); + if(FIPS140.useCompliantAlgorithms() && StringUtils.length(password) < 14) { + throw new Descriptor.FormException(Messages.passwordTooShortFIPS(), "password"); + } this.password = Secret.fromString(password); } @@ -128,5 +140,13 @@ public String getDisplayName() { public String getIconClassName() { return "symbol-id-card"; } + + @RequirePOST + public FormValidation doCheckPassword(@QueryParameter String password) { + if(FIPS140.useCompliantAlgorithms() && StringUtils.length(password) < 14) { + return FormValidation.error(Messages.passwordTooShortFIPS()); + } + return FormValidation.ok(); + } } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsSnapshotTaker.java b/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsSnapshotTaker.java index d429ff916..85dddb6f2 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsSnapshotTaker.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsSnapshotTaker.java @@ -4,6 +4,7 @@ import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; import hudson.Extension; +import hudson.model.Descriptor; import hudson.util.Secret; @Extension @@ -25,8 +26,13 @@ public StandardUsernamePasswordCredentials snapshot(StandardUsernamePasswordCred if (credentials instanceof UsernamePasswordCredentialsImpl) { return credentials; } - UsernamePasswordCredentialsImpl snapshot = new UsernamePasswordCredentialsImpl(credentials.getScope(), credentials.getId(), credentials.getDescription(), credentials.getUsername(), Secret.toString(credentials.getPassword())); - snapshot.setUsernameSecret(credentials.isUsernameSecret()); - return snapshot; + try { + UsernamePasswordCredentialsImpl snapshot = + new UsernamePasswordCredentialsImpl(credentials.getScope(), credentials.getId(), credentials.getDescription(), credentials.getUsername(), Secret.toString(credentials.getPassword())); + snapshot.setUsernameSecret(credentials.isUsernameSecret()); + return snapshot; + } catch (Descriptor.FormException e) { + throw new RuntimeException(e); + } } } \ No newline at end of file diff --git a/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties b/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties index 608714530..90ab3277c 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties @@ -44,3 +44,4 @@ CertificateCredentialsImpl.PEMMultipleKeys=More than 1 key provided CertificateCredentialsImpl.PEMNonKeys=PEM contains non key entries CertificateCredentialsImpl.PEMKeyInfo={0} {1} private key CertificateCredentialsImpl.PEMKeyParseError=Could not parse key: {0} +passwordTooShortFIPS=Password is too short (< 14 characters) diff --git a/src/test/java/com/cloudbees/plugins/credentials/builds/CredentialsParameterBinderTest.java b/src/test/java/com/cloudbees/plugins/credentials/builds/CredentialsParameterBinderTest.java index edcd10ce1..c0bfaba53 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/builds/CredentialsParameterBinderTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/builds/CredentialsParameterBinderTest.java @@ -61,7 +61,7 @@ public class CredentialsParameterBinderTest { private static final String PARAMETER_NAME = "cred"; @BeforeClass - public static void setUpClass() throws IOException { + public static void setUpClass() throws Exception { j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); CredentialsProvider.lookupStores(j.jenkins).iterator().next() .addCredentials(Domain.global(), new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, GLOBAL_CREDENTIALS_ID, "global credential", "root", "correct horse battery staple")); diff --git a/src/test/java/com/cloudbees/plugins/credentials/cli/CLICommandsTest.java b/src/test/java/com/cloudbees/plugins/credentials/cli/CLICommandsTest.java index e142a1f59..1f1cf8880 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/cli/CLICommandsTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/cli/CLICommandsTest.java @@ -307,7 +307,7 @@ public void deleteSmokes() throws Exception { } @Test - public void listCredentialsAsXML() throws IOException { + public void listCredentialsAsXML() throws Exception { Domain smokes = new Domain("smokes", "smoke test domain", Collections.singletonList(new HostnameSpecification("smokes.example.com", null))); UsernamePasswordCredentialsImpl smokey = diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/BaseStandardCredentialsTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/BaseStandardCredentialsTest.java index ec1a1e17c..bc8201adb 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/impl/BaseStandardCredentialsTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/BaseStandardCredentialsTest.java @@ -35,13 +35,11 @@ import hudson.security.ACL; import hudson.security.ACLContext; import hudson.util.FormValidation; -import java.io.IOException; import java.util.Iterator; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockFolder; -import org.xml.sax.SAXException; import static hudson.util.FormValidation.Kind.ERROR; import static hudson.util.FormValidation.Kind.OK; @@ -136,7 +134,7 @@ public void doCheckIdDuplication() throws Exception { } @Test - public void noIDValidationMessageOnCredentialsUpdate() throws IOException, SAXException { + public void noIDValidationMessageOnCredentialsUpdate() throws Exception { // create credentials with ID test CredentialsStore store = lookupStore(r.jenkins); addCreds(store, CredentialsScope.GLOBAL, "test"); @@ -154,7 +152,7 @@ private static CredentialsStore lookupStore(ModelObject object) { return store; } - private static void addCreds(CredentialsStore store, CredentialsScope scope, String id) throws IOException { + private static void addCreds(CredentialsStore store, CredentialsScope scope, String id) throws Exception { // For purposes of this test we do not care about domains. store.addCredentials(Domain.global(), new UsernamePasswordCredentialsImpl(scope, id, null, "x", "y")); } diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplFIPSTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplFIPSTest.java new file mode 100644 index 000000000..c8ef9a51c --- /dev/null +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplFIPSTest.java @@ -0,0 +1,91 @@ +package com.cloudbees.plugins.credentials.impl; + +import com.cloudbees.plugins.credentials.CredentialsMatchers; +import com.cloudbees.plugins.credentials.CredentialsProvider; +import com.cloudbees.plugins.credentials.CredentialsScope; +import com.cloudbees.plugins.credentials.CredentialsStore; +import com.cloudbees.plugins.credentials.domains.Domain; +import hudson.ExtensionList; +import hudson.model.Descriptor; +import hudson.security.ACL; +import hudson.util.FormValidation; +import jenkins.model.Jenkins; +import org.apache.commons.text.StringEscapeUtils; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.RealJenkinsRule; + +import java.io.IOException; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThrows; + +public class UsernamePasswordCredentialsImplFIPSTest { + + @Rule public RealJenkinsRule rule = new RealJenkinsRule().javaOptions("-Djenkins.security.FIPS140.COMPLIANCE=true", "-Xmx512M") + .withDebugPort(8000).withDebugServer(true).withDebugSuspend(true); + + @Test + public void nonCompliantLaunchExceptionTest() throws Throwable { + rule.then(r -> { + new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "all-good-beer", "Best captain and player in the world", + "Pat Cummins", "theaustraliancricketteamisthebest"); + assertThrows(Descriptor.FormException.class, () -> new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "bad-foo", "someone", + "Virat", "tooshort")); + assertThrows(Descriptor.FormException.class, () -> new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "bad-bar", "duck", + "Rohit", "")); + assertThrows(Descriptor.FormException.class, () -> new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "bad-foo", "not too bad", + "Gill", null)); + }); + } + + @Test + public void invalidIsNotSavedInFIPSModeTest() throws Throwable { + rule.then(r -> + { + UsernamePasswordCredentialsImpl entry = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "all-good", "Best captain and player in the world", + "Pat Cummins", "theaustraliancricketteamisthebest"); + CredentialsStore store = CredentialsProvider.lookupStores(Jenkins.get()).iterator().next(); + store.addCredentials(Domain.global(), entry); + store.save(); + // Valid password is saved + UsernamePasswordCredentialsImpl cred = CredentialsMatchers.firstOrNull( + CredentialsProvider.lookupCredentialsInItem(UsernamePasswordCredentialsImpl.class, null, ACL.SYSTEM2), + CredentialsMatchers.withId("all-good")); + assertThat(cred, notNullValue()); + assertThrows(Descriptor.FormException.class, () -> store.addCredentials(Domain.global(), new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "all-good", "someone", + "foo", "tooshort"))); + store.save(); + // Invalid password size threw an exception, so it wasn't saved + cred = CredentialsMatchers.firstOrNull( + CredentialsProvider.lookupCredentialsInItem(UsernamePasswordCredentialsImpl.class, null, ACL.SYSTEM2), + CredentialsMatchers.withId("all-good")); + assertThat(cred, notNullValue()); + assertThat(cred.getPassword().getPlainText(), is("theaustraliancricketteamisthebest")); + }); + } + + private static void checkInvalidKeyIsNotSavedInFIPSMode(JenkinsRule r) throws IOException { + + } + + @Test + public void formValidationTest() throws Throwable { + rule.then(r -> { + UsernamePasswordCredentialsImpl.DescriptorImpl descriptor = ExtensionList.lookupSingleton(UsernamePasswordCredentialsImpl.DescriptorImpl.class); + FormValidation result = descriptor.doCheckPassword("theaustraliancricketteamisthebest"); + assertThat(result.getMessage(), nullValue()); + result = descriptor.doCheckPassword("foo"); + assertThat(result.getMessage(), not(emptyString())); + assertThat(result.getMessage(), containsString(StringEscapeUtils.escapeHtml4(Messages.passwordTooShortFIPS()))); + }); + } + +} diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplTest.java index 4ff083003..9c3f83558 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImplTest.java @@ -27,6 +27,8 @@ import com.cloudbees.plugins.credentials.CredentialsNameProvider; import com.cloudbees.plugins.credentials.CredentialsScope; import java.util.logging.Level; + +import hudson.model.Descriptor; import jenkins.model.Jenkins; import org.junit.Test; import static org.junit.Assert.*; @@ -40,7 +42,7 @@ public class UsernamePasswordCredentialsImplTest { @Rule public JenkinsRule r = new JenkinsRule(); // needed for Secret.fromString to work @Rule public LoggerRule logging = new LoggerRule().record(CredentialsNameProvider.class, Level.FINE); - @Test public void displayName() { + @Test public void displayName() throws Exception { UsernamePasswordCredentialsImpl creds = new UsernamePasswordCredentialsImpl(null, "abc123", "Bob’s laptop", "bob", "s3cr3t"); assertEquals("bob/****** (Bob’s laptop)", CredentialsNameProvider.name(creds)); creds.setUsernameSecret(true); @@ -65,7 +67,7 @@ public class UsernamePasswordCredentialsImplTest { assertTrue(c.isUsernameSecret()); } public static final class SpecialUsernamePasswordCredentialsImpl extends UsernamePasswordCredentialsImpl { - public SpecialUsernamePasswordCredentialsImpl(CredentialsScope scope, String id, String description, String username, String password) { + public SpecialUsernamePasswordCredentialsImpl(CredentialsScope scope, String id, String description, String username, String password) throws Descriptor.FormException { super(scope, id, description, username, password); } transient boolean initialized;