Hash escape hatch password in configuration

jenkinsci · Apr 3, 2024 · f736bfe · f736bfe
1 parent 0c3e868
commit f736bfe
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 6 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
@@ -82,6 +82,7 @@
 import java.util.Random;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 import javax.servlet.ServletException;
 import jenkins.model.Jenkins;
 import jenkins.security.SecurityListener;
@@ -109,6 +110,7 @@
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
+import org.springframework.security.crypto.bcrypt.BCrypt;
 
 import static org.apache.commons.lang.StringUtils.isNotBlank;
 
@@ -254,7 +256,7 @@ public OicSecurityRealm(String clientId, String clientSecret, String wellKnownOp
         this.postLogoutRedirectUrl = postLogoutRedirectUrl;
         this.escapeHatchEnabled = Util.fixNull(escapeHatchEnabled, Boolean.FALSE);
         this.escapeHatchUsername = Util.fixEmpty(escapeHatchUsername);
-        this.escapeHatchSecret = Secret.fromString(escapeHatchSecret);
+        this.setEscapeHatchSecret(Secret.fromString(escapeHatchSecret));
         this.escapeHatchGroup = Util.fixEmpty(escapeHatchGroup);
     }
 
@@ -309,6 +311,8 @@ protected Object readResolve() {
         this.setEmailFieldName(this.emailFieldName);
         this.setFullNameFieldName(this.fullNameFieldName);
         this.setTokenFieldToCheckKey(this.tokenFieldToCheckKey);
+        // ensure escapeHatchSecret is encrypted
+        this.setEscapeHatchSecret(this.escapeHatchSecret);
         return this;
     }
 
@@ -627,9 +631,25 @@ public void setEscapeHatchUsername(String escapeHatchUsername) {
 
     @DataBoundSetter
     public void setEscapeHatchSecret(Secret escapeHatchSecret) {
+        if (escapeHatchSecret != null) {
+            // ensure escapeHatchSecret is BCrypt hash
+            String escapeHatchString = Secret.toString(escapeHatchSecret);
+
+            final Pattern BCryptPattern = Pattern.compile("\\A\\$[^$]+\\$\\d+\\$[./0-9A-Za-z]{53}");
+            if (!BCryptPattern.matcher(escapeHatchString).matches()) {
+                this.escapeHatchSecret = Secret.fromString(BCrypt.hashpw(escapeHatchString, BCrypt.gensalt()));
+                return;
+            }
+        }
         this.escapeHatchSecret = escapeHatchSecret;
     }
 
+    protected boolean checkEscapeHatch(String username, String password) {
+        final boolean isUsernameMatch = username.equals(this.escapeHatchUsername);
+        final boolean isPasswordMatch = BCrypt.checkpw(password, Secret.toString(this.escapeHatchSecret));
+        return isUsernameMatch & isPasswordMatch;
+    }
+
     @DataBoundSetter
     public void setEscapeHatchGroup(String escapeHatchGroup) {
         this.escapeHatchGroup = Util.fixEmpty(escapeHatchGroup);
@@ -702,8 +722,8 @@ public Authentication authenticate(Authentication authentication) throws Authent
 
                         if (authentication instanceof UsernamePasswordAuthenticationToken && escapeHatchEnabled) {
                             randomWait(); // to slowdown brute forcing
-                            if ( authentication.getPrincipal().toString().equals(escapeHatchUsername) &&
-                                authentication.getCredentials().toString().equals(Secret.toString(escapeHatchSecret))) {
+                            if (checkEscapeHatch(authentication.getPrincipal().toString(),
+                                                 authentication.getCredentials().toString())) {
                                     List<GrantedAuthority> grantedAuthorities = new ArrayList<>();
                                     grantedAuthorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2);
                                     if (isNotBlank(escapeHatchGroup)) {

diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly
@@ -89,7 +89,7 @@
     <f:entry title="${%Post logout redirect URL}" field="postLogoutRedirectUrl">
       <f:textbox/>
     </f:entry>
-    <f:entry title="${%Enable Proof Key for Code Exchang (PKCE)}" field="pkceEnabled">
+    <f:entry title="${%Enable Proof Key for Code Exchange (PKCE)}" field="pkceEnabled">
       <f:checkbox/>
     </f:entry>
     <f:entry title="${%Disable Nonce verification}" field="nonceDisabled">

diff --git a/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java b/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java
@@ -51,7 +51,7 @@ public void testConfig() {
         assertEquals("emailFieldName", oicSecurityRealm.getEmailFieldName());
         assertTrue(oicSecurityRealm.isEscapeHatchEnabled());
         assertEquals("escapeHatchGroup", oicSecurityRealm.getEscapeHatchGroup());
-        assertEquals("escapeHatchSecret", Secret.toString(oicSecurityRealm.getEscapeHatchSecret()));
+        assertEquals("$2a$10$fxteEkfDqwqkmUelZmTxlu9WESjVDKQhp6jsqB1AgsLQ2dC6jikga", Secret.toString(oicSecurityRealm.getEscapeHatchSecret()));
         assertEquals("escapeHatchUsername", oicSecurityRealm.getEscapeHatchUsername());
         assertEquals("fullNameFieldName", oicSecurityRealm.getFullNameFieldName());
         assertEquals("groupsFieldName", oicSecurityRealm.getGroupsFieldName());

diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java
@@ -13,9 +13,13 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.springframework.security.crypto.bcrypt.BCrypt;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 public class OicSecurityRealmTest {
 
@@ -113,4 +117,39 @@ public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws IOException
         assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/bar/"));
         assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/jenkins/../bar/"));
     }
+
+    @Test
+    public void testShouldCheckEscapeHatchWithPlainPassword() throws IOException {
+        final String escapeHatchUsername = "aUsername";
+        final String escapeHatchPassword = "aSecretPassword";
+
+        TestRealm realm = new TestRealm.Builder(wireMockRule)
+                .WithMinimalDefaults()
+                .WithEscapeHatch(true, escapeHatchUsername, escapeHatchPassword, "Group")
+                .build();
+
+        assertEquals(escapeHatchUsername, realm.getEscapeHatchUsername());
+        assertNotEquals(escapeHatchPassword, Secret.toString(realm.getEscapeHatchSecret()));
+        assertTrue(realm.doCheckEscapeHatch(escapeHatchUsername, escapeHatchPassword));
+        assertFalse(realm.doCheckEscapeHatch("otherUsername", escapeHatchPassword));
+        assertFalse(realm.doCheckEscapeHatch(escapeHatchUsername, "wrongPassword"));
+    }
+
+    @Test
+    public void testShouldCheckEscapeHatchWithHashedPassword() throws IOException {
+        final String escapeHatchUsername = "aUsername";
+        final String escapeHatchPassword = "aSecretPassword";
+        final String escapeHatchCryptedPassword = BCrypt.hashpw(escapeHatchPassword, BCrypt.gensalt());
+
+        TestRealm realm = new TestRealm.Builder(wireMockRule)
+                .WithMinimalDefaults()
+                .WithEscapeHatch(true, escapeHatchUsername, escapeHatchCryptedPassword, "Group")
+                .build();
+
+        assertEquals(escapeHatchUsername, realm.getEscapeHatchUsername());
+        assertEquals(escapeHatchCryptedPassword, Secret.toString(realm.getEscapeHatchSecret()));
+        assertTrue(realm.doCheckEscapeHatch(escapeHatchUsername, escapeHatchPassword));
+        assertFalse(realm.doCheckEscapeHatch("otherUsername", escapeHatchPassword));
+        assertFalse(realm.doCheckEscapeHatch(escapeHatchUsername, "wrongPassword"));
+    }
 }
diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java
@@ -179,4 +179,8 @@ public String getStringFieldFromJMESPath(Object object, String jmespathField) {
     public Object readResolve() {
         return super.readResolve();
     }
+
+    public boolean doCheckEscapeHatch(String username, String password) {
+        return super.checkEscapeHatch(username, password);
+    }
 }
diff --git a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml
@@ -8,7 +8,7 @@ jenkins:
       emailFieldName: emailFieldName
       escapeHatchEnabled: true
       escapeHatchGroup: escapeHatchGroup
-      escapeHatchSecret: escapeHatchSecret
+      escapeHatchSecret: "$2a$10$fxteEkfDqwqkmUelZmTxlu9WESjVDKQhp6jsqB1AgsLQ2dC6jikga"
       escapeHatchUsername: escapeHatchUsername
       fullNameFieldName: fullNameFieldName
       groupsFieldName: groupsFieldName