From 61dd09ebbbf2782b7d4b629fa7f3ae4af50d37e7 Mon Sep 17 00:00:00 2001 From: Tobias Koch Date: Mon, 8 Apr 2024 10:53:54 +0200 Subject: [PATCH] Require salt and iterationCount configuration --- .../rest/config/SecurityConfig.java | 14 +- .../QBiCTokenAuthenticationProvider.java | 17 +-- .../rest/security/QBiCTokenMatcher.java | 88 ------------ .../rest/security/QBicTokenEncoder.java | 86 +++++++++++ .../rest/security/TokenEncoder.java | 9 ++ .../token/EncodedAccessTokenRepository.java | 6 +- ...itional-spring-configuration-metadata.json | 12 +- .../src/main/resources/application.properties | 2 + .../rest/security/QBiCTokenMatcherTest.java | 22 --- .../rest/security/QBicTokenEncoderTest.java | 133 ++++++++++++++++++ 10 files changed, 260 insertions(+), 129 deletions(-) delete mode 100644 rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenMatcher.java create mode 100644 rest-api/src/main/java/life/qbic/data_download/rest/security/QBicTokenEncoder.java create mode 100644 rest-api/src/main/java/life/qbic/data_download/rest/security/TokenEncoder.java delete mode 100644 rest-api/src/test/java/life/qbic/data_download/rest/security/QBiCTokenMatcherTest.java create mode 100644 rest-api/src/test/java/life/qbic/data_download/rest/security/QBicTokenEncoderTest.java diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/config/SecurityConfig.java b/rest-api/src/main/java/life/qbic/data_download/rest/config/SecurityConfig.java index 9eaeb21..aafa41a 100644 --- a/rest-api/src/main/java/life/qbic/data_download/rest/config/SecurityConfig.java +++ b/rest-api/src/main/java/life/qbic/data_download/rest/config/SecurityConfig.java @@ -5,9 +5,9 @@ import javax.sql.DataSource; import life.qbic.data_download.rest.security.QBiCTokenAuthenticationFilter; import life.qbic.data_download.rest.security.QBiCTokenAuthenticationProvider; -import life.qbic.data_download.rest.security.QBiCTokenMatcher; +import life.qbic.data_download.rest.security.QBicTokenEncoder; import life.qbic.data_download.rest.security.RequestAuthorizationManagerFactory; -import life.qbic.data_download.rest.security.TokenMatcher; +import life.qbic.data_download.rest.security.TokenEncoder; import life.qbic.data_download.rest.security.acl.MeasurementMappingService; import life.qbic.data_download.rest.security.acl.QBiCMeasurementMappingService; import life.qbic.data_download.rest.security.acl.QbicPermissionEvaluator; @@ -64,16 +64,18 @@ public class SecurityConfig { @Bean("accessTokenEncoder") - public TokenMatcher tokenEncoder() { - return new QBiCTokenMatcher(); + public TokenEncoder tokenEncoder( + @Value("${qbic.access-token.salt}") String salt, + @Value("${qbic.access-token.iteration-count}") int iterationCount) { + return new QBicTokenEncoder(salt, iterationCount); } @Bean("tokenAuthenticationProvider") public QBiCTokenAuthenticationProvider authenticationProvider( - @Qualifier("accessTokenEncoder") TokenMatcher tokenMatcher, + @Qualifier("accessTokenEncoder") TokenEncoder tokenEncoder, EncodedAccessTokenRepository encodedAccessTokenRepository, UserDetailsRepository userDetailsRepository) { - return new QBiCTokenAuthenticationProvider(tokenMatcher, encodedAccessTokenRepository, + return new QBiCTokenAuthenticationProvider(tokenEncoder, encodedAccessTokenRepository, userDetailsRepository); } diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenAuthenticationProvider.java b/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenAuthenticationProvider.java index 22ff2f9..8e910e6 100644 --- a/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenAuthenticationProvider.java +++ b/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenAuthenticationProvider.java @@ -19,14 +19,14 @@ */ public class QBiCTokenAuthenticationProvider implements AuthenticationProvider { - private final TokenMatcher tokenMatcher; + private final TokenEncoder tokenEncoder; private final EncodedAccessTokenRepository encodedAccessTokenRepository; private final UserDetailsRepository userDetailsRepository; - public QBiCTokenAuthenticationProvider(TokenMatcher tokenMatcher, + public QBiCTokenAuthenticationProvider(TokenEncoder tokenEncoder, EncodedAccessTokenRepository encodedAccessTokenRepository, UserDetailsRepository userDetailsRepository) { - this.tokenMatcher = requireNonNull(tokenMatcher, "tokenMatcher must not be null"); + this.tokenEncoder = requireNonNull(tokenEncoder, "tokenEncoder must not be null"); this.encodedAccessTokenRepository = requireNonNull(encodedAccessTokenRepository, "encodedAccessTokenRepository must not be null"); this.userDetailsRepository = requireNonNull(userDetailsRepository, @@ -37,13 +37,10 @@ public QBiCTokenAuthenticationProvider(TokenMatcher tokenMatcher, public Authentication authenticate(Authentication authentication) throws AuthenticationException { if (authentication instanceof QBiCTokenAuthenticationRequest authenticationRequest) { String token = authenticationRequest.getToken(); - EncodedAccessToken encodedAccessToken = encodedAccessTokenRepository.findAll() - .parallelStream() - .filter(storedToken -> tokenMatcher.matches(token.toCharArray(), - storedToken.getAccessToken())) - .findAny().orElseThrow( - () -> new BadCredentialsException("not a valid token") - ); + String encodedToken = tokenEncoder.encode(token); + EncodedAccessToken encodedAccessToken = encodedAccessTokenRepository + .findByAccessTokenEquals(encodedToken) + .orElseThrow(() -> new BadCredentialsException("not a valid token")); if (encodedAccessToken.isExpired()) { throw new CredentialsExpiredException("expired token"); } diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenMatcher.java b/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenMatcher.java deleted file mode 100644 index bf95406..0000000 --- a/rest-api/src/main/java/life/qbic/data_download/rest/security/QBiCTokenMatcher.java +++ /dev/null @@ -1,88 +0,0 @@ -package life.qbic.data_download.rest.security; - -import java.security.NoSuchAlgorithmException; -import java.security.spec.InvalidKeySpecException; -import java.security.spec.KeySpec; -import java.util.Arrays; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; -import javax.crypto.spec.SecretKeySpec; - -/** - * A token encoder - */ -public class QBiCTokenMatcher implements TokenMatcher { - - private static final int ITERATION_COUNT_INDEX = 0; // the index of the iteration count in the encoded token - private static final int SALT_INDEX = 1; // the index of the salt content in the encoded token - private static final int HASH_INDEX = 2; // the index of the hash content in the encoded token - - private record EncryptionSettings(int iterationCount, String cipher, int keyBitSize) {} - - private static final EncryptionSettings ENCRYPTION_SETTINGS = new EncryptionSettings( - 100_000, - "AES", - 256 - ); - - @Override - public boolean matches(char[] token, String encodedToken) { - byte[] readSalt = readSalt(encodedToken); - int iterationCount = readIterationCount(encodedToken); - byte[] asEncoded = pbe(token, readSalt, iterationCount); - byte[] hashedValue = readHash(encodedToken); - return Arrays.equals(asEncoded, hashedValue); - } - - - private static byte[] readSalt(String encodedToken) { - return fromHex(encodedToken.split(":")[SALT_INDEX]); - } - - private static int readIterationCount(String encodedToken) { - return Integer.parseInt(encodedToken.split(":")[ITERATION_COUNT_INDEX]); - } - - private static byte[] readHash(String encodedToken) { - return fromHex(encodedToken.split(":")[HASH_INDEX]); - } - - /** - * Converts a hexadecimal String representation to a byte array - * - * @param hex the hexadecimal String - * @return the converted byte array - * @since 1.0.0 - */ - private static byte[] fromHex(String hex) { - byte[] binary = new byte[hex.length() / 2]; - for (int i = 0; i < binary.length; i++) { - binary[i] = (byte) Integer.parseInt(hex.substring(2 * i, 2 * i + 2), 16); - } - return binary; - } - - /** - * Uses Password-Based Encryption to encrypt a token given a salt and iterations - * @param token the token to be encrypted - * @param salt the salt used in the encryption - * @param iterationCount the number of iterations - * @return encryption result - */ - private static byte[] pbe(char[] token, byte[] salt, int iterationCount) { - KeySpec spec = - new PBEKeySpec(token, salt, iterationCount, ENCRYPTION_SETTINGS.keyBitSize()); - SecretKey secretKey; - try { - SecretKeyFactory result; - result = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - SecretKeyFactory factory = result; - secretKey = factory.generateSecret(spec); - } catch (InvalidKeySpecException | NoSuchAlgorithmException e) { - throw new RuntimeException("error encrypting token: " + e.getMessage()); - } - return new SecretKeySpec(secretKey.getEncoded(), ENCRYPTION_SETTINGS.cipher()).getEncoded(); - } - -} diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/security/QBicTokenEncoder.java b/rest-api/src/main/java/life/qbic/data_download/rest/security/QBicTokenEncoder.java new file mode 100644 index 0000000..b5e5f0e --- /dev/null +++ b/rest-api/src/main/java/life/qbic/data_download/rest/security/QBicTokenEncoder.java @@ -0,0 +1,86 @@ +package life.qbic.data_download.rest.security; + +import static java.util.Objects.requireNonNull; + +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.KeySpec; +import java.util.HexFormat; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; + +/** + * A token encoder + */ +public class QBicTokenEncoder implements TokenEncoder { + + private static final int EXPECTED_MIN_SALT_BITS = 128; + private static final int EXPECTED_MIN_SALT_BYTES = (int) Math.ceil( + (double) EXPECTED_MIN_SALT_BITS / 8); + private static final int EXPECTED_MIN_ITERATION_COUNT = 100_000; + + private final byte[] salt; + private final int iterationCount; + + public QBicTokenEncoder(String salt, int iterationCount) { + this.salt = fromHex(requireNonNull(salt, "salt must not be null")); + if (this.salt.length < EXPECTED_MIN_SALT_BYTES) { + throw new IllegalArgumentException( + "salt must have at least " + EXPECTED_MIN_SALT_BITS + " bits."); + } + if (iterationCount < EXPECTED_MIN_ITERATION_COUNT) { + throw new IllegalArgumentException( + "Iteration count n=" + iterationCount + " cannot be less than n=" + + EXPECTED_MIN_ITERATION_COUNT); + } + this.iterationCount = iterationCount; + } + + @Override + public String encode(String token) { + byte[] hash = pbe(token.toCharArray(), salt, iterationCount); + return iterationCount + ":" + toHex(salt) + ":" + toHex(hash); + } + + private record EncryptionSettings(String cipher, int keyBitSize) { + } + + private static final EncryptionSettings ENCRYPTION_SETTINGS = new EncryptionSettings( + "AES", + 256 + ); + + private static byte[] fromHex(String hex) { + return HexFormat.of().parseHex(hex); + } + + private static String toHex(byte[] bytes) { + HexFormat hexFormat = HexFormat.of(); + return hexFormat.formatHex(bytes); + } + + /** + * Uses Password-Based Encryption to encrypt a token given a salt and iterations + * @param token the token to be encrypted + * @param salt the salt used in the encryption + * @param iterationCount the number of iterations + * @return encryption result + */ + private static byte[] pbe(char[] token, byte[] salt, int iterationCount) { + KeySpec spec = + new PBEKeySpec(token, salt, iterationCount, ENCRYPTION_SETTINGS.keyBitSize()); + SecretKey secretKey; + try { + SecretKeyFactory result; + result = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); + SecretKeyFactory factory = result; + secretKey = factory.generateSecret(spec); + } catch (InvalidKeySpecException | NoSuchAlgorithmException e) { + throw new RuntimeException("error encrypting token: " + e.getMessage()); + } + return new SecretKeySpec(secretKey.getEncoded(), ENCRYPTION_SETTINGS.cipher()).getEncoded(); + } + +} diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/security/TokenEncoder.java b/rest-api/src/main/java/life/qbic/data_download/rest/security/TokenEncoder.java new file mode 100644 index 0000000..7878b97 --- /dev/null +++ b/rest-api/src/main/java/life/qbic/data_download/rest/security/TokenEncoder.java @@ -0,0 +1,9 @@ +package life.qbic.data_download.rest.security; + +/** + * Encodes QBiC Access Tokens + */ +public interface TokenEncoder { + + String encode(String token); +} diff --git a/rest-api/src/main/java/life/qbic/data_download/rest/security/jpa/token/EncodedAccessTokenRepository.java b/rest-api/src/main/java/life/qbic/data_download/rest/security/jpa/token/EncodedAccessTokenRepository.java index f75c429..ffb4743 100644 --- a/rest-api/src/main/java/life/qbic/data_download/rest/security/jpa/token/EncodedAccessTokenRepository.java +++ b/rest-api/src/main/java/life/qbic/data_download/rest/security/jpa/token/EncodedAccessTokenRepository.java @@ -1,8 +1,10 @@ package life.qbic.data_download.rest.security.jpa.token; -import org.springframework.data.jpa.repository.JpaRepository; +import java.util.Optional; +import org.springframework.data.repository.Repository; -public interface EncodedAccessTokenRepository extends JpaRepository { +public interface EncodedAccessTokenRepository extends Repository { + Optional findByAccessTokenEquals(String accessToken); } diff --git a/rest-api/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/rest-api/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 8b27970..cb93cac 100644 --- a/rest-api/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/rest-api/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -3,7 +3,7 @@ { "name": "openbis.filename.ignored-prefix", "type": "java.lang.String", - "description": "A prefix that is ignored in the filepaths. Defaults to original/" + "description": "A prefix that is stripped from the file's path." }, { "name": "qbic.access-management.datasource.url", @@ -19,5 +19,15 @@ "name": "qbic.access-management.datasource.password", "type": "java.lang.String", "description": "The password for the datasource from which access control information is fetched." + }, + { + "name": "qbic.access-token.salt", + "type": "java.lang.String", + "description": "A hex string used as salt for the personal access tokens." + }, + { + "name": "qbic.access-token.iteration-count", + "type": "java.lang.String", + "description": "The number of iterations used for encrypting personal access tokens." } ] } diff --git a/rest-api/src/main/resources/application.properties b/rest-api/src/main/resources/application.properties index ad67890..39eb20b 100644 --- a/rest-api/src/main/resources/application.properties +++ b/rest-api/src/main/resources/application.properties @@ -11,6 +11,8 @@ qbic.access-management.datasource.url=${ACCESS_DB_URL:${spring.datasource.url}} qbic.access-management.datasource.username=${ACCESS_DB_USER_NAME:${spring.datasource.username}} qbic.access-management.datasource.password=${ACCESS_DB_USER_PASSWORD:${spring.datasource.password}} qbic.access-management.datasource.driver-class-name=${ACCESS_DB_DRIVER:${spring.datasource.driver-class-name}} +qbic.access-token.salt=${ACCESS_TOKEN_SALT:} +qbic.access-token.iteration-count=${ACCESS_TOKEN_ITERATIONS:100000} ### server settings server.download.token-name=${TOKEN_NAME:Bearer} diff --git a/rest-api/src/test/java/life/qbic/data_download/rest/security/QBiCTokenMatcherTest.java b/rest-api/src/test/java/life/qbic/data_download/rest/security/QBiCTokenMatcherTest.java deleted file mode 100644 index 89a373f..0000000 --- a/rest-api/src/test/java/life/qbic/data_download/rest/security/QBiCTokenMatcherTest.java +++ /dev/null @@ -1,22 +0,0 @@ -package life.qbic.data_download.rest.security; - -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; - -class QBiCTokenMatcherTest { - - @ParameterizedTest - @CsvSource({ - "43aZ3xka9613JC64F01cJA6wvS7qTO1a,100000:dc0fbdfb18b7bd1765660f50d4e60e401aa4ce29:9e67a05c631a0e87d78252b238268aa84fe2f72e9725c5667e7e3be05696b57f", - "P32o4r65r60w62c9NA2ed7NwK74625F6,100000:ad83ad744ab13e89a55f92da5e42c1378f77d4a7:22c11194a8686c56d00c3ba819db9f14a9cf864d5c8d27a8a645c48e1c110100" - }) - @DisplayName("The raw token can be compared to the encoded token") - void theRawTokenCanBeComparedToTheEncodedToken(String rawToken, String encodedToken) { - TokenMatcher qBiCTokenMatcher = new QBiCTokenMatcher(); - assertTrue(qBiCTokenMatcher.matches(rawToken.toCharArray(), encodedToken)); - } - -} diff --git a/rest-api/src/test/java/life/qbic/data_download/rest/security/QBicTokenEncoderTest.java b/rest-api/src/test/java/life/qbic/data_download/rest/security/QBicTokenEncoderTest.java new file mode 100644 index 0000000..c6de2ac --- /dev/null +++ b/rest-api/src/test/java/life/qbic/data_download/rest/security/QBicTokenEncoderTest.java @@ -0,0 +1,133 @@ +package life.qbic.data_download.rest.security; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.security.SecureRandom; +import java.util.HexFormat; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +class QBicTokenEncoderTest { + + final int validIterationCount = 100_000; + final String validSalt = "010203040506070809000a0b0c0d0e0f"; + + + @ParameterizedTest + @ValueSource(strings = { + "test", + "0018928349298347979234", + "alsdkfj2983749234lkjwer0" + }) + @DisplayName("encoded tokens are different than raw input") + void encodedTokensAreDifferentThanRawInput(String input) { + TokenEncoder classUnderTest = new QBicTokenEncoder(validSalt, validIterationCount); + String encoded = classUnderTest.encode(input); + assertNotEquals(input, encoded); + } + + @ParameterizedTest + @CsvSource({ + "test1234,test2345", + "02304980928340,02304980928341" + }) + @DisplayName("different tokens lead to different encoded tokens") + void differentTokensLeadToDifferentEncodedTokens(String tokenOne, String tokenTwo) { + TokenEncoder classUnderTest = new QBicTokenEncoder(validSalt, validIterationCount); + String encodedOne = classUnderTest.encode(tokenOne); + String encodedTwo = classUnderTest.encode(tokenTwo); + + assertNotEquals(encodedOne, encodedTwo); + } + + @ParameterizedTest + @ValueSource(strings = { + "wlekrjwoweirwnwelrkjklwjer", + "023948092349023804980982340982034", + "ksjdhskjfkwernmq293847102398", + "1", + "a" + }) + @DisplayName("identical tokens lead to the same encoded tokens") + void identicalTokensLeadToTheSameEncodedTokens(String token) { + TokenEncoder classUnderTest = new QBicTokenEncoder(validSalt, validIterationCount); + String encodedOne = classUnderTest.encode(token); + String encodedTwo = classUnderTest.encode(token); + + assertEquals(encodedOne, encodedTwo); + } + + @ParameterizedTest + @ValueSource(ints = { + 0, 1, 2, 4, 7, 12, 14, 15 + }) + @DisplayName("when the salt has less than 128 bits then fail") + void whenTheSaltHasLessThan128BitsThenFail(int saltByteNumber) { + var salt = generateRandomSalt(saltByteNumber); + assertThrows(IllegalArgumentException.class, + () -> new QBicTokenEncoder(salt, validIterationCount)); + } + + @Test + @DisplayName("when the salt has 128 bits succeed") + void whenTheSaltHas128BitsSucceed() { + String salt = generateRandomSalt(16); + var underTest = new QBicTokenEncoder(salt, validIterationCount); + assertDoesNotThrow(() -> underTest.encode("myToken")); + } + + @ParameterizedTest + @ValueSource(ints = { + 16, 17, 18, 19, 77, 200 + }) + @DisplayName("when the salt has more than 128 bits succeed") + void whenTheSaltHasMoreThan128BitsSucceed(int saltByteNumber) { + String salt = generateRandomSalt(saltByteNumber); + var underTest = new QBicTokenEncoder(salt, validIterationCount); + assertDoesNotThrow(() -> underTest.encode("myToken")); + } + + @ParameterizedTest + @ValueSource(ints = { + 0,1,2,7,9,42,50,100,1_000,99_998, 99_999 + }) + @DisplayName("when the iteration count is below 100_000 then fail") + void whenTheIterationCountIsBelow100000ThenFail(int iterationCount) { + assertThrows(IllegalArgumentException.class, + () -> new QBicTokenEncoder(validSalt, iterationCount)); + } + + @Test + @DisplayName("when the iteration count is 100_000 then succeed") + void whenTheIterationCountIs100000ThenSucceed() { + assertDoesNotThrow(() -> { + var tokenEncoder = new QBicTokenEncoder(validSalt, 100_000); + tokenEncoder.encode("myToken"); + }); + } + + @ParameterizedTest + @ValueSource(ints = { + 100_001, 100_002, 150_000 + }) @DisplayName("when the iteration count is higher than 100_000 then succeed") + void whenTheIterationCountIsHigherThan100000ThenSucceed(int iterationCount) { + assertDoesNotThrow(() -> { + QBicTokenEncoder qBicTokenEncoder = new QBicTokenEncoder(validSalt, iterationCount); + qBicTokenEncoder.encode("myToken"); + }); + } + + String generateRandomSalt(int byteCount) { + var random = new SecureRandom(); + byte[] bytes = new byte[byteCount]; + random.nextBytes(bytes); + return HexFormat.of().formatHex(bytes); + } + +}