From d0177a13b308f8094086101214b8885dd4226db3 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 7 Dec 2024 20:09:13 +0000 Subject: [PATCH] Handle password with empty string for certificate auth --- .../impl/CertificateCredentialsImpl.java | 18 ++++++++++-------- .../credentials/impl/Messages.properties | 4 ++-- .../impl/CertificateCredentialsImplTest.java | 18 ++++++------------ 3 files changed, 18 insertions(+), 22 deletions(-) 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..defcddc7 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java @@ -147,12 +147,10 @@ public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope, * Helper to convert a {@link Secret} password into a {@code char[]} * * @param password the password. - * @return a {@code char[]} containing the password or {@code null} + * @return a {@code char[]} containing the password */ - @CheckForNull private static char[] toCharArray(@NonNull Secret password) { - String plainText = Util.fixEmpty(password.getPlainText()); - return plainText == null ? null : plainText.toCharArray(); + return password.getPlainText().toCharArray(); } /** @@ -248,7 +246,7 @@ public FormValidation doCheckPassword(@QueryParameter String value) { return FormValidation.error(Messages.CertificateCredentialsImpl_ShortPasswordFIPS()); } if (pw.isEmpty()) { - return FormValidation.warning(Messages.CertificateCredentialsImpl_NoPassword()); + return FormValidation.ok(Messages.CertificateCredentialsImpl_NoPassword()); } if (pw.length() < 14) { return FormValidation.warning(Messages.CertificateCredentialsImpl_ShortPassword()); @@ -624,9 +622,7 @@ protected static FormValidation validateCertificateKeystore(byte[] keystoreBytes } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | IOException e) { return FormValidation.warning(e, Messages.CertificateCredentialsImpl_LoadKeystoreFailed()); } finally { - if (passwordChars != null) { - Arrays.fill(passwordChars, ' '); - } + Arrays.fill(passwordChars, ' '); } } @@ -739,6 +735,9 @@ public FormValidation doCheckCertChain(@QueryParameter String value) { List pemEncodables = PEMEncodable.decodeAll(pemCerts, null); long count = pemEncodables.stream().map(PEMEncodable::toCertificate).filter(Objects::nonNull).count(); if (count < 1) { + if (Util.fixEmpty(value) == null) { + return FormValidation.ok(); + } return FormValidation.error(Messages.CertificateCredentialsImpl_PEMNoCertificates()); } // ensure only certs are provided. @@ -771,6 +770,9 @@ public FormValidation doCheckPrivateKey(@QueryParameter String value, List pemEncodables = PEMEncodable.decodeAll(key, toCharArray(Secret.fromString(password))); long count = pemEncodables.stream().map(PEMEncodable::toPrivateKey).filter(Objects::nonNull).count(); if (count == 0) { + if (Util.fixEmpty(value) == null) { + return FormValidation.ok(); + } return FormValidation.error(Messages.CertificateCredentialsImpl_PEMNoKeys()); } if (count > 1) { 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..e3898abb 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties @@ -24,8 +24,8 @@ UsernamePasswordCredentialsImpl.DisplayName=Username with password CertificateCredentialsImpl.DisplayName=Certificate CertificateCredentialsImpl.EmptyKeystore=Empty keystore -CertificateCredentialsImpl.LoadKeyFailed=Could retrieve key "{0}" -CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Could retrieve key "{0}". You may need to provide a password +CertificateCredentialsImpl.LoadKeyFailed=Couldn''t retrieve key for alias "{0}" +CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Couldn''t retrieve key for alias "{0}". You may need to provide a password CertificateCredentialsImpl.LoadKeystoreFailed=Could not load keystore CertificateCredentialsImpl.NoCertificateUploaded=No certificate uploaded CertificateCredentialsImpl.UploadedKeyStoreSourceDisplayName=Upload PKCS#12 certificate and key 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..670ab823 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java @@ -32,6 +32,7 @@ import com.cloudbees.plugins.credentials.SecretBytes; import com.cloudbees.plugins.credentials.common.CertificateCredentials; import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials; +import hudson.util.FormValidation; import org.htmlunit.FormEncodingType; import org.htmlunit.HttpMethod; import org.htmlunit.Page; @@ -128,14 +129,6 @@ public void doCheckUploadedKeystore_uploadedFileValid_encryptedPassword() throws assertThat(content, containsString(EXPECTED_DISPLAY_NAME)); } - @Test - @Issue("JENKINS-64542") - public void doCheckUploadedKeystore_uploadedFileValid_butMissingPassword() throws Exception { - String content = getContentFrom_doCheckUploadedKeystore("", getValidP12_base64(), ""); - assertThat(content, containsString("warning")); - assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeyFailedQueryEmptyPassword("1")))); - } - @Test @Issue("JENKINS-64542") public void doCheckUploadedKeystore_uploadedFileValid_butInvalidPassword() throws Exception { @@ -193,10 +186,11 @@ public void doCheckUploadedKeystore_keyStoreValid_encryptedPassword() throws Exc @Test @Issue("JENKINS-64542") - public void doCheckUploadedKeystore_keyStoreValid_butMissingPassword() throws Exception { - String content = getContentFrom_doCheckUploadedKeystore(getValidP12_secretBytes(), "", ""); - assertThat(content, containsString("warning")); - assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeyFailedQueryEmptyPassword("1")))); + public void doCheckPassword_missing() { + CertificateCredentialsImpl.DescriptorImpl descriptor = r.jenkins.getDescriptorByType(CertificateCredentialsImpl.DescriptorImpl.class); + FormValidation formValidation = descriptor.doCheckPassword(""); + assertThat(formValidation.kind, is(FormValidation.Kind.OK)); + assertThat(formValidation.getMessage(), is(Messages.CertificateCredentialsImpl_NoPassword())); } @Test