diff --git a/docs/docs/usage/evaluate/findings/findings.md b/docs/docs/usage/evaluate/findings/findings.md index 3f43b8f7e18..f02abf701a4 100644 --- a/docs/docs/usage/evaluate/findings/findings.md +++ b/docs/docs/usage/evaluate/findings/findings.md @@ -33,6 +33,29 @@ Each Finding is deduplicated by its combination of value, type, and field. If th Additional types exist for Active Directory findings (SID, delegation, Kerberoastable accounts, ASREPRoastable accounts, etc.). +## Sensitive Findings + +Some Finding types carry secret material and are flagged as **sensitive**: their value is redacted +everywhere the platform returns it (list, detail, Simulation, Scenario, Endpoint and Inject views). + +| Sensitive type | Value shape | Redacted as | +| --- | --- | --- | +| Credentials | `admin:motdepasse` | `ad******:mo******` | + +Every part of the value - the parts being separated by `:` - is masked the same way: only a two +character fragment is kept, so you can still tell which Finding is which when you already know the +value, without the platform ever disclosing it. A part too short to keep a fragment safely is masked +entirely. + +!!! warning "The secret is not deleted" + + The full value is still stored in the database, because deduplication, correlation and attack + path computation rely on it. Only its API representation is redacted: it is not possible to + retrieve the cleartext value of a sensitive Finding through the REST API. + +Sensitivity is decided per Finding type, not per Finding. Findings created before the upgrade are +flagged retroactively, so previously detected sensitive Findings are redacted as well. + ## Findings list Navigate to **Findings** in the left menu to see all Findings in an aggregated view. The list groups Findings by unique value and type, merging Assets from all occurrences into a single row. @@ -42,7 +65,7 @@ Each row displays: | Column | Description | |---|---| | Type | The Finding category (CVE, Port, Credentials, etc.) | -| Value | The technical value (monospace display) | +| Value | The technical value (monospace display), redacted for sensitive Findings | | Assets | Endpoints where the Finding was detected | | Asset groups | Asset groups containing affected endpoints | | First seen | When the Finding was first detected | diff --git a/openaev-api/src/main/java/io/openaev/migration/V6_20260824180000000__Add_finding_is_sensitive.java b/openaev-api/src/main/java/io/openaev/migration/V6_20260824180000000__Add_finding_is_sensitive.java new file mode 100644 index 00000000000..f13da049d9f --- /dev/null +++ b/openaev-api/src/main/java/io/openaev/migration/V6_20260824180000000__Add_finding_is_sensitive.java @@ -0,0 +1,30 @@ +package io.openaev.migration; + +import java.sql.Statement; +import org.flywaydb.core.api.migration.BaseJavaMigration; +import org.flywaydb.core.api.migration.Context; +import org.springframework.stereotype.Component; + +/** Flags findings whose value holds sensitive material, so the API can redact them. */ +@Component +public class V6_20260824180000000__Add_finding_is_sensitive extends BaseJavaMigration { + + @Override + public void migrate(Context context) throws Exception { + try (Statement statement = context.getConnection().createStatement()) { + statement.execute( + """ + ALTER TABLE findings + ADD COLUMN IF NOT EXISTS finding_is_sensitive BOOLEAN NOT NULL DEFAULT FALSE; + """); + + statement.execute( + """ + UPDATE findings + SET finding_is_sensitive = TRUE + WHERE finding_type = 'Credentials' + AND finding_is_sensitive IS FALSE; + """); + } + } +} diff --git a/openaev-api/src/main/java/io/openaev/output_processor/CredentialsOutputProcessor.java b/openaev-api/src/main/java/io/openaev/output_processor/CredentialsOutputProcessor.java index c4c64b1c12b..570aa2b2ca4 100644 --- a/openaev-api/src/main/java/io/openaev/output_processor/CredentialsOutputProcessor.java +++ b/openaev-api/src/main/java/io/openaev/output_processor/CredentialsOutputProcessor.java @@ -29,7 +29,8 @@ public CredentialsOutputProcessor(FindingService findingService) { new ContractOutputField(PASSWORD, ContractOutputTechnicalType.Text, false), new ContractOutputField(HASH, ContractOutputTechnicalType.Text, false), new ContractOutputField(HOST, ContractOutputTechnicalType.Text, false)), - findingService); + findingService, + true); } @Override diff --git a/openaev-api/src/main/java/io/openaev/output_processor/FindingCapableOutputProcessor.java b/openaev-api/src/main/java/io/openaev/output_processor/FindingCapableOutputProcessor.java index f60c8a91c38..3cdd3f7843c 100644 --- a/openaev-api/src/main/java/io/openaev/output_processor/FindingCapableOutputProcessor.java +++ b/openaev-api/src/main/java/io/openaev/output_processor/FindingCapableOutputProcessor.java @@ -9,6 +9,7 @@ import io.openaev.rest.inject.service.ExecutionProcessingContext; import java.util.Collections; import java.util.List; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; /** Abstract base class for output processors that are capable of generating findings. */ @@ -17,13 +18,26 @@ public abstract class FindingCapableOutputProcessor extends AbstractOutputProces protected final FindingService findingService; + /** Whether the findings produced by this processor hold sensitive material. */ + @Getter private final boolean sensitive; + protected FindingCapableOutputProcessor( ContractOutputType type, ContractOutputTechnicalType technicalType, List fields, FindingService findingService) { + this(type, technicalType, fields, findingService, false); + } + + protected FindingCapableOutputProcessor( + ContractOutputType type, + ContractOutputTechnicalType technicalType, + List fields, + FindingService findingService, + boolean sensitive) { super(type, technicalType, fields); this.findingService = findingService; + this.sensitive = sensitive; } /** @@ -44,7 +58,8 @@ public final void process( this::toFindingValue, this::toFindingAssets, this::toFindingTeams, - this::toFindingUsers); + this::toFindingUsers, + this.sensitive); afterFindings(executionContext, structuredOutputNode); } diff --git a/openaev-api/src/main/java/io/openaev/rest/finding/FindingApi.java b/openaev-api/src/main/java/io/openaev/rest/finding/FindingApi.java index 742b2a961e6..7a9d9e951f6 100644 --- a/openaev-api/src/main/java/io/openaev/rest/finding/FindingApi.java +++ b/openaev-api/src/main/java/io/openaev/rest/finding/FindingApi.java @@ -28,13 +28,13 @@ public class FindingApi extends RestBehavior { // -- CRUD -- @GetMapping({FINDING_URI + "/{id}", TENANT_FINDING_URI + "/{id}"}) - @Transactional + @Transactional(readOnly = true) @AccessControl( resourceId = "#id", actionPerformed = Action.READ, resourceType = ResourceType.FINDING) public ResponseEntity finding(@PathVariable @NotNull final String id) { - return ResponseEntity.ok(this.findingService.finding(id)); + return ResponseEntity.ok(this.findingService.redactValue(this.findingService.finding(id))); } @GetMapping({FINDING_URI + "/{id}/summary", TENANT_FINDING_URI + "/{id}/summary"}) diff --git a/openaev-api/src/main/java/io/openaev/rest/finding/FindingService.java b/openaev-api/src/main/java/io/openaev/rest/finding/FindingService.java index ef5c7a7ad29..1257a3c84d6 100644 --- a/openaev-api/src/main/java/io/openaev/rest/finding/FindingService.java +++ b/openaev-api/src/main/java/io/openaev/rest/finding/FindingService.java @@ -19,6 +19,7 @@ import java.util.*; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -31,6 +32,14 @@ public class FindingService { private static final String HOST = "host"; + + /** Mask substituted to the secret part of a sensitive finding value. */ + public static final String MASK = "******"; + + private static final char PART_SEPARATOR = ':'; + private static final int MASKING_VISIBLE_FRAGMENT_LENGTH = 2; + private static final int MASKING_MIN_LENGTH_FOR_FRAGMENT = 5; + private final InjectService injectService; private final FindingRepository findingRepository; @@ -39,6 +48,57 @@ public class FindingService { private final TeamRepository teamRepository; private final UserRepository userRepository; + // -- REDACTION -- + + /** + * Redacts the value of a sensitive finding before it leaves the platform through the API. The + * database row keeps the full cleartext value (it is needed for deduplication, correlation and + * attack path computation): only the returned representation is masked. + * + *

Every part of the value - the parts being separated by {@code :} - is masked the same way: a + * two character fragment is kept so an operator can still tell WHICH secret was discovered when + * the value is already known to them, without the API ever disclosing it. A part too short to + * keep a fragment without disclosing most of it is masked entirely. + * + *

    + *
  • {@code admin:motdepasse} becomes {@code ad******:mo******} + *
  • {@code Sup3rS3cret} becomes {@code Su******} + *
  • {@code abcd} becomes {@code ******} + *
+ * + * @param value the cleartext finding value + * @param sensitive whether the finding holds sensitive material + * @return the value as-is when the finding is not sensitive, its redacted form otherwise + */ + public static String redact(final String value, final boolean sensitive) { + if (!sensitive || value == null || value.isBlank()) { + return value; + } + + return Arrays.stream(value.split(String.valueOf(PART_SEPARATOR), -1)) + .map(FindingService::maskPart) + .collect(Collectors.joining(String.valueOf(PART_SEPARATOR))); + } + + private static String maskPart(final String part) { + if (part.length() < MASKING_MIN_LENGTH_FOR_FRAGMENT) { + return MASK; + } + return part.substring(0, MASKING_VISIBLE_FRAGMENT_LENGTH) + MASK; + } + + /** + * Redacts in place the value of a finding entity before the API serializes it. The caller must + * run in a read only transaction, so the masked value is never flushed to the database: the row + * keeps the cleartext value. + */ + public Finding redactValue(@NotNull final Finding finding) { + if (finding.isSensitive()) { + finding.setValue(redact(finding.getValue(), true)); + } + return finding; + } + // -- CRUD -- public List findings() { @@ -69,7 +129,8 @@ public FindingSummaryOutput findingSummary(@NotNull final String id) { return FindingSummaryOutput.builder() .id(finding.getId()) .type(type) - .value(value) + .value(redact(value, finding.isSensitive())) + .sensitive(finding.isSensitive()) .firstSeen(seen != null ? seen.getFirstSeen() : finding.getCreationDate()) .lastSeen(seen != null ? seen.getLastSeen() : finding.getUpdateDate()) .occurrences(seen != null ? seen.getOccurrences() : 1) @@ -118,6 +179,8 @@ public void deleteFinding(@NotNull final String id) { * node (used for injector findings). * @param teamExtractor A function to extract associated team IDs for each finding from the JSON * node (used for injector findings). + * @param sensitive Whether the findings produced by this processor hold sensitive material and + * must be redacted when serialized by the API. */ public void generateFindings( ExecutionProcessingContext executionContext, @@ -127,7 +190,8 @@ public void generateFindings( Function valueExtractor, Function> assetExtractor, Function> userExtractor, - Function> teamExtractor) { + Function> teamExtractor, + boolean sensitive) { if (executionContext.isAgentExecution()) { processAgentFindings( @@ -137,7 +201,8 @@ public void generateFindings( contractOutputContext, executionContext.valueTargetedAssetsMap(), validator, - valueExtractor); + valueExtractor, + sensitive); } else { processInjectorFindings( structuredOutputNode, @@ -147,7 +212,8 @@ public void generateFindings( valueExtractor, assetExtractor, userExtractor, - teamExtractor); + teamExtractor, + sensitive); } } @@ -158,7 +224,8 @@ public void processAgentFindings( ContractOutputContext contractOutputContext, Map valueTargetedAssetsMap, Predicate validator, - Function valueExtractor) { + Function valueExtractor, + boolean sensitive) { if (structuredOutputNode == null || !structuredOutputNode.isArray()) { log.debug("Skipping agent findings: structuredOutputNode is null or not an array"); @@ -176,13 +243,21 @@ public void processAgentFindings( .ifPresentOrElse( asset -> saveAgentFinding( - inject, asset, contractOutputContext, valueExtractor.apply(jsonNode)), + inject, + asset, + contractOutputContext, + valueExtractor.apply(jsonNode), + sensitive), () -> log.warn("Finding dropped: No asset match for host in {}", jsonNode)); } } public void saveAgentFinding( - Inject inject, Asset asset, ContractOutputContext contractOutputContext, String value) { + Inject inject, + Asset asset, + ContractOutputContext contractOutputContext, + String value, + boolean sensitive) { findingWriter.saveCompleteFinding( contractOutputContext.key(), @@ -193,6 +268,7 @@ public void saveAgentFinding( contractOutputContext.name(), asset.getId(), contractOutputContext.tagIds(), + sensitive, inject.getTenant() != null ? inject.getTenant().getId() : null); } @@ -217,7 +293,8 @@ public void processInjectorFindings( Function valueExtractor, Function> assetExtractor, Function> userExtractor, - Function> teamExtractor) { + Function> teamExtractor, + boolean sensitive) { if (structuredOutputNode == null) { log.debug("Skipping injector findings: structuredOutputNode is null"); @@ -232,7 +309,8 @@ public void processInjectorFindings( valueExtractor, assetExtractor, userExtractor, - teamExtractor); + teamExtractor, + sensitive); createFindings(findings, inject.getId()); } @@ -321,7 +399,8 @@ public List buildFindings( Function valueExtractor, Function> assetExtractor, Function> userExtractor, - Function> teamExtractor) { + Function> teamExtractor, + boolean sensitive) { if (contractOutputContext.isMultiple() && structuredOutputNode.isArray()) { List findings = new ArrayList<>(); @@ -341,7 +420,8 @@ public List buildFindings( valueExtractor, assetExtractor, userExtractor, - teamExtractor)); + teamExtractor, + sensitive)); } return findings; } @@ -354,7 +434,8 @@ public List buildFindings( valueExtractor, assetExtractor, userExtractor, - teamExtractor)); + teamExtractor, + sensitive)); } private Finding buildSingleFinding( @@ -364,7 +445,8 @@ private Finding buildSingleFinding( Function valueExtractor, Function> assetExtractor, Function> userExtractor, - Function> teamExtractor) { + Function> teamExtractor, + boolean sensitive) { if (!validator.test(structuredOutputNode)) { throw new IllegalArgumentException( @@ -373,6 +455,7 @@ private Finding buildSingleFinding( Finding finding = FindingUtils.createFinding(contractOutputContext); finding.setValue(valueExtractor.apply(structuredOutputNode)); + finding.setSensitive(sensitive); return linkFinding(structuredOutputNode, finding, assetExtractor, userExtractor, teamExtractor); } diff --git a/openaev-api/src/main/java/io/openaev/rest/finding/FindingWriter.java b/openaev-api/src/main/java/io/openaev/rest/finding/FindingWriter.java index 4fe728af7a9..f7a4cc09407 100644 --- a/openaev-api/src/main/java/io/openaev/rest/finding/FindingWriter.java +++ b/openaev-api/src/main/java/io/openaev/rest/finding/FindingWriter.java @@ -29,10 +29,18 @@ public void saveCompleteFinding( String name, String assetId, String[] tagIds, + boolean sensitive, String tenantId) { String findingId = findingRepository.upsertFinding( - findingField, findingType, findingValue, findingLabels, injectId, name, tenantId); + findingField, + findingType, + findingValue, + findingLabels, + injectId, + name, + sensitive, + tenantId); findingRepository.insertFindingAsset(findingId, assetId); findingRepository.insertFindingTags(findingId, tagIds); } diff --git a/openaev-api/src/main/java/io/openaev/rest/finding/form/AggregatedFindingOutput.java b/openaev-api/src/main/java/io/openaev/rest/finding/form/AggregatedFindingOutput.java index 5c6a339ef9e..80340703e43 100644 --- a/openaev-api/src/main/java/io/openaev/rest/finding/form/AggregatedFindingOutput.java +++ b/openaev-api/src/main/java/io/openaev/rest/finding/form/AggregatedFindingOutput.java @@ -32,11 +32,18 @@ public class AggregatedFindingOutput { @NotNull private ContractOutputType type; - @Schema(description = "Finding Value") + @Schema( + description = + "Finding value. Redacted when the finding is sensitive: the API never discloses the" + + " cleartext value of a sensitive finding.") @JsonProperty("finding_value") @NotBlank private String value; + @Schema(description = "Whether the finding holds sensitive material, hence a redacted value") + @JsonProperty("finding_is_sensitive") + private boolean sensitive; + @Schema(description = "First time the finding was seen") @JsonProperty("finding_created_at") @NotNull diff --git a/openaev-api/src/main/java/io/openaev/rest/finding/form/FindingSummaryOutput.java b/openaev-api/src/main/java/io/openaev/rest/finding/form/FindingSummaryOutput.java index af16323d3e2..f367a404b8f 100644 --- a/openaev-api/src/main/java/io/openaev/rest/finding/form/FindingSummaryOutput.java +++ b/openaev-api/src/main/java/io/openaev/rest/finding/form/FindingSummaryOutput.java @@ -31,10 +31,14 @@ public class FindingSummaryOutput { @NotNull private ContractOutputType type; - @Schema(description = "Finding value") + @Schema(description = "Finding value, redacted when the finding is sensitive") @JsonProperty("finding_value") private String value; + @Schema(description = "Whether the finding holds sensitive material, hence a redacted value") + @JsonProperty("finding_is_sensitive") + private boolean sensitive; + @Schema(description = "First time this finding was seen across all occurrences") @JsonProperty("finding_first_seen") private Instant firstSeen; diff --git a/openaev-api/src/main/java/io/openaev/utils/mapper/FindingMapper.java b/openaev-api/src/main/java/io/openaev/utils/mapper/FindingMapper.java index 60247cf86f4..fc9eda3d46e 100644 --- a/openaev-api/src/main/java/io/openaev/utils/mapper/FindingMapper.java +++ b/openaev-api/src/main/java/io/openaev/utils/mapper/FindingMapper.java @@ -3,6 +3,7 @@ import io.openaev.database.model.*; import io.openaev.database.repository.FindingRepository; import io.openaev.rest.atomic_testing.form.TargetSimple; +import io.openaev.rest.finding.FindingService; import io.openaev.rest.finding.form.AggregatedFindingOutput; import io.openaev.rest.finding.form.RelatedFindingOutput; import io.openaev.utils.TargetType; @@ -43,7 +44,8 @@ public AggregatedFindingOutput toAggregatedFindingOutput( Finding finding, List relatedAssets, Instant firstSeen, Instant lastSeen) { return AggregatedFindingOutput.builder() .id(finding.getId()) - .value(finding.getValue()) + .value(FindingService.redact(finding.getValue(), finding.isSensitive())) + .sensitive(finding.isSensitive()) .type(finding.getType()) .creationDate(firstSeen) .updateDate(lastSeen) @@ -59,7 +61,8 @@ public AggregatedFindingOutput toAggregatedFindingOutput( public RelatedFindingOutput toRelatedFindingOutput(Finding finding) { return RelatedFindingOutput.builder() .id(finding.getId()) - .value(finding.getValue()) + .value(FindingService.redact(finding.getValue(), finding.isSensitive())) + .sensitive(finding.isSensitive()) .type(finding.getType()) .updateDate(finding.getUpdateDate()) .assets( diff --git a/openaev-api/src/test/java/io/openaev/output_processor/CredentialsOutputProcessorTest.java b/openaev-api/src/test/java/io/openaev/output_processor/CredentialsOutputProcessorTest.java index c64d1aa5012..1de7714251f 100644 --- a/openaev-api/src/test/java/io/openaev/output_processor/CredentialsOutputProcessorTest.java +++ b/openaev-api/src/test/java/io/openaev/output_processor/CredentialsOutputProcessorTest.java @@ -16,6 +16,12 @@ class CredentialsOutputProcessorTest { new CredentialsOutputProcessor(findingService); private final ObjectMapper objectMapper = new ObjectMapper(); + @Test + @DisplayName("Should be flagged sensitive so the API redacts the value") + void shouldBeFlaggedSensitive() { + assertTrue(processor.isSensitive()); + } + @Test @DisplayName("Should return true when both username and password are present") void shouldReturnTrueWhenBothUsernameAndPasswordPresent() throws Exception { diff --git a/openaev-api/src/test/java/io/openaev/output_processor/OutputProcessorIntegrationTest.java b/openaev-api/src/test/java/io/openaev/output_processor/OutputProcessorIntegrationTest.java index 5a185e37817..77f7dcafc46 100644 --- a/openaev-api/src/test/java/io/openaev/output_processor/OutputProcessorIntegrationTest.java +++ b/openaev-api/src/test/java/io/openaev/output_processor/OutputProcessorIntegrationTest.java @@ -45,6 +45,24 @@ void shouldReturnCorrectHandlerForEachType() { .isInstanceOf(SignatureOutputProcessor.class); } + @Test + @DisplayName("Should flag credentials as the only sensitive finding type") + void given_everyFindingCapableProcessor_should_flagOnlyCredentialsAsSensitive() { + // Processors default to "not sensitive" and only the ones producing secrets opt in, so this + // assertion is the single place where the whole type - sensitivity matrix is stated: a new + // finding type that should be redacted has to be declared here as well. + for (ContractOutputType type : ContractOutputType.values()) { + OutputProcessor processor = registry.getProcessor(type).get(); + if (!(processor instanceof FindingCapableOutputProcessor findingProcessor)) { + continue; + } + + assertThat(findingProcessor.isSensitive()) + .withFailMessage("Unexpected sensitivity for type: " + type) + .isEqualTo(type == ContractOutputType.Credentials); + } + } + @Test @DisplayName("Should return same instance on multiple calls to getProcessor") void shouldReturnSameInstanceOnMultipleCalls() { diff --git a/openaev-api/src/test/java/io/openaev/rest/finding/FindingApiTest.java b/openaev-api/src/test/java/io/openaev/rest/finding/FindingApiTest.java index efd4410e264..37e3947df5b 100644 --- a/openaev-api/src/test/java/io/openaev/rest/finding/FindingApiTest.java +++ b/openaev-api/src/test/java/io/openaev/rest/finding/FindingApiTest.java @@ -1,6 +1,7 @@ package io.openaev.rest.finding; import static io.openaev.helper.StreamHelper.fromIterable; +import static io.openaev.rest.finding.FindingService.MASK; import static io.openaev.utils.JsonTestUtils.asJsonString; import static io.openaev.utils.fixtures.FindingFixture.createDefaultTextFindingWithRandomValue; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; @@ -993,7 +994,9 @@ void should_return_findings_by_scenario() throws Exception { jsonPath("$.content.[0].finding_scenario.scenario_id").value(savedScenario.getId())) .andExpect( jsonPath("$.content.[0].finding_type").value(savedFinding.getType().getLabel())) - .andExpect(jsonPath("$.content.[0].finding_value").value("admin:admin")); + // Credentials are sensitive: the API never returns the cleartext value. + .andExpect(jsonPath("$.content.[0].finding_value").value("ad" + MASK + ":ad" + MASK)) + .andExpect(jsonPath("$.content.[0].finding_is_sensitive").value(true)); } @Test @@ -1184,6 +1187,88 @@ void distinctList_groupSurvivesFilterMatchingOnlyOlderOccurrence() { assertThat(page.getContent().getFirst().getId()).isEqualTo(olderA.getId()); } + @Nested + @DisplayName("When the finding is sensitive") + class WhenTheFindingIsSensitive { + + private Finding persistSensitiveFinding() { + Finding finding = + findingComposer + .forFinding(FindingFixture.createDefaultFindingCredentials()) + .withEndpoint(endpointComposer.forEndpoint(savedEndpoint)) + .withInject(injectWrapper) + .persist() + .get(); + entityManager.flush(); + entityManager.clear(); + return finding; + } + + @Test + @DisplayName("Should redact the value when reading the finding") + void given_aSensitiveFinding_should_redactTheValueOnRead() throws Exception { + // -------- Arrange -------- + Finding finding = persistSensitiveFinding(); + + // -------- Act & Assert -------- + mvc.perform(get(FINDING_URI + "/" + finding.getId()).with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.finding_value").value("ad" + MASK + ":ad" + MASK)) + .andExpect(jsonPath("$.finding_is_sensitive").value(true)); + } + + @Test + @DisplayName("Should redact the value in the finding summary") + void given_aSensitiveFinding_should_redactTheValueInTheSummary() throws Exception { + // -------- Arrange -------- + Finding finding = persistSensitiveFinding(); + + // -------- Act & Assert -------- + mvc.perform(get(FINDING_URI + "/" + finding.getId() + "/summary").with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.finding_value").value("ad" + MASK + ":ad" + MASK)) + .andExpect(jsonPath("$.finding_is_sensitive").value(true)); + } + + @Test + @DisplayName("Should keep the cleartext value in database") + void given_aSensitiveFinding_should_keepTheCleartextValueInDatabase() { + // -------- Arrange -------- + Finding finding = persistSensitiveFinding(); + + // -------- Act -------- + Object storedValue = + entityManager + .createNativeQuery("SELECT finding_value FROM findings WHERE finding_id = :id") + .setParameter("id", finding.getId()) + .getSingleResult(); + + // -------- Assert -------- + assertThat(storedValue).isEqualTo("admin:admin"); + } + + @Test + @DisplayName("Should leave the value of a non sensitive finding untouched") + void given_aNonSensitiveFinding_should_notRedactTheValue() throws Exception { + // -------- Arrange -------- + Finding finding = + findingComposer + .forFinding(FindingFixture.createDefaultTextFinding()) + .withEndpoint(endpointComposer.forEndpoint(savedEndpoint)) + .withInject(injectWrapper) + .persist() + .get(); + entityManager.flush(); + entityManager.clear(); + + // -------- Act & Assert -------- + mvc.perform(get(FINDING_URI + "/" + finding.getId()).with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.finding_value").value("text_value")) + .andExpect(jsonPath("$.finding_is_sensitive").value(false)); + } + } + private void setFindingDates(String findingId, Instant createdAt, Instant updatedAt) { entityManager .createNativeQuery( diff --git a/openaev-api/src/test/java/io/openaev/rest/finding/FindingServiceTest.java b/openaev-api/src/test/java/io/openaev/rest/finding/FindingServiceTest.java index 5b69f3a8ca8..2d0edec6b93 100644 --- a/openaev-api/src/test/java/io/openaev/rest/finding/FindingServiceTest.java +++ b/openaev-api/src/test/java/io/openaev/rest/finding/FindingServiceTest.java @@ -61,7 +61,7 @@ void given_a_finding_already_existent_with_one_asset_should_have_two_assets() { injectTestHelper.forceSaveInject(inject); injectTestHelper.forceSaveFinding(existing); - findingService.saveAgentFinding(inject, asset2, contractOutputContext, value); + findingService.saveAgentFinding(inject, asset2, contractOutputContext, value, false); Finding result = findingRepository @@ -98,7 +98,7 @@ void given_a_finding_already_existent_with_same_asset_should_have_one_asset() { injectTestHelper.forceSaveInject(inject); injectTestHelper.forceSaveFinding(existing); - findingService.saveAgentFinding(inject, asset1, contractOutputContext, value); + findingService.saveAgentFinding(inject, asset1, contractOutputContext, value, false); Finding result = findingRepository @@ -162,7 +162,8 @@ void shouldReturnFindingsForMultipleFindingCompatibleContractOutputs() throws Ex node -> node.get("id").asText(), node -> Collections.emptyList(), node -> Collections.emptyList(), - node -> Collections.emptyList()); + node -> Collections.emptyList(), + false); assertNotNull(findings); assertEquals(2, findings.size()); @@ -217,7 +218,8 @@ void shouldSkipMalformedFindingNodesInMultipleBatch() throws Exception { node -> node.get("port").asText(), node -> Collections.emptyList(), node -> Collections.emptyList(), - node -> Collections.emptyList()); + node -> Collections.emptyList(), + false); assertNotNull(findings); assertEquals(1, findings.size()); @@ -271,7 +273,8 @@ void shouldThrowExceptionWhenSingleFindingNotCorrectlyFormatted() throws Excepti node -> node.get("port").asText(), node -> Collections.emptyList(), node -> Collections.emptyList(), - node -> Collections.emptyList())); + node -> Collections.emptyList(), + false)); } @Nested diff --git a/openaev-api/src/test/java/io/openaev/rest/finding/FindingValueRedactionTest.java b/openaev-api/src/test/java/io/openaev/rest/finding/FindingValueRedactionTest.java new file mode 100644 index 00000000000..802fec50b6d --- /dev/null +++ b/openaev-api/src/test/java/io/openaev/rest/finding/FindingValueRedactionTest.java @@ -0,0 +1,86 @@ +package io.openaev.rest.finding; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +@DisplayName("Finding value redaction") +class FindingValueRedactionTest { + + @Nested + @DisplayName("When the finding is not sensitive") + class WhenNotSensitive { + + @Test + @DisplayName("Should return the value untouched") + void given_aNonSensitiveFinding_should_returnTheValueAsIs() { + // -------- Act -------- + String redacted = FindingService.redact("admin:Sup3rS3cret", false); + + // -------- Assert -------- + assertThat(redacted).isEqualTo("admin:Sup3rS3cret"); + } + } + + @Nested + @DisplayName("When the finding is sensitive") + class WhenSensitive { + + @Test + @DisplayName("Should mask each part of a credential value") + void given_aCredentialShapedValue_should_maskEveryPart() { + // -------- Act -------- + String redacted = FindingService.redact("admin:motdepasse", true); + + // -------- Assert -------- + assertThat(redacted).isEqualTo("ad" + FindingService.MASK + ":mo" + FindingService.MASK); + assertThat(redacted).doesNotContain("motdepasse"); + } + + @Test + @DisplayName("Should mask every part of a value holding several separators") + void given_aValueWithSeveralSeparators_should_maskEveryPart() { + // -------- Act -------- + String redacted = FindingService.redact("admin:aad3b435:31d6cfe0", true); + + // -------- Assert -------- + assertThat(redacted) + .isEqualTo( + "ad" + + FindingService.MASK + + ":aa" + + FindingService.MASK + + ":31" + + FindingService.MASK); + } + + @Test + @DisplayName("Should keep a two character fragment of a value without separator") + void given_aValueWithoutSeparator_should_keepAFragment() { + // -------- Act -------- + String redacted = FindingService.redact("Sup3rS3cret", true); + + // -------- Assert -------- + assertThat(redacted).isEqualTo("Su" + FindingService.MASK); + } + + @Test + @DisplayName("Should mask a short part entirely") + void given_aShortPart_should_maskItEntirely() { + // -------- Act & Assert -------- + assertThat(FindingService.redact("abcd", true)).isEqualTo(FindingService.MASK); + assertThat(FindingService.redact("administrator:pwd", true)) + .isEqualTo("ad" + FindingService.MASK + ":" + FindingService.MASK); + } + + @Test + @DisplayName("Should return blank and null values as is") + void given_aBlankValue_should_returnItAsIs() { + // -------- Act & Assert -------- + assertThat(FindingService.redact(null, true)).isNull(); + assertThat(FindingService.redact(" ", true)).isEqualTo(" "); + } + } +} diff --git a/openaev-api/src/test/java/io/openaev/utils/fixtures/FindingFixture.java b/openaev-api/src/test/java/io/openaev/utils/fixtures/FindingFixture.java index 57f42ade8b1..b1e42a7479b 100644 --- a/openaev-api/src/test/java/io/openaev/utils/fixtures/FindingFixture.java +++ b/openaev-api/src/test/java/io/openaev/utils/fixtures/FindingFixture.java @@ -54,6 +54,8 @@ public static Finding createDefaultFindingCredentials() { finding.setName("Credentials"); finding.setField(CREDENTIALS_FIELD); finding.setValue("admin:admin"); + // Credentials are flagged sensitive by their output processor. + finding.setSensitive(true); return finding; } } diff --git a/openaev-front/src/utils/api-types.d.ts b/openaev-front/src/utils/api-types.d.ts index 8e5a52f6b84..9aa5a9dd8fe 100644 --- a/openaev-front/src/utils/api-types.d.ts +++ b/openaev-front/src/utils/api-types.d.ts @@ -195,6 +195,8 @@ export interface AggregatedFindingOutput { * @minLength 1 */ finding_id: string; + /** Whether the finding holds sensitive material, hence a redacted value */ + finding_is_sensitive?: boolean; /** * Represents the data type being extracted. * @example "text, number, port, portscan, ipv4, ipv6, credentials, cve" @@ -230,7 +232,7 @@ export interface AggregatedFindingOutput { */ finding_updated_at: string; /** - * Finding Value + * Finding value. Redacted when the finding is sensitive: the API never discloses the cleartext value of a sensitive finding. * @minLength 1 */ finding_value: string; @@ -5555,6 +5557,8 @@ export interface Finding { /** @minLength 1 */ finding_id: string; finding_inject_id?: string; + /** Whether the finding value holds sensitive material and is redacted by API */ + finding_is_sensitive?: boolean; /** @deprecated */ finding_labels?: string[]; finding_name?: string; @@ -5647,6 +5651,8 @@ export interface FindingSummaryOutput { finding_first_seen?: string; /** Representative finding id used to resolve the (type, value) group */ finding_id?: string; + /** Whether the finding holds sensitive material, hence a redacted value */ + finding_is_sensitive?: boolean; /** * Last time this finding was seen across all occurrences * @format date-time @@ -5693,7 +5699,7 @@ export interface FindingSummaryOutput { * @format int64 */ finding_users_count?: number; - /** Finding value */ + /** Finding value, redacted when the finding is sensitive */ finding_value?: string; } @@ -9993,6 +9999,8 @@ export interface RelatedFindingOutput { finding_id: string; /** Inject linked to finding */ finding_inject: InjectSimple; + /** Whether the finding holds sensitive material, hence a redacted value */ + finding_is_sensitive?: boolean; /** Scenario linked to inject */ finding_scenario?: ScenarioSimple; /** Simulation linked to inject */ @@ -10042,7 +10050,7 @@ export interface RelatedFindingOutput { */ finding_users?: TargetSimple[]; /** - * Finding Value + * Finding value. Redacted when the finding is sensitive: the API never discloses the cleartext value of a sensitive finding. * @minLength 1 */ finding_value: string; diff --git a/openaev-model/src/main/java/io/openaev/database/model/Finding.java b/openaev-model/src/main/java/io/openaev/database/model/Finding.java index 99b2f143a00..e801c6510b7 100644 --- a/openaev-model/src/main/java/io/openaev/database/model/Finding.java +++ b/openaev-model/src/main/java/io/openaev/database/model/Finding.java @@ -52,12 +52,23 @@ public class Finding implements TenantBase { @NotNull protected ContractOutputType type; + /** + * Cleartext value as detected. It is persisted as-is (deduplication, correlation and attack paths + * rely on it), but a sensitive finding is never disclosed through the API: the API layer redacts + * it before returning the finding. + */ @Queryable(searchable = true, filterable = true, sortable = true) @Column(name = "finding_value", nullable = false) @JsonProperty("finding_value") @NotBlank protected String value; + @Queryable(filterable = true, sortable = true, label = "sensitive") + @Column(name = "finding_is_sensitive", nullable = false) + @JsonProperty("finding_is_sensitive") + @Schema(description = "Whether the finding value holds sensitive material and is redacted by API") + private boolean sensitive = false; + @Deprecated @Type(StringArrayType.class) @Column(name = "finding_labels", columnDefinition = "text[]") diff --git a/openaev-model/src/main/java/io/openaev/database/repository/FindingRepository.java b/openaev-model/src/main/java/io/openaev/database/repository/FindingRepository.java index 503e6095262..53a8857a175 100644 --- a/openaev-model/src/main/java/io/openaev/database/repository/FindingRepository.java +++ b/openaev-model/src/main/java/io/openaev/database/repository/FindingRepository.java @@ -145,12 +145,14 @@ Optional findByInjectIdAndValueAndTypeAndKey( """ INSERT INTO findings (finding_id, finding_field, finding_type, finding_value, - finding_labels, finding_inject_id, finding_name, tenant_id) + finding_labels, finding_inject_id, finding_name, finding_is_sensitive, tenant_id) VALUES (gen_random_uuid(), :findingField, :findingType, :findingValue, - :findingLabels, :findingInjectId, :findingName, :tenantId) + :findingLabels, :findingInjectId, :findingName, :findingIsSensitive, :tenantId) ON CONFLICT (finding_inject_id, finding_field, finding_type, finding_value) - DO UPDATE SET finding_name = EXCLUDED.finding_name, finding_updated_at = now() + DO UPDATE SET finding_name = EXCLUDED.finding_name, + finding_is_sensitive = EXCLUDED.finding_is_sensitive, + finding_updated_at = now() RETURNING finding_id """, nativeQuery = true) @@ -161,6 +163,7 @@ String upsertFinding( @Param("findingLabels") String[] findingLabels, @Param("findingInjectId") String injectId, @Param("findingName") String name, + @Param("findingIsSensitive") boolean sensitive, @Param("tenantId") String tenantId); @Modifying