From ea9366cb2d837abd513d0a0fb6011b2e01a09648 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Tue, 8 Oct 2024 10:48:34 +1000 Subject: [PATCH 1/3] In FIPS env, escapeHatch cannot be enabled Signed-off-by: Olivier Lamy --- .../plugins/oic/OicSecurityRealm.java | 19 ++++++++++- .../plugins/oic/OicSecurityRealm/config.jelly | 33 ++++++++++--------- .../plugins/oic/OicSecurityRealmTest.java | 13 ++++---- .../org/jenkinsci/plugins/oic/PluginTest.java | 2 +- .../SecurityRealmConfigurationFIPSTest.java | 27 +++++++++++++++ .../org/jenkinsci/plugins/oic/TestRealm.java | 14 ++++---- 6 files changed, 76 insertions(+), 32 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 76f98b36..884f3dfb 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -86,6 +86,7 @@ import javax.servlet.http.HttpSession; import jenkins.model.Jenkins; import jenkins.security.ApiTokenProperty; +import jenkins.security.FIPS140; import jenkins.security.SecurityListener; import jenkins.util.SystemProperties; import org.apache.commons.lang.StringUtils; @@ -337,6 +338,14 @@ protected Object readResolve() throws ObjectStreamException { this.setTokenFieldToCheckKey(this.tokenFieldToCheckKey); // ensure escapeHatchSecret is encrypted this.setEscapeHatchSecret(this.escapeHatchSecret); + + // validate this option in FIPS env or not + try { + this.setEscapeHatchEnabled(this.escapeHatchEnabled); + } catch (FormException e) { + throw new IllegalArgumentException(e); + } + try { if (automanualconfigure != null) { if ("auto".equals(automanualconfigure)) { @@ -592,7 +601,10 @@ public void setPostLogoutRedirectUrl(String postLogoutRedirectUrl) { } @DataBoundSetter - public void setEscapeHatchEnabled(boolean escapeHatchEnabled) { + public void setEscapeHatchEnabled(boolean escapeHatchEnabled) throws FormException { + if (FIPS140.useCompliantAlgorithms() && escapeHatchEnabled) { + throw new FormException("Escape Hatch cannot be enabled in FIPS environment", "escapeHatchEnabled"); + } this.escapeHatchEnabled = escapeHatchEnabled; } @@ -1387,5 +1399,10 @@ private FormValidation doCheckFieldName(String fieldName, FormValidation validIf public Descriptor getDefaultServerConfigurationType() { return Jenkins.get().getDescriptor(OicServerWellKnownConfiguration.class); } + + @Restricted(NoExternalUse.class) // used by jelly only + public boolean isFipsEnabled() { + return FIPS140.useCompliantAlgorithms(); + } } } diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly index 42de2c18..a9f302f0 100644 --- a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly @@ -66,22 +66,23 @@ - - - - - - - - - - - - - - -
- + + + + + + + + + + + + + + +
+ + diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java index 98d8658d..32d859cc 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java @@ -3,7 +3,6 @@ import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import com.github.tomakehurst.wiremock.junit.WireMockRule; import hudson.util.Secret; -import java.io.IOException; import org.acegisecurity.AuthenticationManager; import org.acegisecurity.BadCredentialsException; import org.acegisecurity.GrantedAuthority; @@ -65,20 +64,20 @@ public void testAuthenticate_withUsernamePasswordAuthenticationToken() throws Ex } @Test - public void testGetAuthenticationGatewayUrl() throws IOException { + public void testGetAuthenticationGatewayUrl() throws Exception { TestRealm realm = new TestRealm(wireMockRule); assertEquals("securityRealm/escapeHatch", realm.getAuthenticationGatewayUrl()); } @Test - public void testShouldSetNullClientSecretWhenSecretIsNull() throws IOException { + public void testShouldSetNullClientSecretWhenSecretIsNull() throws Exception { TestRealm realm = new TestRealm.Builder(wireMockRule) .WithMinimalDefaults().WithClient("id without secret", null).build(); assertEquals("none", Secret.toString(realm.getClientSecret())); } @Test - public void testGetValidRedirectUrl() throws IOException { + public void testGetValidRedirectUrl() throws Exception { // root url is http://localhost:????/jenkins/ final String rootUrl = jenkinsRule.jenkins.getRootUrl(); @@ -95,7 +94,7 @@ public void testGetValidRedirectUrl() throws IOException { } @Test - public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws IOException { + public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws Exception { // root url is http://localhost:????/jenkins/ String rootUrl = jenkinsRule.jenkins.getRootUrl(); @@ -110,7 +109,7 @@ public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws IOException } @Test - public void testShouldCheckEscapeHatchWithPlainPassword() throws IOException { + public void testShouldCheckEscapeHatchWithPlainPassword() throws Exception { final String escapeHatchUsername = "aUsername"; final String escapeHatchPassword = "aSecretPassword"; @@ -127,7 +126,7 @@ public void testShouldCheckEscapeHatchWithPlainPassword() throws IOException { } @Test - public void testShouldCheckEscapeHatchWithHashedPassword() throws IOException { + public void testShouldCheckEscapeHatchWithHashedPassword() throws Exception { final String escapeHatchUsername = "aUsername"; final String escapeHatchPassword = "aSecretPassword"; final String escapeHatchCryptedPassword = BCrypt.hashpw(escapeHatchPassword, BCrypt.gensalt()); diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index ed10c18c..2a6aeb99 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -177,7 +177,7 @@ private void browseLoginPage() throws IOException, SAXException { webClient.goTo(jenkins.getSecurityRealm().getLoginUrl()); } - private void configureTestRealm(@NonNull Consumer consumer) throws IOException { + private void configureTestRealm(@NonNull Consumer consumer) throws Exception { var securityRealm = new TestRealm(wireMockRule); consumer.accept(securityRealm); jenkins.setSecurityRealm(securityRealm); diff --git a/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java new file mode 100644 index 00000000..1f011cf0 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java @@ -0,0 +1,27 @@ +package org.jenkinsci.plugins.oic; + +import hudson.model.Descriptor; +import org.junit.ClassRule; +import org.junit.Test; +import org.jvnet.hudson.test.FlagRule; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class SecurityRealmConfigurationFIPSTest { + + @ClassRule + public static FlagRule FIPS_RULE = FlagRule.systemProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); + + @Test(expected = Descriptor.FormException.class) + public void escapeHatchThrowsException() throws Exception { + new OicSecurityRealm("clientId", null, null, null).setEscapeHatchEnabled(true); + } + + @Test + public void escapeHatchToFalse() throws Exception { + OicSecurityRealm oicSecurityRealm = new OicSecurityRealm("clientId", null, null, null); + oicSecurityRealm.setEscapeHatchEnabled(false); + assertThat(oicSecurityRealm.isEscapeHatchEnabled(), is(false)); + } +} diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java index fca8f795..eec1d58e 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java +++ b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java @@ -147,7 +147,7 @@ public Builder WithDisableSslVerification(boolean disableSslVerification) { return this; } - public TestRealm build() throws IOException { + public TestRealm build() throws Exception { return new TestRealm(this); } @@ -177,7 +177,7 @@ public OicServerConfiguration buildServerConfiguration() { } } - public TestRealm(Builder builder) throws IOException { + public TestRealm(Builder builder) throws Exception { super( builder.clientId, builder.clientSecret, @@ -203,18 +203,18 @@ public TestRealm(Builder builder) throws IOException { } public TestRealm(WireMockRule wireMockRule, String userInfoServerUrl, String emailFieldName, String groupsFieldName) - throws IOException { + throws Exception { this(new Builder(wireMockRule) .WithUserInfoServerUrl(userInfoServerUrl) .WithEmailFieldName(emailFieldName) .WithGroupsFieldName(groupsFieldName)); } - public TestRealm(WireMockRule wireMockRule) throws IOException { + public TestRealm(WireMockRule wireMockRule) throws Exception { this(new Builder(wireMockRule).WithMinimalDefaults()); } - public TestRealm(WireMockRule wireMockRule, String userInfoServerUrl) throws IOException { + public TestRealm(WireMockRule wireMockRule, String userInfoServerUrl) throws Exception { this(new Builder(wireMockRule).WithMinimalDefaults().WithUserInfoServerUrl(userInfoServerUrl)); } @@ -224,7 +224,7 @@ public TestRealm( String emailFieldName, String groupFieldName, boolean automanualconfigure) - throws IOException { + throws Exception { this(new Builder(wireMockRule) .WithMinimalDefaults() .WithUserInfoServerUrl(userInfoServerUrl) @@ -243,7 +243,7 @@ public TestRealm( String escapeHatchUsername, String escapeHatchSecret, String escapeHatchGroup) - throws IOException { + throws Exception { this(new Builder(wireMockRule) .WithMinimalDefaults() .WithUserInfoServerUrl(userInfoServerUrl) From 0a76e5d67321d81eab0621ace99857defc663d36 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Fri, 11 Oct 2024 09:44:44 +1000 Subject: [PATCH 2/3] try to make codecov happy Signed-off-by: Olivier Lamy --- .../plugins/oic/SecurityRealmConfigurationFIPSTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java index 1f011cf0..9d7c613e 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java @@ -24,4 +24,12 @@ public void escapeHatchToFalse() throws Exception { oicSecurityRealm.setEscapeHatchEnabled(false); assertThat(oicSecurityRealm.isEscapeHatchEnabled(), is(false)); } + + @Test + public void readresolve() throws Exception { + OicSecurityRealm oicSecurityRealm = new OicSecurityRealm("clientId", null, null, null); + oicSecurityRealm.setEscapeHatchEnabled(false); + assertThat(oicSecurityRealm.isEscapeHatchEnabled(), is(false)); + oicSecurityRealm.readResolve(); + } } From ab4ab83950f5684c5ced3fa52ea43f7dae50e150 Mon Sep 17 00:00:00 2001 From: James Nord Date: Fri, 11 Oct 2024 15:59:53 +0100 Subject: [PATCH 3/3] keep the form name in the exception --- src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 188613ab..761905bd 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -357,7 +357,7 @@ protected Object readResolve() throws ObjectStreamException { try { this.setEscapeHatchEnabled(this.escapeHatchEnabled); } catch (FormException e) { - throw new IllegalArgumentException(e); + throw new IllegalArgumentException(e.getFormField() + ": " + e.getMessage()); } try {