From 6db6cda50b8babe052ea512e4692de801c211543 Mon Sep 17 00:00:00 2001 From: Michael Folz Date: Wed, 12 Jul 2023 15:05:42 +0200 Subject: [PATCH] Hotfix 3.1.3 - Replace QueryResultObfuscator and Tests with RandomSiteNameGenerator, using substring of random uuids as obfuscated site ids - Update Spring Boot to 3.1.1 --- CHANGELOG.md | 7 ++ pom.xml | 4 +- .../query/QueryHandlerService.java | 8 +- .../QueryObfuscationSpringConfig.java | 23 ---- .../obfuscation/QueryResultObfuscator.java | 88 --------------- .../query/result/RandomSiteNameGenerator.java | 18 +++ .../query/QueryHandlerServiceIT.java | 2 - .../obfuscation/QueryResultObfuscationIT.java | 105 ------------------ .../QueryResultObfuscationTest.java | 28 ----- .../result/RandomSiteNameGeneratorTest.java | 13 +++ 10 files changed, 43 insertions(+), 253 deletions(-) delete mode 100644 src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryObfuscationSpringConfig.java delete mode 100644 src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscator.java create mode 100644 src/main/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGenerator.java delete mode 100644 src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationIT.java delete mode 100644 src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationTest.java create mode 100644 src/test/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGeneratorTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 30b21080..bca3ebd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +## [3.1.3] - 2023-07-13 + +### Changed +- Obfuscated site ids are no longer consistent over multiple requests of the same result +### Security +- Update Spring Boot to 3.1.1 + ## [3.1.2] - 2023-07-11 ### Security diff --git a/pom.xml b/pom.xml index f0ad114d..b6eb7793 100644 --- a/pom.xml +++ b/pom.xml @@ -5,13 +5,13 @@ org.springframework.boot spring-boot-starter-parent - 3.1.0 + 3.1.1 de.medizininformatik-initiative FeasibilityGuiBackend - 3.1.2 + 3.1.3 FeasibilityGuiBackend Backend of the Feasibility GUI diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java index 0803c4d3..a05de8e1 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java @@ -8,8 +8,8 @@ import de.numcodex.feasibility_gui_backend.query.api.*; import de.numcodex.feasibility_gui_backend.query.dispatch.QueryDispatchException; import de.numcodex.feasibility_gui_backend.query.dispatch.QueryDispatcher; -import de.numcodex.feasibility_gui_backend.query.obfuscation.QueryResultObfuscator; import de.numcodex.feasibility_gui_backend.query.persistence.*; +import de.numcodex.feasibility_gui_backend.query.result.RandomSiteNameGenerator; import de.numcodex.feasibility_gui_backend.query.result.ResultLine; import de.numcodex.feasibility_gui_backend.query.result.ResultService; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateException; @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.UUID; @Service @RequiredArgsConstructor @@ -55,9 +56,6 @@ public enum ResultDetail { @NonNull private final SavedQueryRepository savedQueryRepository; - @NonNull - private final QueryResultObfuscator queryResultObfuscator; - @NonNull private ObjectMapper jsonUtil; @@ -79,7 +77,7 @@ public QueryResult getQueryResult(Long queryId, ResultDetail resultDetail) { if (resultDetail != ResultDetail.SUMMARY) { resultLines = singleSiteResults.stream() .map(ssr -> QueryResultLine.builder() - .siteName(resultDetail == ResultDetail.DETAILED_OBFUSCATED ? queryResultObfuscator.tokenizeSiteName(queryId, ssr.siteName()) : ssr.siteName()) + .siteName(resultDetail == ResultDetail.DETAILED_OBFUSCATED ? RandomSiteNameGenerator.generateRandomSiteName() : ssr.siteName()) .numberOfPatients(ssr.result()) .build()) .toList(); diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryObfuscationSpringConfig.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryObfuscationSpringConfig.java deleted file mode 100644 index 477c64b9..00000000 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryObfuscationSpringConfig.java +++ /dev/null @@ -1,23 +0,0 @@ -package de.numcodex.feasibility_gui_backend.query.obfuscation; - -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - -@Configuration -public class QueryObfuscationSpringConfig { - - @Bean - public QueryResultObfuscator createQueryResultObfuscator(@Qualifier("obfuscation") MessageDigest hashFn) { - return new QueryResultObfuscator(hashFn); - } - - @Qualifier("obfuscation") - @Bean - public MessageDigest createObfuscationHashFn() throws NoSuchAlgorithmException { - return MessageDigest.getInstance("SHA3-256"); - } -} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscator.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscator.java deleted file mode 100644 index 21d651a1..00000000 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscator.java +++ /dev/null @@ -1,88 +0,0 @@ -package de.numcodex.feasibility_gui_backend.query.obfuscation; - -import lombok.NonNull; -import lombok.RequiredArgsConstructor; - -import jakarta.validation.constraints.NotNull; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.util.Arrays; - -/** - * Provides functionality for obfuscating a site using a tokenization technique. - * This allows for replacing real data (site name) with some kind of placeholder without - * this process being reversible. - */ -@RequiredArgsConstructor -public class QueryResultObfuscator { - - /** - * Collision resistance is mainly based on the birthday paradox. - * For a desired collision probability <50% this can be defined as: - *


- * p ~ n^2 / 2m - *


- * Where n is the number of items and m is the number of possibilities per item. - * Since this obfuscator uses a hash function the output of this function will - * always be in hex format. For a hex string the number of possibilities is - * 16^c where c is the number of characters in this string, i.e. its length. - *


- * Assuming that we have around 2000 hospitals in Germany and all of them would - * participate, then given the truncation output length below the collision - * probability would be: - *


- * p ~ 2000^2 / 2 * 16^10
- * p ~ 0.00000181... - *


- * This should be more than sufficient for what we are trying to accomplish here. - */ - private static final int TRUNCATION_OUTPUT_LENGTH = 10; - - @NonNull - private MessageDigest hashFn; - - /** - * Tokenizes the site name of the given queryid and site name. That is, replaces it with a - * placeholder in order to obfuscate it. - *

- * Tokenization is based on a hash function seeded by information identifying the query id and site name. - * - * @param queryId The query id for which the site name shall be tokenized. - * @param siteName The site name that shall be tokenized - * @return The tokenized site name. - */ - public String tokenizeSiteName(Long queryId, @NotNull String siteName) { - if (siteName == null) { - throw new IllegalArgumentException("siteName must not be null"); - } - - var seed = generateSeed(queryId, siteName); - hashFn.reset(); - hashFn.update(seed.getBytes(StandardCharsets.UTF_8)); - var siteNameToken = hashFn.digest(siteName.getBytes(StandardCharsets.UTF_8)); - var siteNameTokenHex = byteToHex(siteNameToken); - return truncateHashValue(siteNameTokenHex); - } - - private String generateSeed(Long queryId, String siteName) { - - var queryIdHash = hashFn.digest(String.valueOf(queryId) - .getBytes(StandardCharsets.UTF_8)); - var siteNameHash = hashFn.digest(siteName.getBytes(StandardCharsets.UTF_8)); - - return Arrays.toString(queryIdHash) + - Arrays.toString(siteNameHash); - } - - private String byteToHex(byte[] hash) { - var hex = new StringBuilder(); - for (byte b : hash) { - hex.append(String.format("%02x", b)); - } - return hex.toString(); - } - - private String truncateHashValue(String hashValue) { - return hashValue.substring(0, TRUNCATION_OUTPUT_LENGTH); - } -} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGenerator.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGenerator.java new file mode 100644 index 00000000..c07ad34d --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGenerator.java @@ -0,0 +1,18 @@ +package de.numcodex.feasibility_gui_backend.query.result; + +import java.util.UUID; + +/** + * Generates a random string with a length of 10 by using the last 10 characters of a UUID. + *

+ * UUIDs have the format 8-4-4-4-12. For historical reasons, we currently need 10 characters, so we just use the + * last 10, meaning the substring begin index is 8+4+4+4+2 + the 4 hyphens = 26 + */ +public class RandomSiteNameGenerator { + + private static final int BEGIN_INDEX = 26; + + public static String generateRandomSiteName() { + return UUID.randomUUID().toString().substring(BEGIN_INDEX); + } +} diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java index 3688071f..8c76605b 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java @@ -6,7 +6,6 @@ import de.numcodex.feasibility_gui_backend.query.broker.BrokerSpringConfig; import de.numcodex.feasibility_gui_backend.query.collect.QueryCollectSpringConfig; import de.numcodex.feasibility_gui_backend.query.dispatch.QueryDispatchSpringConfig; -import de.numcodex.feasibility_gui_backend.query.obfuscation.QueryObfuscationSpringConfig; import de.numcodex.feasibility_gui_backend.query.persistence.*; import de.numcodex.feasibility_gui_backend.query.result.ResultLine; import de.numcodex.feasibility_gui_backend.query.result.ResultService; @@ -39,7 +38,6 @@ QueryDispatchSpringConfig.class, QueryCollectSpringConfig.class, QueryHandlerService.class, - QueryObfuscationSpringConfig.class, QueryTemplateHandler.class, ResultServiceSpringConfig.class }) diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationIT.java deleted file mode 100644 index 19dbe687..00000000 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationIT.java +++ /dev/null @@ -1,105 +0,0 @@ -package de.numcodex.feasibility_gui_backend.query.obfuscation; - - -import de.numcodex.feasibility_gui_backend.query.persistence.*; -import de.numcodex.feasibility_gui_backend.query.result.ResultLine; -import de.numcodex.feasibility_gui_backend.query.result.ResultService; -import de.numcodex.feasibility_gui_backend.query.result.ResultServiceSpringConfig; -import org.junit.jupiter.api.Tag; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; -import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; -import org.springframework.context.annotation.Import; -import org.testcontainers.junit.jupiter.Testcontainers; - -import java.sql.Timestamp; -import java.time.Instant; - -import static de.numcodex.feasibility_gui_backend.query.persistence.ResultType.SUCCESS; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; - -@Tag("query") -@Tag("obfuscation") -@Import({ - QueryObfuscationSpringConfig.class, - ResultServiceSpringConfig.class -}) -@DataJpaTest -@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) -@Testcontainers -public class QueryResultObfuscationIT { - - @Autowired - private QueryContentRepository queryContentRepository; - - @Autowired - private QueryRepository queryRepository; - - @Autowired - private ResultService resultService; - - @Autowired - private QueryResultObfuscator queryResultObfuscator; - - @Test - public void testTokenizeSiteName_ResultsOfTheSameSiteProduceDifferentTokensForDifferentQueries() { - var testQueryContent = new QueryContent("irrelevant-for-this-test"); - testQueryContent.setHash("ab34ffcd"); // irrelevant for this test, too - queryContentRepository.save(testQueryContent); - - var testQueryA = new Query(); - testQueryA.setQueryContent(testQueryContent); - testQueryA.setCreatedAt(Timestamp.from(Instant.now())); - testQueryA.setCreatedBy("someone"); - queryRepository.save(testQueryA); - - var testQueryB = new Query(); - testQueryB.setQueryContent(testQueryContent); - testQueryB.setCreatedAt(Timestamp.from(Instant.now())); - testQueryB.setCreatedBy("someone"); - queryRepository.save(testQueryB); - - var testSite = "A"; - - // Dispatch entries are left out for brevity. Also, they do not matter for this test scenario. - - var testSiteAResult1 = new ResultLine(testSite, SUCCESS, 10L); - resultService.addResultLine(testQueryA.getId(), testSiteAResult1); - - var testSiteAResult2 = new ResultLine(testSite, SUCCESS, 20L); - resultService.addResultLine(testQueryB.getId(), testSiteAResult2); - - var tokenTestSiteAResult1 = queryResultObfuscator.tokenizeSiteName(testQueryA.getId(), testSite); - var tokenTestSiteAResult2 = queryResultObfuscator.tokenizeSiteName(testQueryB.getId(), testSite); - - assertNotEquals(tokenTestSiteAResult1, tokenTestSiteAResult2); - } - - // This one is very important since the UI is actually polling. Thus, the results need to be stable! - @Test - public void testTokenizeSiteName_MultipleCallsProduceTheSameToken() { - var testQueryContent = new QueryContent("irrelevant-for-this-test"); - testQueryContent.setHash("ab34ffcd"); // irrelevant for this test, too - queryContentRepository.save(testQueryContent); - - var testQueryA = new Query(); - testQueryA.setQueryContent(testQueryContent); - testQueryA.setCreatedAt(Timestamp.from(Instant.now())); - testQueryA.setCreatedBy("someone"); - queryRepository.save(testQueryA); - - var testSite = "A"; - - // Dispatch entries are left out for brevity. Also, they do not matter for this test scenario. - - var testSiteAResult = new ResultLine(testSite, SUCCESS, 10L); - resultService.addResultLine(testQueryA.getId(), testSiteAResult); - - var tokenTestSiteAResult1 = queryResultObfuscator.tokenizeSiteName(testQueryA.getId(), testSite); - var tokenTestSiteAResult2 = queryResultObfuscator.tokenizeSiteName(testQueryA.getId(), testSite); - - assertEquals(tokenTestSiteAResult1, tokenTestSiteAResult2); - } -} diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationTest.java deleted file mode 100644 index d747570c..00000000 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/obfuscation/QueryResultObfuscationTest.java +++ /dev/null @@ -1,28 +0,0 @@ -package de.numcodex.feasibility_gui_backend.query.obfuscation; - -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Tag; -import org.junit.jupiter.api.Test; - -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - -import static org.junit.jupiter.api.Assertions.assertThrows; - -@Tag("query") -@Tag("obfuscation") -public class QueryResultObfuscationTest { - - public static QueryResultObfuscator queryResultObfuscator; - - @BeforeAll - public static void setUp() throws NoSuchAlgorithmException { - var hashFn = MessageDigest.getInstance("SHA3-256"); - queryResultObfuscator = new QueryResultObfuscator(hashFn); - } - - @Test - public void testTokenizeSiteName_NullResultThrows() { - assertThrows(IllegalArgumentException.class, () -> queryResultObfuscator.tokenizeSiteName(null, null)); - } -} diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGeneratorTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGeneratorTest.java new file mode 100644 index 00000000..103e6fab --- /dev/null +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/result/RandomSiteNameGeneratorTest.java @@ -0,0 +1,13 @@ +package de.numcodex.feasibility_gui_backend.query.result; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +class RandomSiteNameGeneratorTest { + + @Test + void checkForCorrectPattern() { + assertThat(RandomSiteNameGenerator.generateRandomSiteName()).matches("[0-9a-f]{10}"); + } +}