Skip to content

Commit

Permalink
Merge pull request #1590 from PereBueno/JENKINS-73460
Browse files Browse the repository at this point in the history
[JENKINS-73460] add FIPS compliance checks to plugin whem runnign in FIPS mode
  • Loading branch information
aneveux authored Aug 1, 2024
2 parents 78bd4a1 + 9a9d310 commit d919fa5
Show file tree
Hide file tree
Showing 12 changed files with 527 additions and 5 deletions.
9 changes: 4 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>authentication-tokens</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>bouncycastle-api</artifactId>
</dependency>

<dependency>
<!-- Requires Permission -->
Expand Down Expand Up @@ -173,11 +177,6 @@
<version>4.2.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>bouncycastle-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>docker-workflow</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
import java.net.SocketTimeoutException;
import java.net.URL;
import java.net.UnknownHostException;
import java.security.PublicKey;
import java.security.UnrecoverableKeyException;
import java.security.cert.Certificate;
import java.security.interfaces.DSAPublicKey;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPublicKey;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collection;
Expand All @@ -52,9 +58,11 @@
import java.util.logging.Logger;
import javax.servlet.ServletException;
import jenkins.authentication.tokens.api.AuthenticationTokens;
import jenkins.bouncycastle.api.PEMEncodable;
import jenkins.metrics.api.Metrics;
import jenkins.model.Jenkins;
import jenkins.model.JenkinsLocationConfiguration;
import jenkins.security.FIPS140;
import jenkins.util.SystemProperties;
import jenkins.websocket.WebSockets;
import net.sf.json.JSONObject;
Expand Down Expand Up @@ -268,6 +276,7 @@ public String getServerUrl() {

@DataBoundSetter
public void setServerUrl(@NonNull String serverUrl) {
ensureKubernetesUrlInFipsMode(serverUrl);
this.serverUrl = Util.fixEmpty(serverUrl);
}

Expand All @@ -277,6 +286,7 @@ public String getServerCertificate() {

@DataBoundSetter
public void setServerCertificate(String serverCertificate) {
ensureServerCertificateInFipsMode(serverCertificate);
this.serverCertificate = Util.fixEmpty(serverCertificate);
}

Expand All @@ -286,6 +296,7 @@ public boolean isSkipTlsVerify() {

@DataBoundSetter
public void setSkipTlsVerify(boolean skipTlsVerify) {
ensureSkipTlsVerifyInFipsMode(skipTlsVerify);
this.skipTlsVerify = skipTlsVerify;
}

Expand Down Expand Up @@ -651,6 +662,75 @@ public Collection<NodeProvisioner.PlannedNode> provision(
return Collections.emptyList();
}

/**
* Checks if URL is using HTTPS, required in FIPS mode
* Continues if URL is secure or not in FIPS mode, throws an {@link IllegalArgumentException} if not.
* @param url Kubernetes server URL
*/
private static void ensureKubernetesUrlInFipsMode(String url) {
if (!FIPS140.useCompliantAlgorithms() || StringUtils.isBlank(url)) {
return;
}
if (!url.startsWith("https:")) {
throw new IllegalArgumentException(Messages.KubernetesCloud_kubernetesServerUrlIsNotSecure());
}
}

/**
* Checks if TLS verification is being skipped, which is not allowed in FIPS mode
* Continues if not being skipped or not in FIPS mode, throws an {@link IllegalArgumentException} if not.
* @param skipTlsVerify value to check
*/
private static void ensureSkipTlsVerifyInFipsMode(boolean skipTlsVerify) {
if (FIPS140.useCompliantAlgorithms() && skipTlsVerify) {
throw new IllegalArgumentException(Messages.KubernetesCloud_skipTlsVerifyNotAllowedInFIPSMode());
}
}

/**
* Checks if server certificate is allowed if FIPS mode.
* Allowed certificates use a public key with the following algorithms and sizes:
* <ul>
* <li>DSA with key size >= 2048</li>
* <li>RSA with key size >= 2048</li>
* <li>Elliptic curve (ED25519) with field size >= 224</li>
* </ul>
* If certificate is valid and allowed or not in FIPS mode method will just exit.
* If not it will throw an {@link IllegalArgumentException}.
* @param serverCertificate String containing the certificate PEM.
*/
private static void ensureServerCertificateInFipsMode(String serverCertificate) {
if (!FIPS140.useCompliantAlgorithms()) {
return;
}
if (StringUtils.isBlank(serverCertificate)) {
throw new IllegalArgumentException(Messages.KubernetesCloud_serverCertificateKeyEmpty());
}
try {
PEMEncodable pem = PEMEncodable.decode(serverCertificate);
Certificate cert = pem.toCertificate();
if (cert == null) {
throw new IllegalArgumentException(Messages.KubernetesCloud_serverCertificateNotACertificate());
}
PublicKey publicKey = cert.getPublicKey();
if (publicKey instanceof RSAPublicKey) {
if (((RSAPublicKey) publicKey).getModulus().bitLength() < 2048) {
throw new IllegalArgumentException(Messages.KubernetesCloud_serverCertificateKeySize());
}
} else if (publicKey instanceof DSAPublicKey) {
if (((DSAPublicKey) publicKey).getParams().getP().bitLength() < 2048) {
throw new IllegalArgumentException(Messages.KubernetesCloud_serverCertificateKeySize());
}
} else if (publicKey instanceof ECPublicKey) {
if (((ECPublicKey) publicKey).getParams().getCurve().getField().getFieldSize() < 224) {
throw new IllegalArgumentException(Messages.KubernetesCloud_serverCertificateKeySizeEC());
}
}
} catch (RuntimeException | UnrecoverableKeyException | IOException e) {
throw new IllegalArgumentException(e.getMessage(), e);
}
}

@Override
public void replaceTemplate(PodTemplate oldTemplate, PodTemplate newTemplate) {
this.removeTemplate(oldTemplate);
Expand Down Expand Up @@ -913,6 +993,44 @@ public FormValidation doTestConnection(
}
}

@RequirePOST
@SuppressWarnings({"unused", "lgtm[jenkins/csrf]"
}) // used by jelly and already fixed jenkins security scan warning
public FormValidation doCheckSkipTlsVerify(@QueryParameter boolean skipTlsVerify) {
Jenkins.get().checkPermission(Jenkins.MANAGE);
try {
ensureSkipTlsVerifyInFipsMode(skipTlsVerify);
} catch (IllegalArgumentException ex) {
return FormValidation.error(ex, ex.getLocalizedMessage());
}
return FormValidation.ok();
}

@RequirePOST
@SuppressWarnings({"unused", "lgtm[jenkins/csrf]"
}) // used by jelly and already fixed jenkins security scan warning
public FormValidation doCheckServerCertificate(@QueryParameter String serverCertificate) {
Jenkins.get().checkPermission(Jenkins.MANAGE);
try {
ensureServerCertificateInFipsMode(serverCertificate);
} catch (IllegalArgumentException ex) {
return FormValidation.error(ex, ex.getLocalizedMessage());
}
return FormValidation.ok();
}

@RequirePOST
@SuppressWarnings("unused") // used by jelly
public FormValidation doCheckServerUrl(@QueryParameter String serverUrl) {
Jenkins.get().checkPermission(Jenkins.MANAGE);
try {
ensureKubernetesUrlInFipsMode(serverUrl);
} catch (IllegalArgumentException ex) {
return FormValidation.error(ex.getLocalizedMessage());
}
return FormValidation.ok();
}

@RequirePOST
@SuppressWarnings("unused") // used by jelly
public ListBoxModel doFillCredentialsIdItems(
Expand Down Expand Up @@ -1126,6 +1244,11 @@ private Object readResolve() {
Level.INFO, "Upgraded Kubernetes server certificate key: {0}", serverCertificate.substring(0, 80));
}

// FIPS checks if in FIPS mode
ensureServerCertificateInFipsMode(serverCertificate);
ensureKubernetesUrlInFipsMode(serverUrl);
ensureSkipTlsVerifyInFipsMode(skipTlsVerify);

if (maxRequestsPerHost == 0) {
maxRequestsPerHost = DEFAULT_MAX_REQUESTS_PER_HOST;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,10 @@ KubernetesSlave.HomeWarning=[WARNING] HOME is set to / in the agent container. Y
entry in /etc/passwd. Please add a user to your Dockerfile or set the HOME environment \
variable to a valid directory in the pod template definition.
KubernetesCloudNotAllowed.Description=Kubernetes cloud {0} is not allowed for folder containing job {1}
KubernetesCloud.skipTlsVerifyNotAllowedInFIPSMode=Skipping TLS verification is not allowed in FIPS mode
KubernetesCloud.serverCertificateIsNotApprovedInFIPSMode=Certificate is not valid: {0}
KubernetesCloud.serverCertificateKeySize=Invalid key size, at least 2048 is needed in FIPS mode.
KubernetesCloud.serverCertificateKeySizeEC=Invalid curve size, at least 224 is needed in FIPS mode.
KubernetesCloud.serverCertificateKeyEmpty=Certificate is mandatory in FIPS mode.
KubernetesCloud.serverCertificateNotACertificate=Provided PEM doesn't contain a certificate.
KubernetesCloud.kubernetesServerUrlIsNotSecure=HTTPS secure URLs are mandatory in FIPS mode.
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package org.csanchez.jenkins.plugins.kubernetes;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThrows;

import hudson.ExtensionList;
import io.jenkins.cli.shaded.org.apache.commons.io.FileUtils;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Paths;
import jenkins.security.FIPS140;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.FlagRule;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.LocalData;

public class KubernetesCloudFIPSTest {

@ClassRule
public static FlagRule<String> fipsFlag = FlagRule.systemProperty(FIPS140.class.getName() + ".COMPLIANCE", "true");

@Rule
public JenkinsRule r = new JenkinsRule();

@Test
@Issue("JENKINS-73460")
public void onlyFipsCompliantValuesAreAcceptedTest() throws IOException {
KubernetesCloud cloud = new KubernetesCloud("test-cloud");
assertThrows(IllegalArgumentException.class, () -> cloud.setSkipTlsVerify(true));
cloud.setSkipTlsVerify(false);
assertThrows(IllegalArgumentException.class, () -> cloud.setServerUrl("http://example.org"));
cloud.setServerUrl("https://example.org");
assertThrows(
"Invalid certificates throw exception",
IllegalArgumentException.class,
() -> cloud.setServerCertificate(getCert("not-a-cert")));
Throwable exception = assertThrows(
"Invalid length", IllegalArgumentException.class, () -> cloud.setServerCertificate(getCert("rsa1024")));
assertThat(exception.getLocalizedMessage(), containsString("2048"));
cloud.setServerCertificate(getCert("rsa2048"));
exception = assertThrows(
"invalid length", IllegalArgumentException.class, () -> cloud.setServerCertificate(getCert("dsa1024")));
assertThat(exception.getLocalizedMessage(), containsString("2048"));
cloud.setServerCertificate(getCert("dsa2048"));
exception = assertThrows(
"Invalid field size",
IllegalArgumentException.class,
() -> cloud.setServerCertificate(getCert("ecdsa192")));
assertThat(exception.getLocalizedMessage(), containsString("224"));
cloud.setServerCertificate(getCert("ecdsa224"));
}

@Test
@Issue("JENKINS-73460")
@LocalData
public void nonCompliantCloudsAreCleanedTest() {
assertThat("compliant-cloud is loaded", r.jenkins.getCloud("compliant-cloud"), notNullValue());
assertThat("with-skip-tls is not loaded", r.jenkins.getCloud("with-skip-tls"), nullValue());
assertThat("with-http-endpoint is not loaded", r.jenkins.getCloud("with-http-endpoint"), nullValue());
assertThat("with-invalid-cert is not loaded", r.jenkins.getCloud("with-invalid-cert"), nullValue());
}

@Test
@Issue("JENKINS-73460")
public void formValidationTest() throws IOException {
ExtensionList<KubernetesCloud.DescriptorImpl> descriptors =
ExtensionList.lookup(KubernetesCloud.DescriptorImpl.class);
KubernetesCloud.DescriptorImpl descriptor = descriptors.stream()
.filter(d -> d.getClass().isAssignableFrom(KubernetesCloud.DescriptorImpl.class))
.findFirst()
.orElseGet(KubernetesCloud.DescriptorImpl::new);
assertThat(
"Valid url doesn't raise error",
descriptor.doCheckServerUrl("https://eample.org").getMessage(),
nullValue());
assertThat(
"Invalid url raises error",
descriptor.doCheckServerUrl("http://eample.org").getMessage(),
notNullValue());
assertThat(
"Valid cert doesn't raise error",
descriptor.doCheckServerCertificate(getCert("rsa2048")).getMessage(),
nullValue());
assertThat(
"Invalid cert raises error",
descriptor.doCheckServerCertificate(getCert("rsa1024")).getMessage(),
notNullValue());
assertThat(
"No TLS skip doesn't raise error",
descriptor.doCheckSkipTlsVerify(false).getMessage(),
nullValue());
assertThat(
"TLS skip raises error", descriptor.doCheckSkipTlsVerify(true).getMessage(), notNullValue());
}

private String getCert(String alg) throws IOException {
return FileUtils.readFileToString(
Paths.get("src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudFIPSTest/certs")
.resolve(alg)
.toFile(),
Charset.defaultCharset());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIDDzCCArygAwIBAgIUYBblXc5PhKw86bwvpcfBv/PxOmEwCwYJYIZIAWUDBAMC
MEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJ
bnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjQwNzE4MTU0MDAxWhcNMjQwODE3
MTU0MDAxWjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8G
A1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBvzCCATQGByqGSM44BAEw
ggEnAoGBAN1Se68u8JFTdXrZ2ED2RDG89TRR51j1E7SPwlYItWQGAnJUkTRKyhYO
6oItvkuie/9HqjFVcz0hW1D0EfKKuJjv2VjkN07bQyZ9Kr92tfnkC2IOvbfDs8Ev
g3GBOjKs7Bi9inOcQZCKt1ZeNd4j4jDrLPnisEc8urcBlbxnNCiHAh0A7PRieQst
d6p4yVCVQp/fsjmUp+bL7phoR5jxNQKBgQC4K58rznGE8QmhSsUonv5Uf+gnnpDI
6eDiOUH/DpIDIMftK6AQ4wp6YY4pZP+dxfbBt9uimmfdyuvrO3i1oOD3UqmVSCwd
Qome8YPGfxtTYYB/o05li7KPzHTqVcGXZQont9IK+uQaCwnzv/dsERol2F2aPMnD
6JE2hd/DCUWiqQOBhAACgYBeEMMpp5ROkIDZ5h4HeDDZztn1zWRrnsV89Cs6WcjR
vbeumfVIoo06yws5tZdMfssrBjk+irKFKIU9edhiKcOjB8ssMJi+7tOEWEC9ooHo
F6cOqiYmhLBhLrIyv5dZUe8RtyJRZaP+4bn3PbxZ7Cij8DWHntnwhEjrqlUp6vCq
K6MhMB8wHQYDVR0OBBYEFPJmnQzXspNFvJNwPqxj//pvgzBlMAsGCWCGSAFlAwQD
AgNAADA9AhwNYi4S6Cq9uh9KKBYz9jYdiJYjI2lmDnSFYGtQAh0AvZkHSf6BfSKE
STBsggtQOqDvcao45reTjyaDcw==
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
-----BEGIN CERTIFICATE-----
MIIEkzCCBECgAwIBAgIUDza9+LN/FgNVUl6tr01fR+wt/U0wCwYJYIZIAWUDBAMC
MEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJ
bnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjQwNzE4MTU0MTA1WhcNMjQwODE3
MTU0MTA1WjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8G
A1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIDQzCCAjUGByqGSM44BAEw
ggIoAoIBAQC3MkKb6PbOGFDYkhMjbIbz9lREVMRGkLCSWCwa3R7KwWjE4yPl0ONq
pLGYWSKtS51y06vR+wcxgjQynKL2s1pTpdX3EcGwJtb3SdR1vLLHWoUyr5d4adtL
UNmu7SRe9pSXsw3Y6j8c1T/UlGkREHXHlJlwON13NY5ZEKkZ5vtaOsJaEL4mrxvz
/dR/joWbXNSfPdi5aeu6U2W2/zUwcOcu+PtvYafLGr5FL+fAG0UniQgF86FB//px
CVeHngm54gwLTJJPtsnKKJwa6pCXbmANzsYCCj7tpGEQH1ZcyYzNNuABm5OZCCco
Si4TlwCkVe/Rrb5Ko04Kdqg5/XWA3sn1Ah0Au8eLbfMm7x7dhJ9/Yv55DLoU1Nq8
waI6VGIdJQKCAQBZTWaquC3p30SXJ314Bh1zFPNEqcdsrNWnmZqzSKXfMuRPQRAQ
oAwMMWABtycYFPzK7/RsUihPdj1kIF46ArC+QXeo0zAQjKIcrNGIOBuvILsBx5On
F+Cw260PYIami45AqwhuACBoq6K6OxI24vSms72TMj+HRt8gtzWt1cP1MNxbyxho
rxKHUq/bLbBKfYbg4hSnvqMAw9mawEsRVYQzR+pAZqWE+otcvxkpR2PYWKR/i0dq
+PmJuqbHghMq6kss9a2d/tWQUGyUJkjr2qkjtn1CVuxpcDWcQ/0b0cpdCN83iSRA
EOJJY/g8B4kerasdPSO/e/gOfp6Pe57FI5iVA4IBBgACggEBAJnwtqFo43cc8en3
hH2lbN7A3VDNkNejkl3kdhohHjgXNS2Cwlfn3y1QtYrw1BeG1Sdh4WTFCLIePc6t
3quwLhRLDGzgD7YlvE4WYoE5JidAM+qcQiwdbXOhSljCeJFCdkg+7SH0LWwwW555
I3K64XeE0/ONn9bjkhYPfDv5HG2fiEDL1bc6sgoraN6Cb0p2MmXQiSz+FMdHTLlg
yNrIdkNQxxKlYHJzNqwmesaSy9SmA/woGyrIrmX/XCExJwpasnaL+c/efU8H4zOC
5eO8JUqUiWISTZn47YPrd/zFEDrk9heGgi5ZabUneAS69DaYa+sC/Xxa/ZCIMt0A
9tN26gujITAfMB0GA1UdDgQWBBTDOjTyqpCiM996xYmndSV3QcGy+TALBglghkgB
ZQMEAwIDQAAwPQIdAJGqVySKZk1O4SQHsWAzTI+706myofPowzs1hvsCHCkZmLGG
ZrWsuaK1nMVltEQER+Eodz9oLRDgqew=
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-----BEGIN CERTIFICATE-----
MIIBwDCCAXagAwIBAgIUOwG28vjFm4KoiGieHrBRgzhrzGAwCgYIKoZIzj0EAwIw
RTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGElu
dGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yNDA3MTgxNTQ4MzBaFw0yNDA4MTcx
NTQ4MzBaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYD
VQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwSjAUBgcqhkjOPQIBBgkrJAMD
AggBAQQDMgAEpFiVE3YkcIaJVP9DsLIZE620gyX23AxQahhWjywp8hp+DKO4voH3
HlKfdeDEZ5nfo1MwUTAdBgNVHQ4EFgQUCQxoboqlb8uG3RrOqtk4Dxil4xwwHwYD
VR0jBBgwFoAUCQxoboqlb8uG3RrOqtk4Dxil4xwwDwYDVR0TAQH/BAUwAwEB/zAK
BggqhkjOPQQDAgM4ADA1AhkAiI0732BOdYpjG1EgZ2y1Y1W9qLjgKLH7AhgwSQbA
qPWq3wYiP1gZsVMavRL9K1ggspE=
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-----BEGIN CERTIFICATE-----
MIIBzzCCAX6gAwIBAgIUMxqDFkKRXOeP325owDz02IZomgUwCgYIKoZIzj0EAwIw
RTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGElu
dGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yNDA3MTgxNTQ5MDVaFw0yNDA4MTcx
NTQ5MDVaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYD
VQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwUjAUBgcqhkjOPQIBBgkrJAMD
AggBAQYDOgAEOuIfMAfhqilO6Q1VxiAjuQnTkpFH2MYcyFjyG9O2OG71KFuB4hC8
r6NSSxVCx88TjKzcnm/u/HijUzBRMB0GA1UdDgQWBBQtPSBGTPqBRJQOVhf/c8Xh
5s0aOjAfBgNVHSMEGDAWgBQtPSBGTPqBRJQOVhf/c8Xh5s0aOjAPBgNVHRMBAf8E
BTADAQH/MAoGCCqGSM49BAMCAz8AMDwCHAOWGI94ia/Ck3JgqIPFCGZUqR8uh9vC
ovacsJACHC8VSwu0hEqevytqT7HH9E/DCMYORANJBZz5GyY=
-----END CERTIFICATE-----
Loading

0 comments on commit d919fa5

Please sign in to comment.