Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-73525] add FIPS compliance checks to plugin when running in FIPS mode #50

Merged
merged 8 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>

<!-- jenkins versions -->
<jenkins.version>2.401.3</jenkins.version>
<bom.artifactId>bom-2.401.x</bom.artifactId>
<bom.version>2745.vc7b_fe4c876fa_</bom.version>
<jenkins.baseline>2.426</jenkins.baseline>
<jenkins.version>${jenkins.baseline}.3</jenkins.version>
<bom.artifactId>bom-${jenkins.baseline}.x</bom.artifactId>
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved
<bom.version>3208.vb_21177d4b_cd9</bom.version>

<!-- maven plugins versions -->
<maven-coveralls.version>4.3.0</maven-coveralls.version>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.jenkinsci.plugins.kubernetes.auth;

import jenkins.security.FIPS140;

/**
* Configuration object for {@link KubernetesAuth} operations.
*/
Expand All @@ -18,6 +20,9 @@ public class KubernetesAuthConfig {
private final boolean skipTlsVerify;

public KubernetesAuthConfig(String serverUrl, String caCertificate, boolean skipTlsVerify) {
if (FIPS140.useCompliantAlgorithms() && skipTlsVerify && serverUrl.startsWith("https://")) {
jmdesprez marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException("Skipping TLS verification is not accepted in FIPS mode.");
jmdesprez marked this conversation as resolved.
Show resolved Hide resolved
}
this.serverUrl = serverUrl;
this.caCertificate = caCertificate;
this.skipTlsVerify = skipTlsVerify;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jenkinsci.plugins.kubernetes.credentials;

import jenkins.security.FIPS140;
import org.apache.commons.codec.binary.Base64InputStream;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
Expand Down Expand Up @@ -83,6 +84,9 @@ public static HttpClientBuilder getBuilder(URI uri, String caCertData, boolean s

try {
if (skipTLSVerify) {
if (FIPS140.useCompliantAlgorithms() && uri.getScheme().equals("https")) {
jmdesprez marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException("Skipping TLS verification is not accepted in FIPS mode.");
}
builder.setSSLSocketFactory(getAlwaysTrustSSLFactory());
} else if (caCertData != null) {
builder.setSSLSocketFactory(getVerifyCertSSLFactory(uri.getHost(), caCertData));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import net.sf.json.JSONObject;
import org.apache.commons.codec.binary.Base64;
import org.apache.http.Header;
import org.apache.http.HttpHeaders;
import org.apache.http.NameValuePair;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.methods.CloseableHttpResponse;
Expand Down Expand Up @@ -135,7 +136,9 @@ private synchronized Token refreshToken(String apiServerURL, String caCertData,

final HttpClientBuilder builder = HttpClientWithTLSOptionsFactory.getBuilder(uri, caCertData, skipTLSVerify);
HttpGet authorize = new HttpGet(oauthServerURL + "?client_id=openshift-challenging-client&response_type=token");
authorize.setHeader("Authorization", getBasicAuthenticationHeader(getUsername(), getPassword()));
authorize.setHeader(HttpHeaders.AUTHORIZATION, getBasicAuthenticationHeader(getUsername(), getPassword()));

Utils.ensureFIPSCompliantURIRequest(authorize, skipTLSVerify);
final CloseableHttpResponse response = builder.build().execute(authorize);

if (response.getStatusLine().getStatusCode() != 302) {
Expand All @@ -154,6 +157,7 @@ private String getOauthServerUrl(String apiServerURL, String caCertData, boolean
URI uri = new URI(apiServerURL);
final HttpClientBuilder builder = HttpClientWithTLSOptionsFactory.getBuilder(uri, caCertData, skipTLSVerify);
HttpGet discover = new HttpGet(apiServerURL + "/.well-known/oauth-authorization-server");
Utils.ensureFIPSCompliantURIRequest(discover, skipTLSVerify);
final CloseableHttpResponse response = builder.build().execute(discover);
return JSONObject.fromObject(EntityUtils.toString(response.getEntity())).getString("authorization_endpoint");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.jenkinsci.plugins.kubernetes.credentials;

import jenkins.security.FIPS140;
import org.apache.commons.codec.binary.Base64;
import org.apache.http.HttpHeaders;
import org.apache.http.client.methods.HttpUriRequest;

import java.nio.charset.StandardCharsets;
import java.security.Key;
Expand Down Expand Up @@ -46,4 +49,27 @@ public static String encodeCertificate(Certificate certificate) throws Certifica
public static String encodeKey(Key key) {
return encodeBase64(wrapPrivateKey(Base64.encodeBase64String(key.getEncoded())));
}

/**
* Ensure that the URI request is FIPS compliant for the given HttpUriRequest object and skipTLSVerify option.
* Throw an exception if the request is invalid.
* A request is considered valid if there is no potential leak of credentials (setting a credentials without using TLS) and
* if the TLS verification is not skipped.
* If FIPS mode is not enabled, this method does nothing.
*
* @param uriRequest The request to validate
* @param skipTLSVerify A flag indicating whether to skip TLS verification or not
* @throws IllegalArgumentException If the request is invalid
*/
public static void ensureFIPSCompliantURIRequest(HttpUriRequest uriRequest, boolean skipTLSVerify) {
if (FIPS140.useCompliantAlgorithms()) {
boolean isHttps = uriRequest.getURI().getScheme().equals("https");
if (!isHttps && uriRequest.containsHeader(HttpHeaders.AUTHORIZATION)) {
throw new IllegalArgumentException("Non-TLS connection is not accepted in FIPS mode when a credential is present.");
}
if (isHttps && skipTLSVerify) {
throw new IllegalArgumentException("Skipping TLS verification is not accepted in FIPS mode.");
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.jenkinsci.plugins.kubernetes.auth;

import static org.junit.Assert.fail;

import org.junit.Test;

public abstract class AbstractKubernetesAuthConfigFIPSTest {
protected String scheme;

protected boolean skipTLSVerify;

protected boolean shouldPass;

protected String motivation;

public AbstractKubernetesAuthConfigFIPSTest(
String scheme, boolean skipTLSVerify, boolean shouldPass, String motivation) {
this.scheme = scheme;
this.skipTLSVerify = skipTLSVerify;
this.shouldPass = shouldPass;
this.motivation = motivation;
}

@Test
public void testCreateKubernetesAuthConfig() {
try {
new KubernetesAuthConfig(scheme + "://server", null, skipTLSVerify);
if (!shouldPass) {
fail("This test was expected to fail, reason: " + motivation);
}
} catch (IllegalArgumentException e) {
if (shouldPass) {
fail("This test was expected to pass, reason: " + motivation);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.jenkinsci.plugins.kubernetes.auth;

import java.util.Arrays;
import java.util.Collection;
import jenkins.security.FIPS140;
import org.junit.ClassRule;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.jvnet.hudson.test.FlagRule;

@RunWith(Parameterized.class)
public class KubernetesAuthConfigWithFIPSTest extends AbstractKubernetesAuthConfigFIPSTest {
@ClassRule
public static FlagRule<String> fipsFlag = FlagRule.systemProperty(FIPS140.class.getName() + ".COMPLIANCE", "true");

public KubernetesAuthConfigWithFIPSTest(
String scheme, boolean skipTLSVerify, boolean shouldPass, String motivation) {
super(scheme, skipTLSVerify, shouldPass, motivation);
}

@Parameterized.Parameters
public static Collection<Object[]> parameters() {
return Arrays.asList(new Object[][] {
// Valid use cases
{"https", false, true, "TLS is used and the TLS verification is not skipped, this should be accepted"},
{"http", false, true, "Non-TLS configuration should be accepted"},
{"http", true, true, "Non-TLS configuration should be accepted"},
// Invalid use cases
{"https", true, false, "Skip TLS check is not accepted in FIPS mode"},
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.jenkinsci.plugins.kubernetes.auth;

import java.util.Arrays;
import java.util.Collection;
import jenkins.security.FIPS140;
import org.junit.ClassRule;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.jvnet.hudson.test.FlagRule;

@RunWith(Parameterized.class)
public class KubernetesAuthConfigWithoutFIPSTest extends AbstractKubernetesAuthConfigFIPSTest {
@ClassRule
public static FlagRule<String> fipsFlag = FlagRule.systemProperty(FIPS140.class.getName() + ".COMPLIANCE", "false");

public KubernetesAuthConfigWithoutFIPSTest(
String scheme, boolean skipTLSVerify, boolean shouldPass, String motivation) {
super(scheme, skipTLSVerify, shouldPass, motivation);
}

@Parameterized.Parameters
public static Collection<Object[]> parameters() {
return Arrays.asList(new Object[][] {
// Valid use cases
{"https", false, true, "Not in FIPS mode, any combination should be valid"},
{"http", false, true, "Not in FIPS mode, any combination should be valid"},
{"http", true, true, "Not in FIPS mode, any combination should be valid"},
{"https", true, true, "Not in FIPS mode, any combination should be valid"},
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.jenkinsci.plugins.kubernetes.credentials;

import static org.junit.Assert.fail;

import java.net.URI;
import java.net.URISyntaxException;
import org.junit.Test;

public abstract class AbstractHttpClientWithTLSOptionsFactoryFIPSTest {
protected String scheme;

protected boolean skipTLSVerify;

protected boolean shouldPass;

protected String motivation;

public AbstractHttpClientWithTLSOptionsFactoryFIPSTest(String scheme, boolean skipTLSVerify, boolean shouldPass, String motivation) {
this.scheme = scheme;
this.skipTLSVerify = skipTLSVerify;
this.shouldPass = shouldPass;
this.motivation = motivation;
}

@Test
public void testCreateKubernetesAuthConfig() throws URISyntaxException {
try {
HttpClientWithTLSOptionsFactory.getBuilder(new URI(scheme, "localhost", null, null), null, skipTLSVerify);
if (!shouldPass) {
fail("This test was expected to fail, reason: " + motivation);
}
} catch (HttpClientWithTLSOptionsFactory.TLSConfigurationError e) {
if (shouldPass) {
fail("This test was expected to pass, reason: " + motivation);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package org.jenkinsci.plugins.kubernetes.credentials;

import com.cloudbees.plugins.credentials.CredentialsScope;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.After;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.jvnet.hudson.test.FlagRule;

import java.io.IOException;
import java.net.URL;

import static org.junit.Assert.fail;

public abstract class AbstractOpenShiftBearerTokenCredentialFIPSTest {
private static final URL keystore;

@ClassRule
public static FlagRule<String> truststoreFlag;

@ClassRule
public static FlagRule<String> truststorePasswordFlag;

static {
keystore = AbstractOpenShiftBearerTokenCredentialFIPSTest.class.getResource("keystore.jks");
if (keystore == null) {
fail("Unable to find keystore.jks");
}
truststoreFlag = FlagRule.systemProperty("javax.net.ssl.trustStore", keystore.getPath());
truststorePasswordFlag = FlagRule.systemProperty("javax.net.ssl.trustStorePassword", "unittest");
}

protected String scheme;

protected boolean skipTLSVerify;

protected boolean shouldPass;

protected String motivation;

private Server server;

private ServerConnector sslConnector;

private ServerConnector serverConnector;


public AbstractOpenShiftBearerTokenCredentialFIPSTest(
String scheme, boolean skipTLSVerify, boolean shouldPass, String motivation) {
this.scheme = scheme;
this.skipTLSVerify = skipTLSVerify;
this.shouldPass = shouldPass;
this.motivation = motivation;
}

@Before
public void prepareFakeOAuthServer() throws Exception {
if (keystore == null) {
fail("Unable to find keystore.jks");
}
server = new Server();

HttpConfiguration httpsConfig = new HttpConfiguration();
httpsConfig.addCustomizer(new SecureRequestCustomizer());

SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
sslContextFactory.setKeyStorePath(keystore.toExternalForm());
sslContextFactory.setKeyManagerPassword("unittest");
sslContextFactory.setKeyStorePassword("unittest");

sslConnector = new ServerConnector(server, new SslConnectionFactory(sslContextFactory, "http/1.1"), new HttpConnectionFactory(httpsConfig));
serverConnector = new ServerConnector(server);
server.setConnectors(new Connector[]{serverConnector, sslConnector});

ServletContextHandler context = new ServletContextHandler();
context.setContextPath("/");
context.addServlet(new ServletHolder(new MockHttpServlet()), "/*");
server.setHandler(context);
server.start();
}

@After
public void unprepareFakeOAuthServer() throws Exception {
server.stop();
}

@Test
public void ensureFIPSCompliantURIRequest() throws IOException {
OpenShiftBearerTokenCredentialImpl cred;
cred = new OpenShiftBearerTokenCredentialImpl(CredentialsScope.GLOBAL, "id", "description", "username", "password");
try {
int port;
if ("https".equals(scheme)) {
port = sslConnector.getLocalPort();
} else {
port = serverConnector.getLocalPort();
}
cred.getToken(scheme + "://localhost:" + port + "/valid-response", null, skipTLSVerify);
if (!shouldPass) {
fail("This test was expected to fail, reason: " + motivation);
}
} catch (IOException e) {
// Because of how the code is done, the IllegalArgumentException can be wrapped into multiple Exception
// Search if one of the causes is an IllegalArgumentException
boolean legitException = false;
Throwable cause = e.getCause();
while (cause != null) {
if (cause instanceof IllegalArgumentException) {
if (shouldPass) {
fail("This test was expected to pass, reason: " + motivation);
}
// The IllegalArgumentException was expected, exit the loop now
legitException = true;
break;
}
cause = cause.getCause();
}
if(!legitException) {
throw e;
}
} catch (IllegalArgumentException e) {
if (shouldPass) {
fail("This test was expected to pass, reason: " + motivation);
}
}
}
}
Loading