Skip to content

Commit

Permalink
[JENKINS-74964] Descriptor#newInstanceImpl Linkage error handling in …
Browse files Browse the repository at this point in the history
…doAddCredentials
  • Loading branch information
Priya-CB committed Dec 13, 2024
1 parent cfe1584 commit 0df0971
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import hudson.model.User;
import hudson.security.AccessControlled;
import hudson.security.Permission;
import hudson.util.HttpResponses;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -47,17 +49,20 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.logging.Level;
import java.util.logging.Logger;

import jakarta.servlet.ServletException;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.jenkins.ui.icon.IconSpec;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
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;
Expand All @@ -77,6 +82,8 @@ public class CredentialsSelectHelper extends Descriptor<CredentialsSelectHelper>
*/
public static final Permission CREATE = CredentialsProvider.CREATE;

private static final Logger LOGGER = Logger.getLogger(CredentialsSelectHelper.class.getName());

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -614,7 +621,27 @@ public JSONObject doAddCredentials(StaplerRequest2 req, StaplerResponse2 rsp) th
Credentials credentials = Descriptor.bindJSON(req, Credentials.class,
data.getJSONObject("credentials"));
credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials);
} catch (HttpResponses.HttpResponseException e) {
} catch (LinkageError e) {
/*
* Descriptor#newInstanceImpl throws a LinkageError if the constructor throws any exception other
* than HTTP response exceptions such as Descriptor.FormException.
*
* This approach is taken to maintain backward compatibility, as throwing a FormException directly
* from the constructor would result in a source-incompatible change, potentially breaking dependent plugins.
*
* Here, known exceptions are caught specifically to provide meaningful error response.
*/
Throwable rootCause = ExceptionUtils.getRootCause(e);
if (rootCause instanceof IOException || rootCause instanceof IllegalArgumentException
|| rootCause instanceof IllegalStateException) {
LOGGER.log(Level.WARNING, "Failed to create Credentials", e);
return new JSONObject().element("message", rootCause.getMessage()).element("notificationType",
"ERROR");
}
throw e;
} catch (IOException | IllegalArgumentException | IllegalStateException |
HttpResponses.HttpResponseException e) {
LOGGER.log(Level.WARNING, "Failed to create Credentials", e);
return new JSONObject().element("message", e.getMessage()).element("notificationType", "ERROR");
}
if (credentialsWereAdded) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,11 @@ public class CertificateCredentialsImpl extends BaseStandardCredentials implemen
public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope,
@CheckForNull String id, @CheckForNull String description,
@CheckForNull String password,
@NonNull KeyStoreSource keyStoreSource) throws Descriptor.FormException {
@NonNull KeyStoreSource keyStoreSource) {
super(scope, id, description);
Objects.requireNonNull(keyStoreSource);
if (FIPS140.useCompliantAlgorithms() && StringUtils.length(password) < 14) {
throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_ShortPasswordFIPS(), "password");
throw new IllegalArgumentException(Messages.CertificateCredentialsImpl_ShortPasswordFIPS());
}
this.password = Secret.fromString(password);
this.keyStoreSource = keyStoreSource;
Expand All @@ -142,9 +142,7 @@ public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope,
try {
keyStoreSource.toKeyStore(toCharArray(this.password));
} catch (GeneralSecurityException | IOException e) {
LOGGER.log(Level.WARNING, "Failed to create CertificateCredentials", e);
throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_InvalidKeystore(), e,
"keyStoreSource");
throw new IllegalArgumentException("KeyStore is not valid.", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

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;
Expand Down Expand Up @@ -101,7 +100,6 @@ public void doAddCredentialsFromPopupForInvalidPEMCertificateKeystore() throws E
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"));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.jvnet.hudson.test.RealJenkinsRule;

import hudson.ExtensionList;
import hudson.model.Descriptor;
import hudson.util.FormValidation;

import com.cloudbees.plugins.credentials.CredentialsScope;
Expand All @@ -25,14 +24,15 @@ public class CertificateCredentialsImplFIPSTest {

@Rule
public RealJenkinsRule rule = new RealJenkinsRule().withFIPSEnabled().javaOptions("-Xmx512m");

@Rule
public TemporaryFolder tmp = new TemporaryFolder();

private String pemCert;
private String pemKey;
private static final String VALID_PASSWORD = "passwordFipsCheck";
private static final String INVALID_PASSWORD = "foo";
private static final String INVALID_PASSWORD = "invalidPasswordFipsCheck";
private static final String SHORT_PASSWORD = "password";

@Before
public void setup() throws IOException {
Expand All @@ -49,7 +49,7 @@ public void doCheckPasswordTest() throws Throwable {
CertificateCredentialsImpl.DescriptorImpl.class);
FormValidation result = descriptor.doCheckPassword(VALID_PASSWORD);
assertThat(result.kind, is(FormValidation.Kind.OK));
result = descriptor.doCheckPassword(INVALID_PASSWORD);
result = descriptor.doCheckPassword(SHORT_PASSWORD);
assertThat(result.kind, is(FormValidation.Kind.ERROR));
assertThat(result.getMessage(),
is(StringEscapeUtils.escapeHtml4(Messages.CertificateCredentialsImpl_ShortPasswordFIPS())));
Expand All @@ -63,16 +63,22 @@ public void invalidPEMKeyStoreAndPasswordTest() throws Throwable {
rule.then(r -> {
new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "valid-certificate-and-password-validation",
"Validate the certificate credentials", VALID_PASSWORD, storeSource);
assertThrows(Descriptor.FormException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "password-length-validation",
"Validate the certificate password", "", storeSource));
assertThrows(Descriptor.FormException.class,
assertThrows(IllegalArgumentException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "empty-password-validation",
"Validate the certificate empty password", "",
storeSource));
assertThrows(IllegalArgumentException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "password-length-validation",
"Validate the certificate password length",
SHORT_PASSWORD, storeSource));
assertThrows(IllegalArgumentException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "invalid-password-validation",
"Validate the certificate password", INVALID_PASSWORD,
storeSource));
assertThrows(Descriptor.FormException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "invalid-certificate-validation",
"Validate the certificate keyStore", VALID_PASSWORD,
assertThrows(IllegalArgumentException.class,
() -> new CertificateCredentialsImpl(CredentialsScope.GLOBAL, "empty-keystore-validation",
"Validate the invalid certificate keyStore",
VALID_PASSWORD,
new CertificateCredentialsImpl.PEMEntryKeyStoreSource(
null, null)));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import org.htmlunit.html.HtmlRadioButtonInput;

import hudson.Util;
import hudson.model.Descriptor;
import hudson.model.ItemGroup;
import hudson.security.ACL;
import hudson.util.Secret;
Expand Down Expand Up @@ -107,7 +106,7 @@ public void setup() throws IOException {
}

@Test
public void displayName() throws IOException, Descriptor.FormException {
public void displayName() throws IOException {
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)));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDgzCCAmugAwIBAgIUdl3Su0vyakVGHSTiFvIh+nYzq0UwDQYJKoZIhvcNAQEL
MIIDUTCCAjmgAwIBAgIUM2p34T12bOzrnyga5GarEMbBI3QwDQYJKoZIhvcNAQEL
BQAwUDELMAkGA1UEBhMCSU4xCzAJBgNVBAgMAlROMREwDwYDVQQHDAhUaXJ1cHB1
cjEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMCAXDTI0MTIxMDE1
NTY0M1oYDzIxMjQxMTE2MTU1NjQzWjBQMQswCQYDVQQGEwJJTjELMAkGA1UECAwC
cjEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMCAXDTI0MTIxMjA5
NTgzM1oYDzIxMjQxMTE4MDk1ODMzWjBQMQswCQYDVQQGEwJJTjELMAkGA1UECAwC
VE4xETAPBgNVBAcMCFRpcnVwcHVyMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRz
IFB0eSBMdGQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQChhYxialRH
FZdEY1d6A0zTCi6pMnQlk/DU2xt2qV6xrI/Jkz0xIuDjCA/vD4EyEEh7aPEY5Oox
z6zztmrhF8nG2Nis4UBAxtTHqV2D5eLPfxtrRMccrIYh6121K0B45vqcHpBq/nFM
NGyKq1WJBmb4JTZ4MsZXcpe7rGBhApwgUdP/ey6jwf7oG2YCBOywZ7hu0Vs6yeSb
F84RuTuVBhVdNm2BJHMy7P2BxyDpm5RSbHQGYox68pn3kfVEdv/iC/8qwq7sbyzb
KsgvYJrpqAunTEkgLBkG/NtsSFlTzssEAtSXErfI+6b8Zd4UK6E9CoclKWyzEqMT
9mVchox4+XL/AgMBAAGjUzBRMB0GA1UdDgQWBBRcz3Oqr5FVbBP7nu5k5Pa5x1wa
VzAfBgNVHSMEGDAWgBRcz3Oqr5FVbBP7nu5k5Pa5x1waVzAPBgNVHRMBAf8EBTAD
AQH/MA0GCSqGSIb3DQEBCwUAA4IBAQB0WeoppwXfGAJwhdItbgt+M11+Px2Q6NOa
/wDVwxw59TJ7fzQdMSgdaASsxPUV6nd5itvfaAQ7dnbMst4os/GZQKLXhLr+2J7e
2/yqW9RmRoWTL9zvRc+AG4Tv8Z2/iUDSYpGCJ1CWtUJXGVPNjKq9uXoU5sMyyfJH
pGZTM1UDxd58KFVg8gszTzyUJjESjLG6KlrBDARAsRHgGPY7QJSEWYdC8pjXoj1p
7XUnjWrSmEm8xon19SCvH26yB+ByG2T0gMY4de8bECu3lbzwphA0J6mEkoNNZpQn
zRUwOldMPOuSIedVN5nSwE4TlkmzWvLsP+2JxegtEKB+jqYg9dsq
-----END CERTIFICATE-----
IFB0eSBMdGQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCoI5pI11oR
2nIvBMdViNN9RpztDYZuJ/tDl5JZz3jFHxwUFuneABMIqiySUFS3HK1Kfsz6Lvbz
qZlLriE1cKKk84GBOlkpISaizAMUyYOkZagmVrsf/QrneLQdYjNZFpOjTCduWmsC
3WLXWGA+t2qlT3jmJr+J5yyHBGme27dtab7UgoSdD2o0FFxXCG15NMkWMi+m+3zW
UK9f4wmaY1wIBd3/umsHM1dv+vwqAfgMMp/I4ISTda2sm3txmDzYGBPzvc/bnsCe
zyahE85Fi/otkqxqzDww2a7YhGY0hjcyihpqYffFw7zWo2syylLcFMFJN1nRoQyj
lshfAeLcGYLHAgMBAAGjITAfMB0GA1UdDgQWBBSSmy62eRSGGzkJnFWrcJDoU3Za
4jANBgkqhkiG9w0BAQsFAAOCAQEAW1YhiizwHdxIqf336vqk0Qn8omcg3v/GotjF
hNqFHiCnr2Y1/+7NwqsvpIpjb354atU5nxJ1X1LkIJz/yPztbVbHzqKn/RUztSV6
+wCOArAZzTfXcgOf/jgqPRQMJxhtUeU01MKcoliBLzCIdYvNe3XdkjMkPUHxK5vt
Yv5hxx+sfZ1T68xCFCgdkXtJKxkBDj9tR56vPOTN7kxpVPKFQzsI644xxID+JRbi
RFwV8GYK8rF6nVw9EQAfi+PuJj+V8DecUshMy6d7heUKWdu5i/+HgVKx46/RZMkj
gwAW4FDt8+qsdvgxYzCTkrkxbREwAtTv6xgPO2BAPZsECG/DSg==
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
-----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-----
-----BEGIN ENCRYPTED PRIVATE KEY-----
MIIFNTBfBgkqhkiG9w0BBQ0wUjAxBgkqhkiG9w0BBQwwJAQQHYC8RH2eVm05YRcn
VVJVYgICCAAwDAYIKoZIhvcNAgkFADAdBglghkgBZQMEASoEEMXZ/JFe75dATa+k
FvUVFzYEggTQsScTfFvsV/4Qc4wT/U+iEidlUm5hrVYEsKLBsa+Bk11CAK7Lsak+
gNxnYhvrs+xUll8YkFNUjxwaWGHjgrKNM+dZwO1JqKTy2VYK4LRHcY7oM6PJJszl
DMHWDLVZxMmRS0U9hpwdKA3Xt4lC5hOinlVsWR5geYcmeGjUE+SyydsD7Bypk01g
KSVhTqq1eGJq1s+nmVRlWvFLtw2ftLfH49nhhUvlpFtLO34iDjxs9MX93WJCvvqC
OzbjdoBxjaHI4a3TypjZWGTS/z4itSzRjOV7zn0LH0t30Bp0LUjS0Lxtug+i+kIo
4nFl8RM6SMQLIgYmOT4Y7bcyTq2ZkIGju1Z3I0PEsaIkE2EkIR8E8kvvsG6fXBvH
ahS2Zbga+oOuYAD7VSspgIvfIYkBriEoN3RU9QfOsk/If6JY1vHjkUQU+Yv60/ia
OajDnhOa1cks30P2DYOPBP+lpbMsmg5iUU5QEMF2oEH9afZjpbs2ZvXwo9Gh+qgQ
G7kZ2+v++AZgX5XLjXdIlpo+iXb9VQL9he0nEcbm2yCFPliwHd7S1N1pnZsyyI6U
uVE2cqFv44d/kxB67wYft6yVPsExgvEMlHDIRAMyk6AuaKmDoIYpCGaiCQnQCqBG
xgckie+vjD5zA4q5Ttdzg9inizBEwQgSDHxOh9nuZYjobDfjiDQ00A4NZVNJn/6z
JoUa0eaKEepwW0a0NflKSO98llADaJyE4jqaWAimImvp1frO/OldlqLgFXHSrhZg
l9te4Huj/NwnXAC60hq3OK9H1fEfUiFf+PYBUiLioGyJ9Yd3m5NQAxFDjPi2oBi/
+hdn1x4e+5rOmO6Y6BRec8v+ofLsnfRgLMnuyFi4uo6jbyqN1JC5z05LPsQcCPYX
lv9lD4LazeJobzOzTQGJretFjuPhGeHx+PzbulC7S3sVq7/ddlFLEmmiqFUzynBo
gpxEpPQMgB5fdVrUOsTKThsVveIQw4wVk3+uvl4AiO0yWaP9tD7Iwf2WP8SzpViU
lnb1f6bQeWtk0FxywHcdFonFIwB0qzOoO38W1tqqXBiYTcJre+bBOrprpLisxyZC
IscEJXYHjicft1PCD71Tau5muN/RB1iTXmURKYoybQ23NFlLEunD8q5kQjYAhgIx
57pcrp6dMc3qinyrSW6EK8myUywlozgEHob+azspIA+HDHz5sUNjQ0+oSBJ8VbAy
s9fo0e9ck8skPQNntgrqzSpF+qh0DXiAKKrHads0qloFcXzQ7znn4X7fKjGfE8bu
jFOd8vo9v3aZ7palgOJzYVh1IVfGGRuZXmmJR1e6qay/ZxBVNd6Kg83xRw4/9117
ByyTdYZaVsKOhgcqLqfoELyxf5lSg3ep+4x7jL5iQHf4u3u5gnjo6bd84a7W9NkG
z2mUH079NCmIHUNPPafHF+SnNOqf93VkltjafutaDhIJ7eZ4rJd6Cuhrqy7bKB4H
P/2VVcRfIV65wY5KXFYEIDprKdHrRzvMhswY5KggQ2dGGpiBOIweruqKyBd6BFDf
aQitGNUqDsw1CpulRxSAl7V+eVBPxfzzXp/JHADMAZ0qfuqNL//DaLNyElh3ozA/
RbCFEn/Lei1Poh9Eu4qiMJaTmpp8oKe3GqpzIvLrMZ+vXK7HOCpPrL4=
-----END ENCRYPTED PRIVATE KEY-----

0 comments on commit 0df0971

Please sign in to comment.