Skip to content

Commit

Permalink
In FIPS env, escapeHatch cannot be enabled
Browse files Browse the repository at this point in the history
Signed-off-by: Olivier Lamy <[email protected]>
  • Loading branch information
olamy committed Oct 10, 2024
1 parent 4f73328 commit ea9366c
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 32 deletions.
19 changes: 18 additions & 1 deletion src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Check warning on line 346 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 345-346 are not covered by tests

Check warning on line 346 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L345-L346

Added lines #L345 - L346 were not covered by tests
}

try {
if (automanualconfigure != null) {
if ("auto".equals(automanualconfigure)) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1387,5 +1399,10 @@ private FormValidation doCheckFieldName(String fieldName, FormValidation validIf
public Descriptor<OicServerConfiguration> getDefaultServerConfigurationType() {
return Jenkins.get().getDescriptor(OicServerWellKnownConfiguration.class);
}

@Restricted(NoExternalUse.class) // used by jelly only
public boolean isFipsEnabled() {
return FIPS140.useCompliantAlgorithms();

Check warning on line 1405 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1405 is not covered by tests

Check warning on line 1405 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L1405

Added line #L1405 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,22 +66,23 @@
<f:checkbox/>
</f:entry>
</f:advanced>

<f:block>
<table>
<f:optionalBlock inline="true" title="${%ConfigureEscapeHatch}" field="escapeHatchEnabled">
<f:entry title="${%Username}" field="escapeHatchUsername">
<f:textbox/>
</f:entry>
<f:entry title="${%Secret}" field="escapeHatchSecret">
<f:password/>
</f:entry>
<f:entry title="${%Group}" field="escapeHatchGroup">
<f:textbox/>
</f:entry>
</f:optionalBlock>
</table>
</f:block>
<j:if test="${!descriptor.isFipsEnabled()}">
<f:block>
<table>
<f:optionalBlock inline="true" title="${%ConfigureEscapeHatch}" field="escapeHatchEnabled">
<f:entry title="${%Username}" field="escapeHatchUsername">
<f:textbox/>
</f:entry>
<f:entry title="${%Secret}" field="escapeHatchSecret">
<f:password/>
</f:entry>
<f:entry title="${%Group}" field="escapeHatchGroup">
<f:textbox/>
</f:entry>
</f:optionalBlock>
</table>
</f:block>
</j:if>

</f:section>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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";

Expand All @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private void browseLoginPage() throws IOException, SAXException {
webClient.goTo(jenkins.getSecurityRealm().getLoginUrl());
}

private void configureTestRealm(@NonNull Consumer<OicSecurityRealm> consumer) throws IOException {
private void configureTestRealm(@NonNull Consumer<OicSecurityRealm> consumer) throws Exception {
var securityRealm = new TestRealm(wireMockRule);
consumer.accept(securityRealm);
jenkins.setSecurityRealm(securityRealm);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.jenkinsci.plugins.oic;

Check warning on line 1 in src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java

View check run for this annotation

ci.jenkins.io / Java Compiler

checkstyle:check

ERROR: (misc) NewlineAtEndOfFile: Expected line ending for file is LF(\n), but CRLF(\r\n) is detected.

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<String> 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));
}
}
14 changes: 7 additions & 7 deletions src/test/java/org/jenkinsci/plugins/oic/TestRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -177,7 +177,7 @@ public OicServerConfiguration buildServerConfiguration() {
}
}

public TestRealm(Builder builder) throws IOException {
public TestRealm(Builder builder) throws Exception {
super(
builder.clientId,
builder.clientSecret,
Expand All @@ -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));
}

Expand All @@ -224,7 +224,7 @@ public TestRealm(
String emailFieldName,
String groupFieldName,
boolean automanualconfigure)
throws IOException {
throws Exception {
this(new Builder(wireMockRule)
.WithMinimalDefaults()
.WithUserInfoServerUrl(userInfoServerUrl)
Expand All @@ -243,7 +243,7 @@ public TestRealm(
String escapeHatchUsername,
String escapeHatchSecret,
String escapeHatchGroup)
throws IOException {
throws Exception {
this(new Builder(wireMockRule)
.WithMinimalDefaults()
.WithUserInfoServerUrl(userInfoServerUrl)
Expand Down

0 comments on commit ea9366c

Please sign in to comment.