Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule field validation during IndexRule Action #80

Merged
merged 18 commits into from
Nov 3, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,31 @@ public static List<String> validateIndexMappings(ImmutableOpenMap<String, Mappin
.collect(Collectors.toList());
}

/**
* Traverses mappings tree and collects all fields.
* Nested fields are flattened.
* @return list of fields in mappings.
*/
public static List<String> extractAllFieldsFlat(MappingMetadata mappingMetadata) {
MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
List<String> flatProperties = new ArrayList<>();
// Setup
mappingsTraverser.addListener(new MappingsTraverser.MappingsTraverserListener() {
@Override
public void onLeafVisited(MappingsTraverser.Node node) {
flatProperties.add(node.currentPath);
}

@Override
public void onError(String error) {
throw new IllegalArgumentException(error);
}
});
// Do traverse
mappingsTraverser.traverse();
return flatProperties;
}

public static List<String> getAllNonAliasFieldsFromIndex(MappingMetadata mappingMetadata) {
MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
return mappingsTraverser.extractFlatNonAliasFields();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class Detector implements Writeable, ToXContentObject {
private static final String FINDINGS_INDEX = "findings_index";
private static final String FINDINGS_INDEX_PATTERN = "findings_index_pattern";

public static final String DETECTORS_INDEX = ".opensearch-detectors-config";
public static final String DETECTORS_INDEX = ".opensearch-sap-detectors-config";

public static final NamedXContentRegistry.Entry XCONTENT_REGISTRY = new NamedXContentRegistry.Entry(
Detector.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public class Rule implements Writeable, ToXContentObject {
private static final String QUERIES = "queries";
public static final String RULE = "rule";

public static final String PRE_PACKAGED_RULES_INDEX = ".opensearch-pre-packaged-rules-config";
public static final String CUSTOM_RULES_INDEX = ".opensearch-custom-rules-config";
public static final String PRE_PACKAGED_RULES_INDEX = ".opensearch-sap-pre-packaged-rules-config";
public static final String CUSTOM_RULES_INDEX = ".opensearch-sap-custom-rules-config";

public static final NamedXContentRegistry.Entry XCONTENT_REGISTRY = new NamedXContentRegistry.Entry(
Rule.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@
*/
package org.opensearch.securityanalytics.transport;

import java.util.HashMap;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.search.join.ScoreMode;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.ActionListener;
import org.opensearch.action.ActionRunnable;
import org.opensearch.action.admin.indices.create.CreateIndexResponse;
import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.opensearch.action.index.IndexRequest;
import org.opensearch.action.index.IndexResponse;
import org.opensearch.action.search.SearchRequest;
Expand All @@ -19,6 +23,7 @@
import org.opensearch.action.support.HandledTransportAction;
import org.opensearch.action.support.master.AcknowledgedResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.MappingMetadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.Settings;
Expand All @@ -41,6 +46,8 @@
import org.opensearch.securityanalytics.action.IndexRuleAction;
import org.opensearch.securityanalytics.action.IndexRuleRequest;
import org.opensearch.securityanalytics.action.IndexRuleResponse;
import org.opensearch.securityanalytics.config.monitors.DetectorMonitorConfig;
import org.opensearch.securityanalytics.mapper.MapperUtils;
import org.opensearch.securityanalytics.model.Detector;
import org.opensearch.securityanalytics.model.Rule;
import org.opensearch.securityanalytics.rules.backend.OSQueryBackend;
Expand Down Expand Up @@ -181,8 +188,35 @@ void prepareRuleIndexing() {
final QueryBackend backend = new OSQueryBackend(category, true, true);
List<Object> queries = backend.convertRule(parsedRule);

Rule ruleDoc = new Rule(NO_ID, NO_VERSION, parsedRule, category, queries.stream().map(Object::toString).collect(Collectors.toList()), rule);
indexRule(ruleDoc);
//verify rule
verifyRule(backend.getQueryFields().keySet(), DetectorMonitorConfig.getRuleIndex(category), new ActionListener<>() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the custom rule is created using the fields not part of the rule alias mapping, but using some additional fields from the Source Index. That still is a valid use case.
Should we revisit the approach, where the fields in rule could also be present in the source index mapping?

@Override
public void onResponse(List<String> missingFields) {
try {
if (missingFields.size() > 0) {
onFailures(
new SecurityAnalyticsException(
"Rule is incompatible with ruleIndex. Unknown fields: [" +
String.join(",", missingFields) + "]",
RestStatus.INTERNAL_SERVER_ERROR,
new IllegalStateException()
)
);
return;
}
Rule ruleDoc = new Rule(NO_ID, NO_VERSION, parsedRule, category, queries.stream().map(Object::toString).collect(Collectors.toList()), rule);
indexRule(ruleDoc);
} catch (IOException e) {
onFailures(e);
}
}

@Override
public void onFailure(Exception e) {
onFailures(e);
}
});

} catch (IOException | SigmaError e) {
onFailures(e);
}
Expand Down Expand Up @@ -323,6 +357,36 @@ public void onFailure(Exception e) {
});
}

private void verifyRule(Set<String> ruleFields, String indexName, ActionListener<List<String>> listener) {

// Get index mappings
client.admin().indices().getMappings(new GetMappingsRequest().indices(indexName), new ActionListener<>() {
@Override
public void onResponse(GetMappingsResponse getMappingsResponse) {
Map<Rule, List<String>> result = new HashMap<>();
OSQueryBackend queryBackend = null;
try {
MappingMetadata mappingMetadata = getMappingsResponse.getMappings().iterator().next().value;
List<String> indexFields = MapperUtils.extractAllFieldsFlat(mappingMetadata);
// check if all rule fields are present in index fields
List<String> missingRuleFields = ruleFields
.stream()
.filter(e -> indexFields.contains(e) == false)
.collect(Collectors.toList());

listener.onResponse(missingRuleFields);
} catch (Exception e) {
listener.onFailure(SecurityAnalyticsException.wrap(e));
}
}

@Override
public void onFailure(Exception e) {
listener.onFailure(SecurityAnalyticsException.wrap(e));
}
});
}

private void onComplete(IndexResponse response, Rule rule, int target) {
if (checker.incrementAndGet() == target) {
onOperation(response, rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package org.opensearch.securityanalytics;

import java.io.File;
import java.nio.file.Path;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.entity.ContentType;
Expand All @@ -30,6 +32,7 @@
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentParser;
import org.opensearch.common.xcontent.XContentParserUtils;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.commons.alerting.model.ScheduledJob;
import org.opensearch.commons.alerting.util.IndexUtilsKt;
Expand All @@ -38,8 +41,11 @@
import org.opensearch.search.SearchHit;
import org.opensearch.securityanalytics.action.CreateIndexMappingsRequest;
import org.opensearch.securityanalytics.action.UpdateIndexMappingsRequest;
import org.opensearch.securityanalytics.config.monitors.DetectorMonitorConfig;
import org.opensearch.securityanalytics.model.Detector;
import org.opensearch.securityanalytics.model.Rule;
import org.opensearch.securityanalytics.util.DetectorIndices;
import org.opensearch.securityanalytics.util.RuleTopicIndices;
import org.opensearch.test.rest.OpenSearchRestTestCase;

import java.io.IOException;
Expand All @@ -52,9 +58,42 @@
import java.util.stream.Collectors;

import static org.opensearch.action.admin.indices.create.CreateIndexRequest.MAPPINGS;
import static org.opensearch.securityanalytics.util.RuleTopicIndices.ruleTopicIndexMappings;
import static org.opensearch.securityanalytics.util.RuleTopicIndices.ruleTopicIndexSettings;

public class SecurityAnalyticsRestTestCase extends OpenSearchRestTestCase {

protected void createRuleTopicIndex(String detectorType, String additionalMapping) throws IOException {

String mappings = "" +
" \"_meta\": {" +
" \"schema_version\": 1" +
" }," +
" \"properties\": {" +
" \"query\": {" +
" \"type\": \"percolator_ext\"" +
" }," +
" \"monitor_id\": {" +
" \"type\": \"text\"" +
" }," +
" \"index\": {" +
" \"type\": \"text\"" +
" }" +
" }";

String indexName = DetectorMonitorConfig.getRuleIndex(detectorType);
createIndex(
indexName,
Settings.builder().loadFromSource(ruleTopicIndexSettings(), XContentType.JSON).build(),
mappings
);
// Update mappings
if (additionalMapping != null) {
Response response = makeRequest(client(), "PUT", indexName + "/_mapping", Collections.emptyMap(), new StringEntity(additionalMapping), new BasicHeader("Content-Type", "application/json"));
assertEquals(RestStatus.OK, restStatus(response));
}
}

protected String createTestIndex(String index, String mapping) throws IOException {
createTestIndex(index, mapping, Settings.EMPTY);
return index;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.junit.Assert;
import org.opensearch.client.Request;
import org.opensearch.client.Response;
import org.opensearch.common.settings.Settings;
import org.opensearch.commons.alerting.model.action.Action;
import org.opensearch.rest.RestStatus;
import org.opensearch.search.SearchHit;
Expand All @@ -31,6 +32,8 @@
import org.opensearch.securityanalytics.model.DetectorInput;
import org.opensearch.securityanalytics.model.DetectorRule;
import org.opensearch.securityanalytics.model.DetectorTrigger;
import org.opensearch.securityanalytics.util.RuleTopicIndices;


import static org.opensearch.securityanalytics.TestHelpers.netFlowMappings;
import static org.opensearch.securityanalytics.TestHelpers.randomAction;
Expand All @@ -45,6 +48,10 @@ public class AlertsIT extends SecurityAnalyticsRestTestCase {

@SuppressWarnings("unchecked")
public void testGetAlerts_success() throws IOException {

String fieldMapping = "{\"properties\": { \"event_uid\":{\"type\":\"long\"}}}";
createRuleTopicIndex(Detector.DetectorType.WINDOWS.getDetectorType(), fieldMapping);

String index = createTestIndex(randomIndex(), windowsIndexMapping());

String rule = randomRule();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ public void testSearchingDetectors() throws IOException {

@SuppressWarnings("unchecked")
public void testCreatingADetectorWithCustomRules() throws IOException {

String fieldMapping = "{\"properties\": { \"event_uid\":{\"type\":\"long\"}}}";
createRuleTopicIndex(Detector.DetectorType.WINDOWS.getDetectorType(), fieldMapping);

String index = createTestIndex(randomIndex(), windowsIndexMapping());

// Execute CreateMappingsAction to add alias mapping for index
Expand Down Expand Up @@ -225,6 +229,11 @@ public void testCreatingADetectorWithCustomRules() throws IOException {
}

public void testUpdateADetector() throws IOException {

String fieldMapping = "{\"properties\": { \"event_uid\":{\"type\":\"long\"}}}";
createRuleTopicIndex(Detector.DetectorType.WINDOWS.getDetectorType(), fieldMapping);


String index = createTestIndex(randomIndex(), windowsIndexMapping());

// Execute CreateMappingsAction to add alias mapping for index
Expand Down
Loading