diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java index e07df340..ddbeb8d0 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java +++ b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java @@ -57,6 +57,7 @@ import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.CmdLineException; import org.kohsuke.args4j.Localizable; +import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest2; import org.kohsuke.stapler.StaplerResponse2; @@ -608,8 +609,14 @@ public JSONObject doAddCredentials(StaplerRequest2 req, StaplerResponse2 rsp) th .element("notificationType", "ERROR"); } store.checkPermission(CredentialsStoreAction.CREATE); - Credentials credentials = Descriptor.bindJSON(req, Credentials.class, data.getJSONObject("credentials")); - boolean credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials); + boolean credentialsWereAdded; + try { + Credentials credentials = Descriptor.bindJSON(req, Credentials.class, + data.getJSONObject("credentials")); + credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials); + } catch (HttpResponses.HttpResponseException e) { + return new JSONObject().element("message", e.getMessage()).element("notificationType", "ERROR"); + } if (credentialsWereAdded) { return new JSONObject() .element("message", "Credentials created") diff --git a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java index 34e7ccb0..b3b8c29b 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java @@ -129,9 +129,12 @@ public class CertificateCredentialsImpl extends BaseStandardCredentials implemen public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope, @CheckForNull String id, @CheckForNull String description, @CheckForNull String password, - @NonNull KeyStoreSource keyStoreSource) { + @NonNull KeyStoreSource keyStoreSource) throws Descriptor.FormException { super(scope, id, description); Objects.requireNonNull(keyStoreSource); + if (FIPS140.useCompliantAlgorithms() && StringUtils.isNotBlank(password) && password.length() < 14) { + throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_ShortPasswordFIPS(), "password"); + } this.password = Secret.fromString(password); this.keyStoreSource = keyStoreSource; // ensure the keySore is valid @@ -139,7 +142,9 @@ public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope, try { keyStoreSource.toKeyStore(toCharArray(this.password)); } catch (GeneralSecurityException | IOException e) { - throw new IllegalArgumentException("KeyStore is not valid.", e); + LOGGER.log(Level.WARNING, Messages.CertificateCredentialsImpl_InvalidKeystore(), e); + throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_InvalidKeystore(), e, + "keyStoreSource"); } } 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 34321f8f..f652cdca 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java @@ -42,8 +42,6 @@ import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.interceptor.RequirePOST; -import java.util.Objects; - /** * Concrete implementation of {@link StandardUsernamePasswordCredentials}. * 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 90ab3277..92dc7953 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties @@ -24,6 +24,7 @@ UsernamePasswordCredentialsImpl.DisplayName=Username with password CertificateCredentialsImpl.DisplayName=Certificate CertificateCredentialsImpl.EmptyKeystore=Empty keystore +CertificateCredentialsImpl.InvalidKeystore=KeyStore is not valid CertificateCredentialsImpl.LoadKeyFailed=Could retrieve key "{0}" CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Could retrieve key "{0}". You may need to provide a password CertificateCredentialsImpl.LoadKeystoreFailed=Could not load keystore diff --git a/src/main/resources/lib/credentials/select/select.js b/src/main/resources/lib/credentials/select/select.js index 1114fb97..932f19e7 100644 --- a/src/main/resources/lib/credentials/select/select.js +++ b/src/main/resources/lib/credentials/select/select.js @@ -123,6 +123,7 @@ window.credentials.addSubmit = function (_) { .catch((e) => { // notificationBar.show(...) with logging ID could be handy here? console.error("Could not add credentials:", e); + window.notificationBar.show("Credentials creation failed.", window.notificationBar["ERROR"]); }) } }; diff --git a/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java b/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java index 5703712c..a4483302 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java @@ -1,18 +1,32 @@ package com.cloudbees.plugins.credentials; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertTrue; import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials; +import com.cloudbees.plugins.credentials.impl.CertificateCredentialsImpl; +import com.cloudbees.plugins.credentials.impl.Messages; import hudson.model.UnprotectedRootAction; +import java.io.IOException; import java.util.List; +import net.sf.json.JSONObject; import org.hamcrest.Matchers; +import org.htmlunit.Page; +import org.htmlunit.html.DomNode; +import org.htmlunit.html.DomNodeList; import org.htmlunit.html.HtmlButton; +import org.htmlunit.html.HtmlElementUtil; import org.htmlunit.html.HtmlForm; +import org.htmlunit.html.HtmlFormUtil; import org.htmlunit.html.HtmlInput; +import org.htmlunit.html.HtmlOption; import org.htmlunit.html.HtmlPage; +import org.htmlunit.html.HtmlRadioButtonInput; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; @@ -56,6 +70,59 @@ public void doAddCredentialsFromPopupWorksAsExpected() throws Exception { } } + @Test + @Issue("JENKINS-74964") + public void doAddCredentialsFromPopupForInvalidPEMCertificateKeystore() throws Exception { + + try (JenkinsRule.WebClient wc = j.createWebClient()) { + HtmlPage htmlPage = wc.goTo("credentials-selection"); + + HtmlButton addCredentialsButton = htmlPage.querySelector(".credentials-add-menu"); + addCredentialsButton.fireEvent("mouseenter"); + addCredentialsButton.click(); + + HtmlButton jenkinsCredentialsOption = htmlPage.querySelector(".jenkins-dropdown__item"); + jenkinsCredentialsOption.click(); + + wc.waitForBackgroundJavaScript(4000); + HtmlForm form = htmlPage.querySelector("#credentials-dialog-form"); + String certificateDisplayName = j.jenkins.getDescriptor(CertificateCredentialsImpl.class).getDisplayName(); + String KeyStoreSourceDisplayName = j.jenkins.getDescriptor( + CertificateCredentialsImpl.PEMEntryKeyStoreSource.class).getDisplayName(); + DomNodeList allOptions = htmlPage.getDocumentElement().querySelectorAll( + "select.dropdownList option"); + boolean optionFound = selectOption(allOptions, certificateDisplayName); + assertTrue("The Certificate option was not found in the credentials type select", optionFound); + List inputs = htmlPage.getDocumentElement().getByXPath( + "//input[contains(@name, 'keyStoreSource') and following-sibling::label[contains(.,'" + + KeyStoreSourceDisplayName + "')]]"); + assertThat("query should return only a singular input", inputs, hasSize(1)); + HtmlElementUtil.click(inputs.get(0)); + wc.waitForBackgroundJavaScript(4000); + Page submit = HtmlFormUtil.submit(form); + JSONObject responseJson = JSONObject.fromObject(submit.getWebResponse().getContentAsString()); + assertThat(responseJson.getString("message"), is(Messages.CertificateCredentialsImpl_InvalidKeystore())); + assertThat(responseJson.getString("notificationType"), is("ERROR")); + } + } + + private static boolean selectOption(DomNodeList allOptions, String optionName) { + return allOptions.stream().anyMatch(domNode -> { + if (domNode instanceof HtmlOption option) { + if (option.getVisibleText().equals(optionName)) { + try { + HtmlElementUtil.click(option); + } catch (IOException e) { + throw new RuntimeException(e); + } + return true; + } + } + + return false; + }); + } + @TestExtension public static class CredentialsSelectionAction implements UnprotectedRootAction { @Override diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplFIPSTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplFIPSTest.java new file mode 100644 index 00000000..6cb9f0c4 --- /dev/null +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplFIPSTest.java @@ -0,0 +1,75 @@ +package com.cloudbees.plugins.credentials.impl; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.text.StringEscapeUtils; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.RealJenkinsRule; + +import hudson.ExtensionList; +import hudson.model.Descriptor; +import hudson.util.FormValidation; + +import com.cloudbees.plugins.credentials.CredentialsScope; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThrows; + +public class CertificateCredentialsImplFIPSTest { + + @Rule + public RealJenkinsRule rule = new RealJenkinsRule().javaOptions("-Djenkins.security.FIPS140.COMPLIANCE=true"); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + String pemCert; + String pemKey; + + @Before + public void setup() throws IOException { + pemCert = IOUtils.toString(CertificateCredentialsImplFIPSTest.class.getResource("FIPSCerts.pem"), + StandardCharsets.UTF_8); + pemKey = IOUtils.toString(CertificateCredentialsImplFIPSTest.class.getResource("FIPSKey.pem"), + StandardCharsets.UTF_8); + } + + @Test + public void doCheckPasswordTest() throws Throwable { + rule.then(r -> { + CertificateCredentialsImpl.DescriptorImpl descriptor = ExtensionList.lookupSingleton( + CertificateCredentialsImpl.DescriptorImpl.class); + FormValidation result = descriptor.doCheckPassword("passwordFipsCheck"); + assertThat(result.kind, is(FormValidation.Kind.OK)); + result = descriptor.doCheckPassword("foo"); + assertThat(result.kind, is(FormValidation.Kind.ERROR)); + assertThat(result.getMessage(), + is(StringEscapeUtils.escapeHtml4(Messages.CertificateCredentialsImpl_ShortPasswordFIPS()))); + }); + } + + @Test + public void invalidPEMKeyStoreTest() throws Throwable { + CertificateCredentialsImpl.PEMEntryKeyStoreSource storeSource = new CertificateCredentialsImpl.PEMEntryKeyStoreSource( + pemCert, pemKey); + rule.then(r -> { + new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "certificate-validation", + "Validate the certificate credentials", "passwordFipsCheck", storeSource); + + assertThrows(Descriptor.FormException.class, + () -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "certificate-validation", + "Validate the certificate password", "foo", storeSource)); + assertThrows(Descriptor.FormException.class, + () -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "certificate-validation", + "Validate the certificate keyStore", "passwordFipsCheck", + new CertificateCredentialsImpl.PEMEntryKeyStoreSource( + null, null))); + }); + } +} diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java index 96ff209a..182d3697 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java @@ -47,6 +47,7 @@ import org.htmlunit.html.HtmlRadioButtonInput; import hudson.Util; +import hudson.model.Descriptor; import hudson.model.ItemGroup; import hudson.security.ACL; import hudson.util.Secret; @@ -106,7 +107,7 @@ public void setup() throws IOException { } @Test - public void displayName() throws IOException { + public void displayName() throws IOException, Descriptor.FormException { SecretBytes uploadedKeystore = SecretBytes.fromBytes(Files.readAllBytes(p12.toPath())); CertificateCredentialsImpl.UploadedKeyStoreSource storeSource = new CertificateCredentialsImpl.UploadedKeyStoreSource(uploadedKeystore); assertEquals(EXPECTED_DISPLAY_NAME, CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource))); diff --git a/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSCerts.pem b/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSCerts.pem new file mode 100644 index 00000000..7c15c314 --- /dev/null +++ b/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSCerts.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDTzCCAjegAwIBAgIUWcaB9PB40lCu4um/CD8Ni6yNxkwwDQYJKoZIhvcNAQEL +BQAwUDELMAkGA1UEBhMCSU4xCzAJBgNVBAgMAlROMREwDwYDVQQHDAhUaXJ1cHB1 +cjEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMB4XDTI0MTEyODE2 +MzI1OFoXDTI1MTEyODE2MzI1OFowUDELMAkGA1UEBhMCSU4xCzAJBgNVBAgMAlRO +MREwDwYDVQQHDAhUaXJ1cHB1cjEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQ +dHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAo8dgK5e2+wy3 +2uRxF8hiaSFnUjfKOVDYrj+RbZ+ss9Kzxm2T3nfGcjGKHVHTvkaoduVdF95V8RtQ +I2lbwPnb/RKY6+Rlx+74e6Aw+KvCAEFa0lt0dST7IpErEetDppkkR5N/RbVXvon8 +sbJSHRZQOazFD2fG6MJJYiHqLC9xqfKOrZ07JQ9/J/mXN05mTG3gWXIlaJxf6hv3 ++n2NfkaTioor9dHGwr03S9D4CJWbJvk2006WCh9TEKCrG+oOAU/Fm+mlWndeResv +EWHiH13BMOsqWj/q9QlMYnmRl4MN00SSKKREQAUAAd6WbXg6+iavxGpN8eJw4O72 +wti2bS6I4wIDAQABoyEwHzAdBgNVHQ4EFgQUsACQZeota/vgn2vOY0EVIMoKM7Iw +DQYJKoZIhvcNAQELBQADggEBAEZ7qOLFPL4QJYcD3DYohFyOR2QeQQ5sceLlCg+/ +vtET3TPqBGswHER4fkOUgehud9i/h6ff33KCgTGMpGCWkYISaAbCQgGeD8AJQIOX +aWQ12Ux/E98a4Bk9nid0moiXKTXzMBule44JolGWroxayrJnYDCo39IraJO6XxlK +SJnI1dA1uRuJl6XKHr1N/knH3QBE4wI3CjqHc9qjTXVprhRosmTykvIeLaNL/ZxW +/QnKD5VRWRmHFom03COkmPSyGWNp225dLgN+rv9e2Pvn0AP8CeUJAoWSXwxGpv9Y +AgIsGfIhwQIt3AsmmujlaLvjRQpxBh00ba77kteFR4S3WMk= +-----END CERTIFICATE----- \ No newline at end of file diff --git a/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSKey.pem b/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSKey.pem new file mode 100644 index 00000000..94b70bdd --- /dev/null +++ b/src/test/resources/com/cloudbees/plugins/credentials/impl/FIPSKey.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQChhYxialRHFZdE +Y1d6A0zTCi6pMnQlk/DU2xt2qV6xrI/Jkz0xIuDjCA/vD4EyEEh7aPEY5Ooxz6zz +tmrhF8nG2Nis4UBAxtTHqV2D5eLPfxtrRMccrIYh6121K0B45vqcHpBq/nFMNGyK +q1WJBmb4JTZ4MsZXcpe7rGBhApwgUdP/ey6jwf7oG2YCBOywZ7hu0Vs6yeSbF84R +uTuVBhVdNm2BJHMy7P2BxyDpm5RSbHQGYox68pn3kfVEdv/iC/8qwq7sbyzbKsgv +YJrpqAunTEkgLBkG/NtsSFlTzssEAtSXErfI+6b8Zd4UK6E9CoclKWyzEqMT9mVc +hox4+XL/AgMBAAECggEANYYAqBgeB1QzRRk6QpdXXNOR9MVgUZd9hbt5lU+4rl3F +ZAGjlGW/adwhE5HquQFGU4bJ5frtVEZCRJxdPGvalEcFPfyCgzSgC+2mrG+AQkwX +dOtco7bT1+ebrM5BVg8MWrGSH7JjLuJsWWM/O+HgOzhxnVEOAqpZd3o+kccAn4CX +XjxbIc3/VJ4PXm7kOfU4NL7tJ1G81UQMTNMODisvi++yAa+Rw30NpnQ15GcdnWfF +D/w2ZQW1MSbNFJzXWettDUm87oszNtIBRbyqQZD1WmHjCc/DON7vx8+8sN152zjM +ZKsb/4K/3pG0JTvR7C0MM+5ZGUU/0j+JNUavF5gEuQKBgQDNfi2lP0YZ0oesA+Yu +I6c1TKJefXfLY2HMFTagEhI879hbcNvWKYL+/yIKf4GJCAvUTcmOzPCPGW90H7S8 +dTanmD2xW2yf+8ZMf1mqdbOflhebPCQwZz2QY5ccBZ8oaa/uvKrBagoPPOm1YCAc +LJGWxRGx5cmKg212sC+DKW/IvQKBgQDJOKgcgT7CvRR2pFn1oAgraFsfarw5XQuC +4LvE1m6q/qzfv3hZeqPTEMlFy4SGX+veuxfrbdtB5VBSWiXKNkHu+BBVd1uTfEbD +tx/M3Lxb/wOZ0cADZh0kOBLxvLtim/1wfH85WtImUfGzVLdB329HtoEIE6TdTOoq +iO4bZul8awKBgC0voMPkfPqyo6i8lsHwjxUWS+HxPwVXTir9QyzBrIb/ypiY4Y5f +RHHkEk0yqn5Caa9+h2LCR+d/lVV4n1qNf74sqOw2CVXInFs36bSk+yGNdJVrDR4j +pZL5g0HjLpNJYiliDT5Infupzk5W29i2KDF6FiEDQWUW71wY8+mok+8VAoGAa2m7 +E7xKbFnSmqKRAvUyZzmFqvenElgA1RRyJ1jwKodYcPgcnmdBHGJRjthdHf4GQxdM +ZXh3Gm32un80vQTJnW7+CSF12Pz2KXOPniQWyGUQ3wOApE/WLodgVXqR7MmoOGu8 +3jkFBT+o7jnCuX80P+vEZTNXRmrQdXQy5p3A9ZECgYEAiMTdP/GEGhD4LSfkmOh/ +jjL2llf68YgLyRvTCFh+iFWbpb3fVd2L9dUt1lQpgliO7vPhw5AYxLoEx3kne8BB +zGoTWMuGboeBgGS/1iuPLMvNfJum2CHXsofr9ICO0tLHrDUw94mY25TmxlZeB0hc +UPPZOQ5AsQX82MTBSnkdHQI= +-----END PRIVATE KEY----- \ No newline at end of file