SONAR-19225 Drop BCRYPT hash method for user passwords

11 files changed, 21 insertions, 98 deletions

diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java
index 18c19baa8f7..33ee933299c 100644
--- a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java
+++ b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java
@@ -465,7 +465,7 @@ class UserDaoIT {
       .setResetPassword(true)
       .setSalt("12345")
       .setCryptedPassword("abcde")
-      .setHashMethod("BCRYPT")
+      .setHashMethod("PBKDF2")
       .setExternalLogin("johngithub")
       .setExternalIdentityProvider("github")
       .setExternalId("EXT_ID")
@@ -485,7 +485,7 @@ class UserDaoIT {
     assertThat(reloaded.getSortedScmAccounts()).containsExactly("jo.hn", "john2", "johndoo");
     assertThat(reloaded.getSalt()).isEqualTo("12345");
     assertThat(reloaded.getCryptedPassword()).isEqualTo("abcde");
-    assertThat(reloaded.getHashMethod()).isEqualTo("BCRYPT");
+    assertThat(reloaded.getHashMethod()).isEqualTo("PBKDF2");
     assertThat(reloaded.getExternalLogin()).isEqualTo("johngithub");
     assertThat(reloaded.getExternalIdentityProvider()).isEqualTo("github");
     assertThat(reloaded.getExternalId()).isEqualTo("EXT_ID");
diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java
index 474f5e4760c..cdb44c43719 100644
--- a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java
+++ b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java
@@ -92,7 +92,7 @@ class UserDaoWithPersisterIT {
       .setResetPassword(true)
       .setSalt("12345")
       .setCryptedPassword("abcde")
-      .setHashMethod("BCRYPT")
+      .setHashMethod("PBKDF2")
       .setExternalLogin("johngithub")
       .setExternalIdentityProvider("github")
       .setExternalId("EXT_ID")
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
index 0bcfcf15c6a..a6e4fab242c 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java
@@ -43,7 +43,7 @@ public class UserDto implements UserId {
   private String externalIdentityProvider;
   // Hashed password that may be null in case of external authentication
   private String cryptedPassword;
-  // Salt used for PBKDF2, null when bcrypt is used or for external authentication
+  // Null for external authentication
   private String salt;
   // Hash method used to generate cryptedPassword, my be null in case of external authentication
   private String hashMethod;
diff --git a/server/sonar-db-migration/build.gradle b/server/sonar-db-migration/build.gradle
index 9b5a4032e13..7e51703034d 100644
--- a/server/sonar-db-migration/build.gradle
+++ b/server/sonar-db-migration/build.gradle
@@ -28,7 +28,6 @@ dependencies {
   testImplementation 'org.assertj:assertj-core'
   testImplementation 'org.junit.jupiter:junit-jupiter-api'
   testImplementation 'org.junit.jupiter:junit-jupiter-params'
-  testImplementation 'org.mindrot:jbcrypt'
   testImplementation 'org.mockito:mockito-core'
   testImplementation 'org.xmlunit:xmlunit-core'
   testImplementation 'org.xmlunit:xmlunit-matchers'
diff --git a/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaIT.java b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaIT.java
index d0de5241b0f..8cd32fb3219 100644
--- a/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaIT.java
+++ b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchemaIT.java
@@ -103,14 +103,14 @@ class PopulateInitialSchemaIT {
       .containsEntry("EXTERNAL_LOGIN", "admin")
       .containsEntry("EXT_IDENT_PROVIDER", "sonarqube")
       .containsEntry("USER_LOCAL", true)
-      .containsEntry("CRYPTED_PASSWORD", "$2a$12$uCkkXmhW5ThVK8mpBvnXOOJRLd64LJeHTeCkSuB3lfaR2N0AYBaSi")
-      .containsEntry("HASH_METHOD", "BCRYPT")
+      .containsEntry("CRYPTED_PASSWORD", "100000$R9xDN18ebKxA3ZTaputi6wDt+fcKhP2h3GgAjGbcBlCSlkMLENxw9wziHS46QIW3fWOjEMpeyEts+pNuPXSbYA==")
+      .containsEntry("SALT", "pSDhsn3IM3KCa74CRRf7T7Vx+OE=")
+      .containsEntry("HASH_METHOD", "PBKDF2")
       .containsEntry("CREATED_AT", NOW)
       .containsEntry("RESET_PASSWORD", true)
       .containsEntry("UPDATED_AT", NOW);
 
     assertThat(cols.get("EMAIL")).isNull();
-    assertThat(cols.get("SALT")).isNull();
   }
 
   private void verifyGroup(String expectedName, String expectedDescription) {
diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java
index d4ada1e3561..67a25b6dbbd 100644
--- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v00/PopulateInitialSchema.java
@@ -42,7 +42,8 @@ public class PopulateInitialSchema extends DataChange {
   private static final String ADMINS_GROUP = "sonar-administrators";
   private static final String USERS_GROUP = "sonar-users";
   private static final String ADMIN_USER = "admin";
-  private static final String ADMIN_CRYPTED_PASSWORD = "$2a$12$uCkkXmhW5ThVK8mpBvnXOOJRLd64LJeHTeCkSuB3lfaR2N0AYBaSi";
+  private static final String ADMIN_CRYPTED_PASSWORD = "100000$R9xDN18ebKxA3ZTaputi6wDt+fcKhP2h3GgAjGbcBlCSlkMLENxw9wziHS46QIW3fWOjEMpeyEts+pNuPXSbYA==";
+  private static final String ADMIN_SALT = "pSDhsn3IM3KCa74CRRf7T7Vx+OE=";
   private static final List<String> ADMIN_ROLES = Arrays.asList("admin", "profileadmin", "gateadmin", "provisioning", "applicationcreator", "portfoliocreator");
 
   private final System2 system2;
@@ -75,14 +76,15 @@ public class PopulateInitialSchema extends DataChange {
       "(uuid, login, name, email, external_id, external_login, external_identity_provider, user_local, crypted_password, salt, hash_method, reset_password, " +
       "created_at, updated_at)" +
       " values " +
-      "(?, ?, 'Administrator', null, 'admin', 'admin', 'sonarqube', ?, ?, null, 'BCRYPT', ?, ?, ?)")
+      "(?, ?, 'Administrator', null, 'admin', 'admin', 'sonarqube', ?, ?, ?, 'PBKDF2', ?, ?, ?)")
       .setString(1, uuidFactory.create())
       .setString(2, ADMIN_USER)
       .setBoolean(3, true)
       .setString(4, ADMIN_CRYPTED_PASSWORD)
-      .setBoolean(5, true)
-      .setLong(6, now)
+      .setString(5, ADMIN_SALT)
+      .setBoolean(6, true)
       .setLong(7, now)
+      .setLong(8, now)
       .execute()
       .commit();
 
diff --git a/server/sonar-webserver-api/build.gradle b/server/sonar-webserver-api/build.gradle
index adae50c4454..01364a1bf3c 100644
--- a/server/sonar-webserver-api/build.gradle
+++ b/server/sonar-webserver-api/build.gradle
@@ -25,7 +25,6 @@ dependencies {
   api project(':server:sonar-process')
   api project(':server:sonar-server-common')
   api project(':sonar-plugin-api-impl')
-  api 'org.mindrot:jbcrypt'
 
   compileOnlyApi 'com.github.spotbugs:spotbugs-annotations'
   compileOnlyApi 'jakarta.servlet:jakarta.servlet-api'
diff --git a/server/sonar-webserver-auth/build.gradle b/server/sonar-webserver-auth/build.gradle
index 121114d1d8a..b6b042a8eee 100644
--- a/server/sonar-webserver-auth/build.gradle
+++ b/server/sonar-webserver-auth/build.gradle
@@ -18,7 +18,6 @@ dependencies {
   api project(':server:sonar-webserver-api')
   api project(':sonar-plugin-api-impl')
   api project(':server:sonar-auth-ldap')
-  api 'org.mindrot:jbcrypt'
 
   compileOnlyApi 'com.github.spotbugs:spotbugs-annotations'
   compileOnlyApi 'jakarta.servlet:jakarta.servlet-api'
diff --git a/server/sonar-webserver-auth/src/it/java/org/sonar/server/authentication/CredentialsLocalAuthenticationIT.java b/server/sonar-webserver-auth/src/it/java/org/sonar/server/authentication/CredentialsLocalAuthenticationIT.java
index 57986e45475..d559d6a9778 100644
--- a/server/sonar-webserver-auth/src/it/java/org/sonar/server/authentication/CredentialsLocalAuthenticationIT.java
+++ b/server/sonar-webserver-auth/src/it/java/org/sonar/server/authentication/CredentialsLocalAuthenticationIT.java
@@ -21,13 +21,11 @@ package org.sonar.server.authentication;
 
 import java.security.SecureRandom;
 import java.util.Base64;
-import java.util.Optional;
 import java.util.Random;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.mindrot.jbcrypt.BCrypt;
 import org.sonar.api.config.internal.MapSettings;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
@@ -41,7 +39,6 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.sonar.db.user.UserTesting.newUserDto;
-import static org.sonar.server.authentication.CredentialsLocalAuthentication.HashMethod.BCRYPT;
 import static org.sonar.server.authentication.CredentialsLocalAuthentication.HashMethod.PBKDF2;
 
 public class CredentialsLocalAuthenticationIT {
@@ -84,17 +81,6 @@ public class CredentialsLocalAuthenticationIT {
   }
 
   @Test
-  public void authentication_with_bcrypt_with_correct_password_should_work() {
-    String password = secure().nextAlphanumeric(60);
-
-    UserDto user = newUserDto()
-      .setHashMethod(BCRYPT.name())
-      .setCryptedPassword(BCrypt.hashpw(password, BCrypt.gensalt(12)));
-
-    underTest.authenticate(db.getSession(), user, password, AuthenticationEvent.Method.BASIC);
-  }
-
-  @Test
   public void authentication_with_sha1_should_throw_AuthenticationException() {
     String password = secure().nextAlphanumeric(60);
 
@@ -114,51 +100,17 @@ public class CredentialsLocalAuthenticationIT {
   }
 
   @Test
-  public void authentication_with_bcrypt_with_incorrect_password_should_throw_AuthenticationException() {
-    DbSession dbSession = db.getSession();
-    String password = secure().nextAlphanumeric(60);
-
-    UserDto user = newUserDto()
-      .setHashMethod(BCRYPT.name())
-      .setCryptedPassword(BCrypt.hashpw(password, BCrypt.gensalt(12)));
-
-    assertThatThrownBy(() -> underTest.authenticate(dbSession, user, "WHATEVER", AuthenticationEvent.Method.BASIC))
-      .isInstanceOf(AuthenticationException.class)
-      .hasMessage(CredentialsLocalAuthentication.ERROR_WRONG_PASSWORD);
-  }
-
-  @Test
-  public void authentication_with_bcrypt_with_empty_password_should_throw_AuthenticationException() {
-    DbSession dbSession = db.getSession();
-    UserDto user = newUserDto()
-      .setCryptedPassword(null)
-      .setHashMethod(BCRYPT.name());
-
-    assertThatThrownBy(() -> underTest.authenticate(dbSession, user, "WHATEVER", AuthenticationEvent.Method.BASIC))
-      .isInstanceOf(AuthenticationException.class)
-      .hasMessage(CredentialsLocalAuthentication.ERROR_NULL_PASSWORD_IN_DB);
-  }
-
-  @Test
-  public void authentication_upgrade_hash_function_when_BCRYPT_was_used() {
+  public void authentication_with_bcrypt_should_throw_AuthenticationException() {
     String password = secure().nextAlphanumeric(60);
 
     UserDto user = newUserDto()
-      .setLogin("myself")
-      .setHashMethod(BCRYPT.name())
-      .setCryptedPassword(BCrypt.hashpw(password, BCrypt.gensalt(12)))
-      .setSalt(null);
-    db.users().insertUser(user);
-
-    underTest.authenticate(db.getSession(), user, password, AuthenticationEvent.Method.BASIC);
-
-    Optional<UserDto> myself = db.users().selectUserByLogin("myself");
-    assertThat(myself).isPresent();
-    assertThat(myself.get().getHashMethod()).isEqualTo(PBKDF2.name());
-    assertThat(myself.get().getSalt()).isNotNull();
+      .setHashMethod("BCRYPT")
+      .setCryptedPassword(password);
 
-    // authentication must work with upgraded hash method
-    underTest.authenticate(db.getSession(), user, password, AuthenticationEvent.Method.BASIC);
+    DbSession session = db.getSession();
+    assertThatExceptionOfType(AuthenticationException.class)
+      .isThrownBy(() -> underTest.authenticate(session, user, password, AuthenticationEvent.Method.BASIC))
+      .withMessage("Unknown hash method [BCRYPT]");
   }
 
   @Test
diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/CredentialsLocalAuthentication.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/CredentialsLocalAuthentication.java
index b63be030b60..7fdb174d2bc 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/CredentialsLocalAuthentication.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/CredentialsLocalAuthentication.java
@@ -28,7 +28,6 @@ import javax.annotation.Nullable;
 import javax.crypto.SecretKeyFactory;
 import javax.crypto.spec.PBEKeySpec;
 import org.apache.commons.lang3.RandomStringUtils;
-import org.mindrot.jbcrypt.BCrypt;
 import org.sonar.api.config.Configuration;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -59,12 +58,11 @@ public class CredentialsLocalAuthentication {
   private final EnumMap<HashMethod, HashFunction> hashFunctions = new EnumMap<>(HashMethod.class);
 
   public enum HashMethod {
-    BCRYPT, PBKDF2
+    PBKDF2
   }
 
   public CredentialsLocalAuthentication(DbClient dbClient, Configuration configuration) {
     this.dbClient = dbClient;
-    hashFunctions.put(HashMethod.BCRYPT, new BcryptFunction());
     hashFunctions.put(HashMethod.PBKDF2, new PBKDF2Function(configuration.getInt(PBKDF2_ITERATIONS_PROP).orElse(null)));
   }
 
@@ -244,29 +242,4 @@ public class CredentialsLocalAuthentication {
       }
     }
   }
-
-  /**
-   * Implementation of deprecated bcrypt hash function
-   */
-  private static final class BcryptFunction implements HashFunction {
-    @Override
-    public AuthenticationResult checkCredentials(UserDto user, String password) {
-      if (user.getCryptedPassword() == null) {
-        return new AuthenticationResult(false, ERROR_NULL_PASSWORD_IN_DB);
-      }
-      // This behavior is overridden in most of integration tests for performance reasons, any changes to BCrypt calls should be propagated to
-      // Byteman classes
-      if (!BCrypt.checkpw(password, user.getCryptedPassword())) {
-        return new AuthenticationResult(false, ERROR_WRONG_PASSWORD);
-      }
-      return new AuthenticationResult(true, "");
-    }
-
-    @Override
-    public void storeHashPassword(UserDto user, String password) {
-      user.setHashMethod(HashMethod.BCRYPT.name())
-        .setCryptedPassword(BCrypt.hashpw(password, BCrypt.gensalt(12)))
-        .setSalt(null);
-    }
-  }
 }
diff --git a/server/sonar-webserver-core/build.gradle b/server/sonar-webserver-core/build.gradle
index d178f7c7fbb..63f2814ea6a 100644
--- a/server/sonar-webserver-core/build.gradle
+++ b/server/sonar-webserver-core/build.gradle
@@ -32,7 +32,6 @@ dependencies {
   api 'org.slf4j:slf4j-api'
   api 'org.sonarsource.api.plugin:sonar-plugin-api'
   api 'org.sonarsource.update-center:sonar-update-center-common'
-  api 'org.mindrot:jbcrypt'
 
   api project(':server:sonar-ce-common')
   api project(':server:sonar-ce-task')