Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
import org.elasticsearch.xpack.security.ingest.HashProcessor;
import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor;
import org.elasticsearch.xpack.security.rest.SecurityRestFilter;
import org.elasticsearch.xpack.security.rest.action.RestAuthenticateAction;
Expand Down Expand Up @@ -615,6 +616,10 @@ public static List<Setting<?>> getSettings(List<SecurityExtension> securityExten
// hide settings
settingsList.add(Setting.listSetting(SecurityField.setting("hide_settings"), Collections.emptyList(), Function.identity(),
Property.NodeScope, Property.Filtered));

// ingest processor settings
settingsList.add(HashProcessor.HMAC_KEY_SETTING);

return settingsList;
}

Expand Down Expand Up @@ -790,7 +795,11 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC

@Override
public Map<String, Processor.Factory> getProcessors(Processor.Parameters parameters) {
return Collections.singletonMap(SetSecurityUserProcessor.TYPE, new SetSecurityUserProcessor.Factory(parameters.threadContext));
Map<String, Processor.Factory> processors = new HashMap<>();
processors.put(SetSecurityUserProcessor.TYPE, new SetSecurityUserProcessor.Factory(parameters.threadContext));
processors.put(HashProcessor.TYPE, new HashProcessor.Factory(parameters.env.settings(),
parameters.ingestService.getClusterService()));
return processors;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.security.ingest;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.ConsistentSettingsService;
import org.elasticsearch.common.settings.SecureSetting;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.ingest.AbstractProcessor;
import org.elasticsearch.ingest.ConfigurationUtils;
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.xpack.core.security.SecurityField;

import javax.crypto.Mac;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException;

/**
* A processor that hashes the contents of a field or fields using various hashing algorithms
*/
public final class HashProcessor extends AbstractProcessor implements Consumer<ClusterState> {
public static final String TYPE = "hash";
public static final Setting.AffixSetting<SecureString> HMAC_KEY_SETTING = SecureSetting
.affixKeySetting(SecurityField.setting("ingest." + TYPE) + ".", "key",
(key) -> SecureSetting.secureString(key, null));

private final List<String> fields;
private final String targetField;
private final Method method;
private final Mac mac;
private final byte[] salt;
private final boolean ignoreMissing;
private final AtomicBoolean consistentHashes = new AtomicBoolean(true);

HashProcessor(String tag, List<String> fields, String targetField, byte[] salt, Method method, @Nullable Mac mac,
boolean ignoreMissing) {
super(tag);
this.fields = fields;
this.targetField = targetField;
this.method = method;
this.mac = mac;
this.salt = salt;
this.ignoreMissing = ignoreMissing;
}

List<String> getFields() {
return fields;
}

String getTargetField() {
return targetField;
}

byte[] getSalt() {
return salt;
}

@Override
public IngestDocument execute(IngestDocument document) {
if (consistentHashes.get()) {
Map<String, String> hashedFieldValues = fields.stream().map(f -> {
String value = document.getFieldValue(f, String.class, ignoreMissing);
if (value == null && ignoreMissing) {
return new Tuple<String, String>(null, null);
}
try {
return new Tuple<>(f, method.hash(mac, salt, value));
} catch (Exception e) {
throw new IllegalArgumentException("field[" + f + "] could not be hashed", e);
}
}).filter(tuple -> Objects.nonNull(tuple.v1())).collect(Collectors.toMap(Tuple::v1, Tuple::v2));
if (fields.size() == 1) {
document.setFieldValue(targetField, hashedFieldValues.values().iterator().next());
} else {
document.setFieldValue(targetField, hashedFieldValues);
}
return document;
} else {
throw new IllegalArgumentException("inconsistent hash key");
}
}

@Override
public String getType() {
return TYPE;
}

@Override
public void accept(ClusterState clusterState) {
// check hash keys for consistency and unset consistentHashes flag if inconsistent

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IngestService#addIngestClusterStateListener(...) should be invoked in the factory, otherwise this method will never be invoked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also in the enrich branch the enrich processor factory implements Consumer<ClusterState> and here the processor. This is a subtle difference, but processor instance are created when pipelines are created and discarded when a pipeline does not exist. However if in this case we discard this kind processor then we still keep a reference to this processor via the ingestClusterStateListeners list in ClusterService. In order to make this work the following changes should be made:

  • A IngestService#removeIngestClusterStateListener(...) should be added.
  • The innerUpdatePipelines(...) method in IngestService around line 569 should invoke close on processors that implement Closeable.
  • This processor should implement Closeable interface and then invoke IngestService#removeIngestClusterStateListener(this)

@danhermann danhermann Sep 25, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review comments, @martijnvg. What do you think about centralizing the consistency checking of settings in ConsistencySettingsService? Each hash processor would then verify the consistency of its own hash key with the service. That would eliminate the need to track the lifecycle of the processor and would also resolve the question above about failing the pipeline validation check on the elected master node. It might also have a performance benefit if multiple hash processors used the same key since the consistency check for all of them would happen only once.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, that makes sense. But then ConsistentSettingsService should a public component that should be made accessible in Processor.Parameters?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll make the changes so ConsistentSettingsService is accessible in Processor.Parameters and request another review pass then.

}

public static final class Factory implements Processor.Factory {

private final Settings settings;
private final ClusterService clusterService;
private final Map<String, SecureString> secureKeys;

public Factory(Settings settings, ClusterService clusterService) {
this.settings = settings;
this.clusterService = clusterService;
this.secureKeys = new HashMap<>();
HMAC_KEY_SETTING.getAllConcreteSettings(settings).forEach(k -> {
secureKeys.put(k.getKey(), k.get(settings));
});
}

private static Mac createMac(Method method, SecureString password, byte[] salt, int iterations) {
try {
SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance("PBKDF2With" + method.getAlgorithm());
PBEKeySpec keySpec = new PBEKeySpec(password.getChars(), salt, iterations, 128);
byte[] pbkdf2 = secretKeyFactory.generateSecret(keySpec).getEncoded();
Mac mac = Mac.getInstance(method.getAlgorithm());
mac.init(new SecretKeySpec(pbkdf2, method.getAlgorithm()));
return mac;
} catch (NoSuchAlgorithmException | InvalidKeySpecException | InvalidKeyException e) {
throw new IllegalArgumentException("invalid settings", e);
}
}

@Override
public HashProcessor create(Map<String, Processor.Factory> registry, String processorTag, Map<String, Object> config) {
boolean ignoreMissing = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "ignore_missing", false);
List<String> fields = ConfigurationUtils.readList(TYPE, processorTag, config, "fields");
if (fields.isEmpty()) {
throw ConfigurationUtils.newConfigurationException(TYPE, processorTag, "fields", "must specify at least one field");
} else if (fields.stream().anyMatch(Strings::isNullOrEmpty)) {
throw ConfigurationUtils.newConfigurationException(TYPE, processorTag, "fields",
"a field-name entry is either empty or null");
}
String targetField = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "target_field");
String keySettingName = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "key_setting");
SecureString key = secureKeys.get(keySettingName);
if (key == null) {
throw ConfigurationUtils.newConfigurationException(TYPE, processorTag, "key_setting",
"key [" + keySettingName + "] must match [xpack.security.ingest.hash.*.key]. It is not set");
}

Collection<Setting<?>> consistentSettings = HMAC_KEY_SETTING.getAllConcreteSettings(settings).collect(Collectors.toList());
ConsistentSettingsService consistentSettingsService = new ConsistentSettingsService(settings, clusterService, consistentSettings);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instantiating ConsistentSettingsService here seems unnecessarily heavy, especially since the state publishing capability of ConsistentSettingsService isn't useful here. I'd suggest refactoring ConsistentSettingsService to expose the logic for checking the consistency of settings in the areAllConsistent method and call that directly both here and in the accept(ClusterState) method above.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or ConsistentSettingsService should be made accessible in Processor.Parameters so that it can be used here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 to making it accessible via Processor.Parameters

...So it would look something like this:

(create consistent setting service in) Node -> IngestService -> Processor.Params , then pass that the ConsistentSettingsService service to this factory .

...however I don't think that is possible today because to instantiate a ConsistentSettingsService you need the secureSettings to check at time of construction...which you wont have in the Node object.

I haven't vetted this too deep .. but I think you can change

new ConsistentSettingsService(settings, clusterService, consistentSettings)
consistentSettingsService.areAllConsistent()

to

new ConsistentSettingsService(settings, clusterService)
consistentSettingsService.areAllConsistent(consistentSettings)

and then create the ConsistentSettingsService in Node, and pass it down to Processor.Params then pull the service from the params when creating the factory.

If this does work, can you please submit that change as a separate PR ?

if (consistentSettingsService.areAllConsistent() == false) {
throw ConfigurationUtils.newConfigurationException(TYPE, processorTag, "key_setting", "inconsistent hash key [" + keySettingName + "]");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this expected to fail often? I'm asking, because this method here is both used to validate the processor config on the elected master node and to instantiate the processor instance for a pipeline on all ingest nodes. In the latter case there is no good retry mechanism if this fails here (only if a next cluster state update comes, but that may take a while). So maybe we can fail only in the processor itself when it is detected via the accept(...) that settings are inconsistent? Also then we have a single place where this logic is applied.

}

String saltString = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "salt");
byte[] salt = saltString.getBytes(StandardCharsets.UTF_8);
String methodProperty = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "method", "SHA256");
Method method = Method.fromString(processorTag, "method", methodProperty);
int iterations = ConfigurationUtils.readIntProperty(TYPE, processorTag, config, "iterations", 5);
Mac mac = createMac(method, key, salt, iterations);
return new HashProcessor(processorTag, fields, targetField, salt, method, mac, ignoreMissing);
}

}

enum Method {
SHA1("HmacSHA1"),
SHA256("HmacSHA256"),
SHA384("HmacSHA384"),
SHA512("HmacSHA512");

private final String algorithm;

Method(String algorithm) {
this.algorithm = algorithm;
}

public String getAlgorithm() {
return algorithm;
}

@Override
public String toString() {
return name().toLowerCase(Locale.ROOT);
}

public String hash(Mac mac, byte[] salt, String input) {
try {
byte[] encrypted = mac.doFinal(input.getBytes(StandardCharsets.UTF_8));
byte[] messageWithSalt = new byte[salt.length + encrypted.length];
System.arraycopy(salt, 0, messageWithSalt, 0, salt.length);
System.arraycopy(encrypted, 0, messageWithSalt, salt.length, encrypted.length);
return Base64.getEncoder().encodeToString(messageWithSalt);
} catch (IllegalStateException e) {
throw new ElasticsearchException("error hashing data", e);
}
}

public static Method fromString(String processorTag, String propertyName, String type) {
try {
return Method.valueOf(type.toUpperCase(Locale.ROOT));
} catch(IllegalArgumentException e) {
throw newConfigurationException(TYPE, processorTag, propertyName, "type [" + type +
"] not supported, cannot convert field. Valid hash methods: " + Arrays.toString(Method.values()));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.security.ingest;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;

import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;

public class HashProcessorFactoryTests extends ESTestCase {

public void testProcessor() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList("_field"));
config.put("target_field", "_target");
config.put("salt", "_salt");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
for (HashProcessor.Method method : HashProcessor.Method.values()) {
config.put("method", method.toString());
HashProcessor processor = factory.create(null, "_tag", new HashMap<>(config));
assertThat(processor.getFields(), equalTo(Collections.singletonList("_field")));
assertThat(processor.getTargetField(), equalTo("_target"));
assertArrayEquals(processor.getSalt(), "_salt".getBytes(StandardCharsets.UTF_8));
}
}

public void testProcessorNoFields() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("target_field", "_target");
config.put("salt", "_salt");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
config.put("method", HashProcessor.Method.SHA1.toString());
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(e.getMessage(), equalTo("[fields] required property is missing"));
}

public void testProcessorNoTargetField() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList("_field"));
config.put("salt", "_salt");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
config.put("method", HashProcessor.Method.SHA1.toString());
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(e.getMessage(), equalTo("[target_field] required property is missing"));
}

public void testProcessorFieldsIsEmpty() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList(randomBoolean() ? "" : null));
config.put("salt", "_salt");
config.put("target_field", "_target");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
config.put("method", HashProcessor.Method.SHA1.toString());
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(e.getMessage(), equalTo("[fields] a field-name entry is either empty or null"));
}

public void testProcessorMissingSalt() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList("_field"));
config.put("target_field", "_target");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(e.getMessage(), equalTo("[salt] required property is missing"));
}

public void testProcessorInvalidMethod() {
MockSecureSettings mockSecureSettings = new MockSecureSettings();
mockSecureSettings.setString("xpack.security.ingest.hash.processor.key", "my_key");
Settings settings = Settings.builder().setSecureSettings(mockSecureSettings).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList("_field"));
config.put("salt", "_salt");
config.put("target_field", "_target");
config.put("key_setting", "xpack.security.ingest.hash.processor.key");
config.put("method", "invalid");
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(e.getMessage(), equalTo("[method] type [invalid] not supported, cannot convert field. " +
"Valid hash methods: [sha1, sha256, sha384, sha512]"));
}

public void testProcessorInvalidOrMissingKeySetting() {
Settings settings = Settings.builder().setSecureSettings(new MockSecureSettings()).build();
HashProcessor.Factory factory = new HashProcessor.Factory(settings);
Map<String, Object> config = new HashMap<>();
config.put("fields", Collections.singletonList("_field"));
config.put("salt", "_salt");
config.put("target_field", "_target");
config.put("key_setting", "invalid");
config.put("method", HashProcessor.Method.SHA1.toString());
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", new HashMap<>(config)));
assertThat(e.getMessage(),
equalTo("[key_setting] key [invalid] must match [xpack.security.ingest.hash.*.key]. It is not set"));
config.remove("key_setting");
ElasticsearchException ex = expectThrows(ElasticsearchException.class,
() -> factory.create(null, "_tag", config));
assertThat(ex.getMessage(), equalTo("[key_setting] required property is missing"));
}
}
Loading