From 3d0b7c63404411687ace6f9662e819392db4bb11 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Mon, 15 Oct 2018 11:19:55 +0200 Subject: [PATCH 1/6] AMBARI-24742. Encrypting/decrypting PASSWORD type properties when inserting them into the DB/using them --- .../server/configuration/Configuration.java | 11 ++ .../server/controller/ControllerModule.java | 6 + .../encryption/AESEncryptionService.java | 19 ++- .../server/security/encryption/Encryptor.java | 49 +++++++ .../PasswordPropertiesEncryptor.java | 121 ++++++++++++++++++ .../ambari/server/state/ConfigImpl.java | 34 +++-- .../server/update/HostUpdateHelper.java | 2 +- .../server/agent/AgentResourceTest.java | 9 ++ ...erosAdminPersistedCredentialCheckTest.java | 2 +- .../server/controller/KerberosHelperTest.java | 2 +- .../internal/HostResourceProviderTest.java | 2 +- .../testutils/PartialNiceMockBinder.java | 21 +++ .../server/update/HostUpdateHelperTest.java | 8 +- .../server/upgrade/UpgradeCatalog251Test.java | 3 +- .../server/upgrade/UpgradeCatalog252Test.java | 3 +- .../server/upgrade/UpgradeCatalog260Test.java | 4 +- .../server/upgrade/UpgradeCatalog270Test.java | 2 +- 17 files changed, 275 insertions(+), 23 deletions(-) create mode 100644 ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java create mode 100644 ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index d644c085ae2..63c1777484f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -2572,6 +2572,9 @@ public class Configuration { @Markdown(description = "Whether security password encryption is enabled or not. In case it is we store passwords in their own file(s); otherwise we store passwords in the Ambari credential store.") public static final ConfigurationProperty SECURITY_PASSWORD_ENCRYPTON_ENABLED = new ConfigurationProperty<>("security.passwords.encryption.enabled", false); + @Markdown(description="Whether to encrypt sensitive data (at rest) on service level configuration.") + public static final ConfigurationProperty SECURITY_SENSITIVE_DATA_ENCRYPTON_ENABLED = new ConfigurationProperty<>("security.server.encrypt_sensitive_data", false); + /** * The maximum number of authentication attempts permitted to a local user. Once the number of failures reaches this limit the user will be locked out. 0 indicates unlimited failures */ @@ -5518,6 +5521,14 @@ public boolean isSecurityPasswordEncryptionEnabled() { return Boolean.parseBoolean(getProperty(SECURITY_PASSWORD_ENCRYPTON_ENABLED)); } + public boolean isSensitiveDataEncryptionEnabled() { + return Boolean.parseBoolean(getProperty(SECURITY_SENSITIVE_DATA_ENCRYPTON_ENABLED)); + } + + public boolean shouldEncryptSensitiveData() { + return isSecurityPasswordEncryptionEnabled() && isSensitiveDataEncryptionEnabled(); + } + /** * @return default value of number of tasks to run in parallel during upgrades */ diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java index 176e709b4c3..45f7b9410bf 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java @@ -41,6 +41,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.Set; @@ -118,6 +119,8 @@ import org.apache.ambari.server.security.encryption.CredentialStoreService; import org.apache.ambari.server.security.encryption.CredentialStoreServiceImpl; import org.apache.ambari.server.security.encryption.EncryptionService; +import org.apache.ambari.server.security.encryption.Encryptor; +import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerFactory; import org.apache.ambari.server.serveraction.users.CollectionPersisterService; import org.apache.ambari.server.serveraction.users.CollectionPersisterServiceFactory; @@ -179,6 +182,7 @@ import com.google.inject.AbstractModule; import com.google.inject.Scopes; import com.google.inject.Singleton; +import com.google.inject.TypeLiteral; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.name.Names; import com.google.inject.persist.PersistModule; @@ -332,6 +336,8 @@ protected void configure() { bind(CredentialStoreService.class).to(CredentialStoreServiceImpl.class); bind(EncryptionService.class).to(AESEncryptionService.class); + //to support different Encryptor implementation we have to annotate them by their name and use them as @Named injects + bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).to(PasswordPropertiesEncryptor.class); bind(Configuration.class).toInstance(configuration); bind(OsFamily.class).toInstance(os_family); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/AESEncryptionService.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/AESEncryptionService.java index ffb92ba7462..25b3fc7866b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/AESEncryptionService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/AESEncryptionService.java @@ -24,6 +24,8 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.codec.binary.Hex; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.inject.Singleton; @Singleton @@ -32,6 +34,8 @@ public class AESEncryptionService implements EncryptionService { private static final String ENCODED_TEXT_FIELD_DELIMITER = "::"; private static final String UTF_8_CHARSET = StandardCharsets.UTF_8.name(); + private final Cache aesEncryptorCache = CacheBuilder.newBuilder().build(); + private MasterKeyService environmentMasterKeyService; @Override @@ -51,11 +55,20 @@ public String encrypt(String toBeEncrypted, String key) throws Exception { @Override public String encrypt(String toBeEncrypted, String key, TextEncoding textEncoding) throws Exception { - final AESEncryptor aes = new AESEncryptor(key); - final EncryptionResult encryptionResult = aes.encrypt(toBeEncrypted); + final EncryptionResult encryptionResult = getAesEncryptor(key).encrypt(toBeEncrypted); return TextEncoding.BASE_64 == textEncoding ? encodeEncryptionResultBase64(encryptionResult) : encodeEncryptionResultBinHex(encryptionResult); } + private AESEncryptor getAesEncryptor(String key) { + AESEncryptor aesEncryptor = aesEncryptorCache.getIfPresent(key); + if (aesEncryptor == null) { + aesEncryptor = new AESEncryptor(key); + aesEncryptorCache.put(key, aesEncryptor); + } + + return aesEncryptor; + } + private final String getAmbariMasterKey() { initEnvironmentMasterKeyService(); return String.valueOf(environmentMasterKeyService.getMasterSecret()); @@ -100,7 +113,7 @@ public String decrypt(String toBeDecrypted, String key, TextEncoding textEncodin final byte[] decodedValue = TextEncoding.BASE_64 == textEncoding ? Base64.decodeBase64(toBeDecrypted) : Hex.decodeHex(toBeDecrypted.toCharArray()); final String decodedText = new String(decodedValue, UTF_8_CHARSET); final String[] decodedParts = decodedText.split(ENCODED_TEXT_FIELD_DELIMITER); - final AESEncryptor aes = new AESEncryptor(key); + final AESEncryptor aes = getAesEncryptor(key); if (TextEncoding.BASE_64 == textEncoding) { return new String(aes.decrypt(Base64.decodeBase64(decodedParts[0]), Base64.decodeBase64(decodedParts[1]), Base64.decodeBase64(decodedParts[2])), UTF_8_CHARSET); } else { diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java new file mode 100644 index 00000000000..3936fbf88c7 --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ambari.server.security.encryption; + +/** + * Defines a generic contract on encrypting/decrypting sensitive data + */ +public interface Encryptor { + + /** + * Encrypts the given encryptible object + * + * @param encryptible + * to be encrypted + * @param additionalInfo + * any additional information that is needed during the encryption + * process (may be null) + * @return the encrypted value + */ + T encryptSensitiveData(T encryptible, Object... additionalInfo); + + /** + * Decrypts the given decryptible object + * + * @param decryptible + * to be decrypted + * @param additionalInfo + * any additional information that is needed during the decryption + * process (may be null) + * @return the decrypted value + */ + T decryptSensitiveData(T decryptible, Object... additionalInfo); + +} diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java new file mode 100644 index 00000000000..08927680c91 --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ambari.server.security.encryption; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import org.apache.ambari.server.AmbariException; +import org.apache.ambari.server.AmbariRuntimeException; +import org.apache.ambari.server.state.Cluster; +import org.apache.ambari.server.state.Clusters; +import org.apache.ambari.server.state.PropertyInfo.PropertyType; +import org.apache.ambari.server.utils.TextEncoding; + +import com.google.inject.Inject; +import com.google.inject.Singleton; + +/** + * {@link Encryptor} implementation for encrypting/decrypting PASSWORD type properties + * + */ + +@Singleton +public class PasswordPropertiesEncryptor implements Encryptor> { + + private static final String ENCRYPTED_PROPERTY_PREFIX = "${enc=aes256_hex, value="; + private static final String ENCRYPTED_PROPERTY_SCHEME = ENCRYPTED_PROPERTY_PREFIX + "%s}"; + + private final EncryptionService encryptionService; + private final Map>> clusterPasswordProperties = new HashMap<>(); + + @Inject + public PasswordPropertiesEncryptor(Clusters clusters, EncryptionService encryptionService) { + this.encryptionService = encryptionService; + } + + @Override + public Map encryptSensitiveData(Map properties, Object... additionaKeys) { + try { + final Map encryptedProperties = new HashMap<>(properties.size()); + if (properties != null) { + final Cluster cluster = (Cluster) additionaKeys[0]; + final String configType = (String) additionaKeys[1]; + final Set passwordProperties = getPasswordProperties(cluster, configType); + if (passwordProperties != null && !passwordProperties.isEmpty()) { + String encryptedValue; + for (Map.Entry property : properties.entrySet()) { + encryptedValue = passwordProperties.contains(property.getKey()) && !isEncryptedPassword(property.getValue()) + ? encryptAndDecoratePropertyValue(property.getValue()) + : property.getValue(); + encryptedProperties.put(property.getKey(), encryptedValue); + } + } + } + return encryptedProperties; + } catch (Exception e) { + throw new AmbariRuntimeException("Error while encrypting sensitive data", e); + } + } + + private boolean isEncryptedPassword(String password) { + return password != null && password.startsWith(ENCRYPTED_PROPERTY_PREFIX); // assuming previous encryption by this class + } + + private Set getPasswordProperties(Cluster cluster, String configType) throws AmbariException { + final long clusterId = cluster.getClusterId(); + if (!clusterPasswordProperties.containsKey(clusterId)) { + clusterPasswordProperties.put(clusterId, new HashMap<>()); + } + if (!clusterPasswordProperties.get(clusterId).containsKey(configType)) { + clusterPasswordProperties.get(clusterId).put(configType, cluster.getConfigPropertiesTypes(configType).get(PropertyType.PASSWORD)); + } + + return clusterPasswordProperties.get(clusterId).get(configType); + } + + private String encryptAndDecoratePropertyValue(String propertyValue) throws Exception { + final String encrypted = encryptionService.encrypt(propertyValue, TextEncoding.BIN_HEX); + return String.format(ENCRYPTED_PROPERTY_SCHEME, encrypted); + } + + @Override + public Map decryptSensitiveData(Map properties, Object... additionalInfo) { + final Map decryptedProperties = new HashMap<>(properties.size()); + if (properties != null) { + String decryptedValue; + for (Map.Entry property : properties.entrySet()) { + decryptedValue = isEncryptedPassword(property.getValue()) ? decryptProperty(property.getValue()) : property.getValue(); + decryptedProperties.put(property.getKey(), decryptedValue); + } + } + return decryptedProperties; + } + + private String decryptProperty(String property) { + try { + // sample value: ${enc=aes256_hex, value=5248...303d} + final String encrypted = property.substring(ENCRYPTED_PROPERTY_PREFIX.length(), property.indexOf('}')); + return encryptionService.decrypt(encrypted, TextEncoding.BIN_HEX); + } catch (Exception e) { + throw new AmbariRuntimeException("Error while decrypting property", e); + } + } + +} diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java index b3ff8433ea4..e082b40d9fa 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java @@ -27,7 +27,9 @@ import java.util.concurrent.locks.ReadWriteLock; import javax.annotation.Nullable; +import javax.inject.Named; +import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.events.ClusterConfigChangedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.logging.LockFactory; @@ -37,11 +39,13 @@ import org.apache.ambari.server.orm.entities.ClusterConfigEntity; import org.apache.ambari.server.orm.entities.ClusterEntity; import org.apache.ambari.server.orm.entities.StackEntity; +import org.apache.ambari.server.security.encryption.Encryptor; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import com.google.gson.JsonSyntaxException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -98,15 +102,18 @@ public class ConfigImpl implements Config { private final AmbariEventPublisher eventPublisher; + private final Encryptor> passwordPropertiesEncryptor; + @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted("type") String type, @Assisted("tag") @Nullable String tag, @Assisted Map properties, @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, StackDAO stackDAO, - Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory) { + Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, + Configuration serverConfiguration, @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { this(cluster.getDesiredStackVersion(), cluster, type, tag, properties, propertiesAttributes, - clusterDAO, stackDAO, gson, eventPublisher, lockFactory); + clusterDAO, stackDAO, gson, eventPublisher, lockFactory, serverConfiguration, passwordPropertiesEncryptor); } @@ -116,20 +123,22 @@ public class ConfigImpl implements Config { @Assisted Map properties, @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, StackDAO stackDAO, - Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory) { + Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, + Configuration serverConfiguration, @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.cluster = cluster; this.type = type; - this.properties = properties; + this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; + this.properties = serverConfiguration.shouldEncryptSensitiveData() ? passwordPropertiesEncryptor.encryptSensitiveData(properties, cluster, type) : properties; // only set this if it's non-null this.propertiesAttributes = null == propertiesAttributes ? null : new HashMap<>(propertiesAttributes); this.clusterDAO = clusterDAO; - this.gson = gson; + this.gson = new GsonBuilder().disableHtmlEscaping().create(); this.eventPublisher = eventPublisher; version = cluster.getNextConfigVersion(type); @@ -148,10 +157,10 @@ public class ConfigImpl implements Config { entity.setTag(this.tag); entity.setTimestamp(System.currentTimeMillis()); entity.setStack(stackEntity); - entity.setData(gson.toJson(properties)); + entity.setData(this.gson.toJson(this.properties)); if (null != propertiesAttributes) { - entity.setAttributes(gson.toJson(propertiesAttributes)); + entity.setAttributes(this.gson.toJson(propertiesAttributes)); } // when creating a brand new config without a backing entity, use the @@ -166,7 +175,8 @@ public class ConfigImpl implements Config { @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher, - LockFactory lockFactory) { + LockFactory lockFactory, + @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.cluster = cluster; @@ -184,6 +194,8 @@ public class ConfigImpl implements Config { propertiesTypes = cluster.getConfigPropertiesTypes(type); + this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; + // incur the hit on deserialization since this business object is stored locally try { Map deserializedProperties = gson.> fromJson( @@ -230,12 +242,14 @@ public class ConfigImpl implements Config { @Assisted("tag") @Nullable String tag, @Assisted Map properties, @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, - Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory) { + Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, + @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.tag = tag; this.type = type; + this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; this.properties = new HashMap<>(properties); this.propertiesAttributes = null == propertiesAttributes ? null : new HashMap<>(propertiesAttributes); @@ -286,7 +300,7 @@ public Long getVersion() { public Map getProperties() { propertyLock.readLock().lock(); try { - return properties == null ? new HashMap<>() : new HashMap<>(properties); + return properties == null ? new HashMap<>() : passwordPropertiesEncryptor.decryptSensitiveData(properties); } finally { propertyLock.readLock().unlock(); } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java index 27b46787a17..ac6bba8505f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java @@ -245,7 +245,7 @@ void updateHostsInConfigurations() throws AmbariException { for (Map.Entry property : config.getProperties().entrySet()) { updatedPropertyValue = replaceHosts(property.getValue(), currentHostNames, hostMapping); - + if (updatedPropertyValue != null) { Map propertiesWithUpdates = config.getProperties(); propertiesWithUpdates.put(property.getKey(), updatedPropertyValue); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java index 37d516601d8..83a9d6bec8f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java @@ -23,6 +23,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.Map; + import javax.persistence.EntityManager; import javax.ws.rs.core.MediaType; @@ -59,8 +61,12 @@ import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl; import org.apache.ambari.server.security.SecurityHelper; import org.apache.ambari.server.security.SecurityHelperImpl; +import org.apache.ambari.server.security.encryption.AESEncryptionService; import org.apache.ambari.server.security.encryption.CredentialStoreService; import org.apache.ambari.server.security.encryption.CredentialStoreServiceImpl; +import org.apache.ambari.server.security.encryption.EncryptionService; +import org.apache.ambari.server.security.encryption.Encryptor; +import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.stack.upgrade.orchestrate.UpgradeContextFactory; import org.apache.ambari.server.stageplanner.RoleGraphFactory; @@ -107,6 +113,7 @@ import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.TypeLiteral; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.name.Names; import com.sun.jersey.api.client.Client; @@ -352,6 +359,8 @@ protected void configure() { bind(AmbariManagementController.class).toInstance(createNiceMock(AmbariManagementController.class)); bind(KerberosHelper.class).toInstance(createNiceMock(KerberosHelper.class)); bind(MpackManagerFactory.class).toInstance(createNiceMock(MpackManagerFactory.class)); + bind(EncryptionService.class).to(AESEncryptionService.class); + bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).to(PasswordPropertiesEncryptor.class); } private void installDependencies() { diff --git a/ambari-server/src/test/java/org/apache/ambari/server/checks/KerberosAdminPersistedCredentialCheckTest.java b/ambari-server/src/test/java/org/apache/ambari/server/checks/KerberosAdminPersistedCredentialCheckTest.java index edea0a7c789..ff3be4942a1 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/checks/KerberosAdminPersistedCredentialCheckTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/checks/KerberosAdminPersistedCredentialCheckTest.java @@ -217,7 +217,7 @@ Injector getInjector() { @Override protected void configure() { PartialNiceMockBinder.newBuilder().addActionDBAccessorConfigsBindings().addFactoriesInstallBinding() - .build().configure(binder()); + .addPasswordEncryptorBindings().build().configure(binder()); bind(ExecutionScheduler.class).toInstance(createNiceMock(ExecutionSchedulerImpl.class)); bind(EntityManager.class).toInstance(createNiceMock(EntityManager.class)); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java index 584eff91784..c229eb2a128 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java @@ -238,7 +238,7 @@ public boolean createKeytabFile(Keytab keytab, File destinationKeytabFile) throw @Override protected void configure() { PartialNiceMockBinder.newBuilder().addActionDBAccessorConfigsBindings().addFactoriesInstallBinding() - .build().configure(binder()); + .addPasswordEncryptorBindings().build().configure(binder()); bind(ActionDBAccessor.class).to(ActionDBAccessorImpl.class); bind(ExecutionScheduler.class).to(ExecutionSchedulerImpl.class); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java index 36590194c4d..125a9e9b1a4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java @@ -1403,7 +1403,7 @@ private Injector createInjector() throws Exception { Injector injector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder()); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().addPasswordEncryptorBindings().build().configure(binder()); bind(EntityManager.class).toInstance(createNiceMock(EntityManager.class)); bind(DBAccessor.class).toInstance(createNiceMock(DBAccessor.class)); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java b/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java index d87a335cd48..19cd83c13af 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import javax.persistence.EntityManager; @@ -55,6 +56,8 @@ import org.apache.ambari.server.scheduler.ExecutionScheduler; import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl; import org.apache.ambari.server.security.encryption.CredentialStoreService; +import org.apache.ambari.server.security.encryption.EncryptionService; +import org.apache.ambari.server.security.encryption.Encryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.stack.upgrade.orchestrate.UpgradeContextFactory; import org.apache.ambari.server.stageplanner.RoleGraphFactory; @@ -90,6 +93,7 @@ import com.google.inject.Binder; import com.google.inject.Module; +import com.google.inject.TypeLiteral; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.name.Names; import com.google.inject.persist.UnitOfWork; @@ -134,6 +138,10 @@ public Builder addAlertDefinitionBinding() { } public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagementController) { + return addAmbariMetaInfoBinding(ambariManagementController, easyMockSupport.createNiceMock(Encryptor.class)); + } + + public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagementController, Encryptor> passwordEncryptor) { configurers.add((Binder binder) -> { binder.bind(PersistedState.class).toInstance(easyMockSupport.createNiceMock(PersistedState.class)); binder.bind(HostRoleCommandFactory.class).to(HostRoleCommandFactoryImpl.class); @@ -153,6 +161,7 @@ public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagem }); addConfigsBindings(); addFactoriesInstallBinding(); + addPasswordEncryptorBindings(passwordEncryptor); return this; } @@ -206,6 +215,18 @@ public Builder addConfigsBindings() { return this; } + public Builder addPasswordEncryptorBindings() { + return addPasswordEncryptorBindings(easyMockSupport.createNiceMock(Encryptor.class)); + } + + public Builder addPasswordEncryptorBindings(Encryptor> passwordEncryptor) { + configurers.add((Binder binder) -> { + binder.bind(EncryptionService.class).toInstance(easyMockSupport.createNiceMock(EncryptionService.class)); + binder.bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).toInstance(passwordEncryptor); + }); + return this; + } + public Builder addHostRoleCommandsConfigsBindings() { configurers.add((Binder binder) -> { binder.bindConstant().annotatedWith(Names.named(HostRoleCommandDAO.HRC_STATUS_SUMMARY_CACHE_ENABLED)).to(true); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java index 9c438c91f62..ac7b4e61e7f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java @@ -67,6 +67,8 @@ import org.apache.ambari.server.scheduler.ExecutionScheduler; import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl; import org.apache.ambari.server.security.encryption.CredentialStoreService; +import org.apache.ambari.server.security.encryption.Encryptor; +import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; @@ -246,6 +248,7 @@ public void testUpdateHostsInConfigurations() throws AmbariException { ClusterConfigEntity mockClusterConfigEntity1 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); ClusterConfigEntity mockClusterConfigEntity2 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); StackEntity mockStackEntity = easyMockSupport.createNiceMock(StackEntity.class); + final Encryptor> passwordEncryptor = new PasswordPropertiesEncryptor(null, null); Map> clusterHostsToChange = new HashMap<>(); Map hosts = new HashMap<>(); List clusterConfigEntities1 = new ArrayList<>(); @@ -253,7 +256,7 @@ public void testUpdateHostsInConfigurations() throws AmbariException { final Injector mockInjector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addClustersBinding(mockAmbariManagementController).build().configure(binder()); + PartialNiceMockBinder.newBuilder().addAmbariMetaInfoBinding(mockAmbariManagementController, passwordEncryptor).build().configure(binder()); bind(StackManagerFactory.class).toInstance(easyMockSupport.createNiceMock(StackManagerFactory.class)); bind(DBAccessor.class).toInstance(dbAccessor); bind(EntityManager.class).toInstance(entityManager); @@ -495,7 +498,8 @@ public void testUpdateHostsForAlertsInDB() throws AmbariException { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder()); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().addPasswordEncryptorBindings() + .build().configure(binder()); bind(DBAccessor.class).toInstance(dbAccessor); bind(EntityManager.class).toInstance(entityManager); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog251Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog251Test.java index 6ecea16cbf3..03437fa4a64 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog251Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog251Test.java @@ -292,7 +292,8 @@ private Injector getInjector(Clusters clusters, AmbariManagementController ambar Module module = new Module() { @Override public void configure(Binder binder) { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding() + .addPasswordEncryptorBindings().build().configure(binder); binder.bind(DBAccessor.class).toInstance(dbAccessor); binder.bind(OsFamily.class).toInstance(osFamily); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog252Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog252Test.java index 1d5168c764c..7dfee87f4ef 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog252Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog252Test.java @@ -440,7 +440,8 @@ private Injector getInjector(Clusters clusters, AmbariManagementController ambar Module module = new Module() { @Override public void configure(Binder binder) { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding() + .addPasswordEncryptorBindings().build().configure(binder); binder.bind(DBAccessor.class).toInstance(dbAccessor); binder.bind(OsFamily.class).toInstance(osFamily); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog260Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog260Test.java index ba09e97519c..02a45c2916e 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog260Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog260Test.java @@ -1025,7 +1025,8 @@ public void testHDFSWidgetUpdate() throws Exception { final Injector mockInjector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder()); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding() + .addPasswordEncryptorBindings().build().configure(binder()); bind(EntityManager.class).toInstance(createNiceMock(EntityManager.class)); bind(AmbariManagementController.class).toInstance(controller); @@ -1083,6 +1084,7 @@ private Injector getInjector() { return Guice.createInjector(new Module() { @Override public void configure(Binder binder) { + PartialNiceMockBinder.newBuilder().addPasswordEncryptorBindings().build().configure(binder); binder.bindConstant().annotatedWith(Names.named("actionTimeout")).to(600000L); binder.bindConstant().annotatedWith(Names.named("schedulerSleeptime")).to(1L); binder.bindConstant().annotatedWith(Names.named(HostRoleCommandDAO.HRC_STATUS_SUMMARY_CACHE_ENABLED)).to(true); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog270Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog270Test.java index c2096d5ef24..6fa317bb58e 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog270Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog270Test.java @@ -600,7 +600,7 @@ private Module getTestGuiceModule() { Module module = new AbstractModule() { @Override public void configure() { - PartialNiceMockBinder.newBuilder().addConfigsBindings().addFactoriesInstallBinding().build().configure(binder()); + PartialNiceMockBinder.newBuilder().addConfigsBindings().addPasswordEncryptorBindings().addFactoriesInstallBinding().build().configure(binder()); bind(DBAccessor.class).toInstance(dbAccessor); bind(OsFamily.class).toInstance(osFamily); From 0e2dc7905dbfba40bb873fadd6138ba2a4e2131f Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Mon, 15 Oct 2018 22:39:30 +0200 Subject: [PATCH 2/6] AMBARI-24742. Handling the situation when stack is upgraded; documented additional information we need for encryption and using 3rd party to check if password property set of a config type is netiher null nor empty --- .../PasswordPropertiesEncryptor.java | 51 +++++++++++++++---- .../server/update/HostUpdateHelperTest.java | 13 +++-- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java index 08927680c91..9f108c1f7d6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java @@ -20,22 +20,26 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.AmbariRuntimeException; +import org.apache.ambari.server.events.StackUpgradeFinishEvent; +import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.state.Cluster; -import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.PropertyInfo.PropertyType; import org.apache.ambari.server.utils.TextEncoding; +import org.apache.commons.collections.CollectionUtils; +import com.google.common.eventbus.Subscribe; import com.google.inject.Inject; import com.google.inject.Singleton; /** - * {@link Encryptor} implementation for encrypting/decrypting PASSWORD type properties - * + * {@link Encryptor} implementation for encrypting/decrypting PASSWORD type + * properties */ - + @Singleton public class PasswordPropertiesEncryptor implements Encryptor> { @@ -43,22 +47,34 @@ public class PasswordPropertiesEncryptor implements Encryptor>> clusterPasswordProperties = new HashMap<>(); + private final Map>> clusterPasswordProperties = new ConcurrentHashMap<>(); @Inject - public PasswordPropertiesEncryptor(Clusters clusters, EncryptionService encryptionService) { + public PasswordPropertiesEncryptor(EncryptionService encryptionService, AmbariEventPublisher ambariEventPublisher) { this.encryptionService = encryptionService; + ambariEventPublisher.register(this); } + /** + * It is a must that any caller of this method adds the following additional + * information on the following indexes in additionalInfo with the + * following types: + *

    + *
  • [0] - org.apache.ambari.server.state.Cluster (the cluster + * whose properties should be scanned for PASSWORD properties)
    • + *
  • [1] - java.lang.String (to keep properties only that match + * this configuration type )
    • + *
+ */ @Override - public Map encryptSensitiveData(Map properties, Object... additionaKeys) { + public Map encryptSensitiveData(Map properties, Object... additionaInfo) { try { final Map encryptedProperties = new HashMap<>(properties.size()); if (properties != null) { - final Cluster cluster = (Cluster) additionaKeys[0]; - final String configType = (String) additionaKeys[1]; + final Cluster cluster = (Cluster) additionaInfo[0]; + final String configType = (String) additionaInfo[1]; final Set passwordProperties = getPasswordProperties(cluster, configType); - if (passwordProperties != null && !passwordProperties.isEmpty()) { + if (CollectionUtils.isNotEmpty(passwordProperties)) { String encryptedValue; for (Map.Entry property : properties.entrySet()) { encryptedValue = passwordProperties.contains(property.getKey()) && !isEncryptedPassword(property.getValue()) @@ -81,7 +97,7 @@ private boolean isEncryptedPassword(String password) { private Set getPasswordProperties(Cluster cluster, String configType) throws AmbariException { final long clusterId = cluster.getClusterId(); if (!clusterPasswordProperties.containsKey(clusterId)) { - clusterPasswordProperties.put(clusterId, new HashMap<>()); + clusterPasswordProperties.put(clusterId, new ConcurrentHashMap<>()); } if (!clusterPasswordProperties.get(clusterId).containsKey(configType)) { clusterPasswordProperties.get(clusterId).put(configType, cluster.getConfigPropertiesTypes(configType).get(PropertyType.PASSWORD)); @@ -118,4 +134,17 @@ private String decryptProperty(String property) { } } + @Subscribe + public void onStackUpgradeFinished(StackUpgradeFinishEvent event) { + // during stack upgrade it is very unlikely that one edits cluster configuration + // therefore it's safe to assume that other threads will not encrypting + // sensitive data at the same time we clear the cached password properties. Thus + // additional Java locking is not needed (using concurrent map to store this + // information is enough) + final long clusterId = event.getClusterId(); + if (clusterPasswordProperties.containsKey(clusterId)) { + clusterPasswordProperties.get(clusterId).clear(); + } + } + } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java index ac7b4e61e7f..dc167cb10e7 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java @@ -67,7 +67,6 @@ import org.apache.ambari.server.scheduler.ExecutionScheduler; import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl; import org.apache.ambari.server.security.encryption.CredentialStoreService; -import org.apache.ambari.server.security.encryption.Encryptor; import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.state.Cluster; @@ -88,6 +87,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.StandardPasswordEncoder; +import com.google.common.collect.ImmutableMap; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import com.google.inject.AbstractModule; @@ -248,7 +248,7 @@ public void testUpdateHostsInConfigurations() throws AmbariException { ClusterConfigEntity mockClusterConfigEntity1 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); ClusterConfigEntity mockClusterConfigEntity2 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); StackEntity mockStackEntity = easyMockSupport.createNiceMock(StackEntity.class); - final Encryptor> passwordEncryptor = new PasswordPropertiesEncryptor(null, null); + final PasswordPropertiesEncryptor passwordEncryptorMock = easyMockSupport.createNiceMock(PasswordPropertiesEncryptor.class); Map> clusterHostsToChange = new HashMap<>(); Map hosts = new HashMap<>(); List clusterConfigEntities1 = new ArrayList<>(); @@ -256,7 +256,7 @@ public void testUpdateHostsInConfigurations() throws AmbariException { final Injector mockInjector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addAmbariMetaInfoBinding(mockAmbariManagementController, passwordEncryptor).build().configure(binder()); + PartialNiceMockBinder.newBuilder().addAmbariMetaInfoBinding(mockAmbariManagementController, passwordEncryptorMock).build().configure(binder()); bind(StackManagerFactory.class).toInstance(easyMockSupport.createNiceMock(StackManagerFactory.class)); bind(DBAccessor.class).toInstance(dbAccessor); bind(EntityManager.class).toInstance(entityManager); @@ -304,6 +304,9 @@ protected void configure() { expect(mockClusterConfigEntity1.getType()).andReturn("testType1").atLeastOnce(); expect(mockClusterConfigEntity1.getVersion()).andReturn(1L).atLeastOnce(); expect(mockClusterDAO.findConfig(1L)).andReturn(mockClusterConfigEntity1).atLeastOnce(); + final Map expectedMap1 = new HashMap<>(ImmutableMap.of("testProperty1", "testValue_host1", "testProperty2", "testValue_host5", "testProperty3", + "testValue_host11", "testProperty4", "testValue_host55")); + expect(passwordEncryptorMock.decryptSensitiveData(expectedMap1)).andReturn(expectedMap1).atLeastOnce(); expect(mockClusterConfigEntity2.getClusterId()).andReturn(1L).atLeastOnce(); expect(mockClusterConfigEntity2.getConfigId()).andReturn(2L).anyTimes(); @@ -313,6 +316,8 @@ protected void configure() { expect(mockClusterConfigEntity2.getType()).andReturn("testType2").atLeastOnce(); expect(mockClusterConfigEntity2.getVersion()).andReturn(2L).atLeastOnce(); expect(mockClusterDAO.findConfig(2L)).andReturn(mockClusterConfigEntity2).atLeastOnce(); + final Map expectedMap2 = new HashMap<>(ImmutableMap.of("testProperty5", "test_host1_test_host5_test_host11_test_host55")); + expect(passwordEncryptorMock.decryptSensitiveData(expectedMap2)).andReturn(expectedMap2).atLeastOnce(); Capture dataCapture = EasyMock.newCapture(); mockClusterConfigEntity1.setData(EasyMock.capture(dataCapture)); @@ -320,7 +325,7 @@ protected void configure() { mockClusterConfigEntity2.setData("{\"testProperty5\":\"test_host5_test_host1_test_host55_test_host11\"}"); expectLastCall(); - + HostUpdateHelper hostUpdateHelper = new HostUpdateHelper(null, null, mockInjector); hostUpdateHelper.setHostChangesFileMap(clusterHostsToChange); From 751189b7447b87b34b418f264bda549b16120bd2 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Wed, 17 Oct 2018 13:05:46 +0200 Subject: [PATCH 3/6] AMBARI-24472. Using a more effective way to populate password properties cache --- .../encryption/PasswordPropertiesEncryptor.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java index 9f108c1f7d6..65348f7ddc2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java @@ -18,6 +18,7 @@ package org.apache.ambari.server.security.encryption; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -96,13 +97,8 @@ private boolean isEncryptedPassword(String password) { private Set getPasswordProperties(Cluster cluster, String configType) throws AmbariException { final long clusterId = cluster.getClusterId(); - if (!clusterPasswordProperties.containsKey(clusterId)) { - clusterPasswordProperties.put(clusterId, new ConcurrentHashMap<>()); - } - if (!clusterPasswordProperties.get(clusterId).containsKey(configType)) { - clusterPasswordProperties.get(clusterId).put(configType, cluster.getConfigPropertiesTypes(configType).get(PropertyType.PASSWORD)); - } - + clusterPasswordProperties.computeIfAbsent(clusterId, (x -> new ConcurrentHashMap<>())).computeIfAbsent(configType, k -> new HashSet<>()) + .addAll(cluster.getConfigPropertiesTypes(configType).get(PropertyType.PASSWORD)); return clusterPasswordProperties.get(clusterId).get(configType); } From 64812db757a348443ec284f54be524bb58139b43 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Thu, 18 Oct 2018 22:46:33 +0200 Subject: [PATCH 4/6] AMBARI-24742. Caching properties types by stacks --- .../PasswordPropertiesEncryptor.java | 63 +++++++++---------- .../apache/ambari/server/state/Cluster.java | 10 ++- .../server/state/cluster/ClusterImpl.java | 9 ++- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java index 65348f7ddc2..6e82f5de581 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java @@ -25,14 +25,12 @@ import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.AmbariRuntimeException; -import org.apache.ambari.server.events.StackUpgradeFinishEvent; -import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.PropertyInfo.PropertyType; +import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.utils.TextEncoding; import org.apache.commons.collections.CollectionUtils; -import com.google.common.eventbus.Subscribe; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -48,12 +46,11 @@ public class PasswordPropertiesEncryptor implements Encryptor>> clusterPasswordProperties = new ConcurrentHashMap<>(); + private final Map>>> clusterPasswordProperties = new ConcurrentHashMap<>(); //Map>>>; @Inject - public PasswordPropertiesEncryptor(EncryptionService encryptionService, AmbariEventPublisher ambariEventPublisher) { + public PasswordPropertiesEncryptor(EncryptionService encryptionService) { this.encryptionService = encryptionService; - ambariEventPublisher.register(this); } /** @@ -70,22 +67,21 @@ public PasswordPropertiesEncryptor(EncryptionService encryptionService, AmbariEv @Override public Map encryptSensitiveData(Map properties, Object... additionaInfo) { try { - final Map encryptedProperties = new HashMap<>(properties.size()); if (properties != null) { + final Map encryptedProperties = new HashMap<>(properties); final Cluster cluster = (Cluster) additionaInfo[0]; final String configType = (String) additionaInfo[1]; final Set passwordProperties = getPasswordProperties(cluster, configType); if (CollectionUtils.isNotEmpty(passwordProperties)) { - String encryptedValue; for (Map.Entry property : properties.entrySet()) { - encryptedValue = passwordProperties.contains(property.getKey()) && !isEncryptedPassword(property.getValue()) - ? encryptAndDecoratePropertyValue(property.getValue()) - : property.getValue(); - encryptedProperties.put(property.getKey(), encryptedValue); + if (passwordProperties.contains(property.getKey()) && !isEncryptedPassword(property.getValue())) { + encryptedProperties.put(property.getKey(), encryptAndDecoratePropertyValue(property.getValue())); + } } } + return encryptedProperties; } - return encryptedProperties; + return properties; } catch (Exception e) { throw new AmbariRuntimeException("Error while encrypting sensitive data", e); } @@ -96,10 +92,20 @@ private boolean isEncryptedPassword(String password) { } private Set getPasswordProperties(Cluster cluster, String configType) throws AmbariException { + //in case of normal configuration change on the UI - or via the API - the current and desired stacks are equal + //in case of an upgrade they are different; in this case we want to get password properties from the desired stack + if (cluster.getCurrentStackVersion().equals(cluster.getDesiredStackVersion())) { + return getPasswordProperties(cluster, cluster.getCurrentStackVersion(), configType); + } else { + return getPasswordProperties(cluster, cluster.getDesiredStackVersion(), configType); + } + } + + private Set getPasswordProperties(Cluster cluster, StackId stackId, String configType) { final long clusterId = cluster.getClusterId(); - clusterPasswordProperties.computeIfAbsent(clusterId, (x -> new ConcurrentHashMap<>())).computeIfAbsent(configType, k -> new HashSet<>()) - .addAll(cluster.getConfigPropertiesTypes(configType).get(PropertyType.PASSWORD)); - return clusterPasswordProperties.get(clusterId).get(configType); + clusterPasswordProperties.computeIfAbsent(clusterId, v -> new ConcurrentHashMap<>()).computeIfAbsent(stackId, v -> new ConcurrentHashMap<>()) + .computeIfAbsent(configType, v -> cluster.getConfigPropertiesTypes(configType, stackId).getOrDefault(PropertyType.PASSWORD, new HashSet<>())); + return clusterPasswordProperties.get(clusterId).get(stackId).getOrDefault(configType, new HashSet<>()); } private String encryptAndDecoratePropertyValue(String propertyValue) throws Exception { @@ -109,15 +115,16 @@ private String encryptAndDecoratePropertyValue(String propertyValue) throws Exce @Override public Map decryptSensitiveData(Map properties, Object... additionalInfo) { - final Map decryptedProperties = new HashMap<>(properties.size()); if (properties != null) { - String decryptedValue; + final Map decryptedProperties = new HashMap<>(properties); for (Map.Entry property : properties.entrySet()) { - decryptedValue = isEncryptedPassword(property.getValue()) ? decryptProperty(property.getValue()) : property.getValue(); - decryptedProperties.put(property.getKey(), decryptedValue); + if (isEncryptedPassword(property.getValue())) { + decryptedProperties.put(property.getKey(), decryptProperty(property.getValue())); + } } + return decryptedProperties; } - return decryptedProperties; + return properties; } private String decryptProperty(String property) { @@ -129,18 +136,4 @@ private String decryptProperty(String property) { throw new AmbariRuntimeException("Error while decrypting property", e); } } - - @Subscribe - public void onStackUpgradeFinished(StackUpgradeFinishEvent event) { - // during stack upgrade it is very unlikely that one edits cluster configuration - // therefore it's safe to assume that other threads will not encrypting - // sensitive data at the same time we clear the cached password properties. Thus - // additional Java locking is not needed (using concurrent map to store this - // information is enough) - final long clusterId = event.getClusterId(); - if (clusterPasswordProperties.containsKey(clusterId)) { - clusterPasswordProperties.get(clusterId).clear(); - } - } - } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java index c201310648c..a150796ca72 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java @@ -268,12 +268,20 @@ List transitionHostsToInstalling(RepositoryVersionEntity repoVersionEntity Map getConfigsByType(String configType); /** - * Gets all properties types that mach the specified type. + * Gets all properties types that matches the specified type for the current stack. * @param configType the config type to return * @return properties types for given config type */ Map> getConfigPropertiesTypes(String configType); + /** + * Gets all properties types that matches the specified type for the given stack. + * @param configType the config type to return + * @param stackId the stack to scan properties for + * @return properties types for given config type + */ + Map> getConfigPropertiesTypes(String configType, StackId stackId); + /** * Gets the specific config that matches the specified type and tag. This not * necessarily a DESIRED configuration that applies to a cluster. diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java index eb1176ece93..4ca2cafcbb7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java @@ -2548,8 +2548,15 @@ public void applyLatestConfigurations(StackId stackId, String serviceName) { */ @Override public Map> getConfigPropertiesTypes(String configType){ + return getConfigPropertiesTypes(configType, getCurrentStackVersion()); + } + + /** + * {@inheritDoc} + */ + @Override + public Map> getConfigPropertiesTypes(String configType, StackId stackId) { try { - StackId stackId = getCurrentStackVersion(); StackInfo stackInfo = ambariMetaInfo.getStack(stackId.getStackName(), stackId.getStackVersion()); return stackInfo.getConfigPropertiesTypes(configType); } catch (AmbariException ignored) { From bd579c46666edd83ba1f557c8c85e3f532fa5f46 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Fri, 19 Oct 2018 13:28:42 +0200 Subject: [PATCH 5/6] AMBARI-24742. To make encryptor really generic we implement a Config encryptor --- .../server/controller/ControllerModule.java | 5 +- ...or.java => ConfigPropertiesEncryptor.java} | 46 +++++++------------ .../server/security/encryption/Encryptor.java | 10 +--- .../apache/ambari/server/state/Config.java | 5 ++ .../ambari/server/state/ConfigImpl.java | 31 +++++++------ .../server/update/HostUpdateHelper.java | 2 +- .../server/agent/AgentResourceTest.java | 6 +-- .../testutils/PartialNiceMockBinder.java | 13 +----- .../server/update/HostUpdateHelperTest.java | 12 +---- 9 files changed, 49 insertions(+), 81 deletions(-) rename ambari-server/src/main/java/org/apache/ambari/server/security/encryption/{PasswordPropertiesEncryptor.java => ConfigPropertiesEncryptor.java} (77%) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java index 45f7b9410bf..2e60debd819 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java @@ -41,7 +41,6 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.Set; @@ -116,11 +115,11 @@ import org.apache.ambari.server.security.authorization.internal.InternalAuthenticationInterceptor; import org.apache.ambari.server.security.authorization.internal.RunWithInternalSecurityContext; import org.apache.ambari.server.security.encryption.AESEncryptionService; +import org.apache.ambari.server.security.encryption.ConfigPropertiesEncryptor; import org.apache.ambari.server.security.encryption.CredentialStoreService; import org.apache.ambari.server.security.encryption.CredentialStoreServiceImpl; import org.apache.ambari.server.security.encryption.EncryptionService; import org.apache.ambari.server.security.encryption.Encryptor; -import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerFactory; import org.apache.ambari.server.serveraction.users.CollectionPersisterService; import org.apache.ambari.server.serveraction.users.CollectionPersisterServiceFactory; @@ -337,7 +336,7 @@ protected void configure() { bind(CredentialStoreService.class).to(CredentialStoreServiceImpl.class); bind(EncryptionService.class).to(AESEncryptionService.class); //to support different Encryptor implementation we have to annotate them by their name and use them as @Named injects - bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).to(PasswordPropertiesEncryptor.class); + bind(new TypeLiteral>() {}).annotatedWith(Names.named("ConfigPropertiesEncryptor")).to(ConfigPropertiesEncryptor.class); bind(Configuration.class).toInstance(configuration); bind(OsFamily.class).toInstance(os_family); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/ConfigPropertiesEncryptor.java similarity index 77% rename from ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java rename to ambari-server/src/main/java/org/apache/ambari/server/security/encryption/ConfigPropertiesEncryptor.java index 6e82f5de581..68710f46240 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/PasswordPropertiesEncryptor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/ConfigPropertiesEncryptor.java @@ -26,6 +26,7 @@ import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.AmbariRuntimeException; import org.apache.ambari.server.state.Cluster; +import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.PropertyInfo.PropertyType; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.utils.TextEncoding; @@ -36,11 +37,11 @@ /** * {@link Encryptor} implementation for encrypting/decrypting PASSWORD type - * properties + * properties in {@link Config}'s properties */ @Singleton -public class PasswordPropertiesEncryptor implements Encryptor> { +public class ConfigPropertiesEncryptor implements Encryptor { private static final String ENCRYPTED_PROPERTY_PREFIX = "${enc=aes256_hex, value="; private static final String ENCRYPTED_PROPERTY_SCHEME = ENCRYPTED_PROPERTY_PREFIX + "%s}"; @@ -49,39 +50,26 @@ public class PasswordPropertiesEncryptor implements Encryptor>>> clusterPasswordProperties = new ConcurrentHashMap<>(); //Map>>>; @Inject - public PasswordPropertiesEncryptor(EncryptionService encryptionService) { + public ConfigPropertiesEncryptor(EncryptionService encryptionService) { this.encryptionService = encryptionService; } - /** - * It is a must that any caller of this method adds the following additional - * information on the following indexes in additionalInfo with the - * following types: - *
    - *
  • [0] - org.apache.ambari.server.state.Cluster (the cluster - * whose properties should be scanned for PASSWORD properties)
    • - *
  • [1] - java.lang.String (to keep properties only that match - * this configuration type )
    • - *
- */ @Override - public Map encryptSensitiveData(Map properties, Object... additionaInfo) { + public void encryptSensitiveData(Config config) { try { - if (properties != null) { - final Map encryptedProperties = new HashMap<>(properties); - final Cluster cluster = (Cluster) additionaInfo[0]; - final String configType = (String) additionaInfo[1]; - final Set passwordProperties = getPasswordProperties(cluster, configType); + final Map configProperties = config.getProperties(); + if (configProperties != null) { + final Set passwordProperties = getPasswordProperties(config.getCluster(), config.getType()); if (CollectionUtils.isNotEmpty(passwordProperties)) { - for (Map.Entry property : properties.entrySet()) { + final Map encryptedProperties = new HashMap<>(configProperties); + for (Map.Entry property : configProperties.entrySet()) { if (passwordProperties.contains(property.getKey()) && !isEncryptedPassword(property.getValue())) { encryptedProperties.put(property.getKey(), encryptAndDecoratePropertyValue(property.getValue())); } } + config.setProperties(encryptedProperties); } - return encryptedProperties; } - return properties; } catch (Exception e) { throw new AmbariRuntimeException("Error while encrypting sensitive data", e); } @@ -114,17 +102,17 @@ private String encryptAndDecoratePropertyValue(String propertyValue) throws Exce } @Override - public Map decryptSensitiveData(Map properties, Object... additionalInfo) { - if (properties != null) { - final Map decryptedProperties = new HashMap<>(properties); - for (Map.Entry property : properties.entrySet()) { + public void decryptSensitiveData(Config config) { + final Map configProperties = config.getProperties(); + if (configProperties != null) { + final Map decryptedProperties = new HashMap<>(configProperties); + for (Map.Entry property : configProperties.entrySet()) { if (isEncryptedPassword(property.getValue())) { decryptedProperties.put(property.getKey(), decryptProperty(property.getValue())); } } - return decryptedProperties; + config.setProperties(decryptedProperties); } - return properties; } private String decryptProperty(String property) { diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java index 3936fbf88c7..4fad7237df0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/encryption/Encryptor.java @@ -27,23 +27,17 @@ public interface Encryptor { * * @param encryptible * to be encrypted - * @param additionalInfo - * any additional information that is needed during the encryption - * process (may be null) * @return the encrypted value */ - T encryptSensitiveData(T encryptible, Object... additionalInfo); + void encryptSensitiveData(T encryptible); /** * Decrypts the given decryptible object * * @param decryptible * to be decrypted - * @param additionalInfo - * any additional information that is needed during the decryption - * process (may be null) * @return the decrypted value */ - T decryptSensitiveData(T decryptible, Object... additionalInfo); + void decryptSensitiveData(T decryptible); } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java index c1a8dc1e371..d687e84d503 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java @@ -97,4 +97,9 @@ public interface Config { * Persist the configuration. */ void save(); + + /** + * @return the cluster where this config belongs to + */ + Cluster getCluster(); } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java index e082b40d9fa..2e3fbb383ba 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java @@ -102,8 +102,6 @@ public class ConfigImpl implements Config { private final AmbariEventPublisher eventPublisher; - private final Encryptor> passwordPropertiesEncryptor; - @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted("type") String type, @Assisted("tag") @Nullable String tag, @@ -111,9 +109,9 @@ public class ConfigImpl implements Config { @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, StackDAO stackDAO, Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, - Configuration serverConfiguration, @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { + Configuration serverConfiguration, @Named("ConfigPropertiesEncryptor") Encryptor configPropertiesEncryptor) { this(cluster.getDesiredStackVersion(), cluster, type, tag, properties, propertiesAttributes, - clusterDAO, stackDAO, gson, eventPublisher, lockFactory, serverConfiguration, passwordPropertiesEncryptor); + clusterDAO, stackDAO, gson, eventPublisher, lockFactory, serverConfiguration, configPropertiesEncryptor); } @@ -124,14 +122,16 @@ public class ConfigImpl implements Config { @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, StackDAO stackDAO, Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, - Configuration serverConfiguration, @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { + Configuration serverConfiguration, @Named("ConfigPropertiesEncryptor") Encryptor configPropertiesEncryptor) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.cluster = cluster; this.type = type; - this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; - this.properties = serverConfiguration.shouldEncryptSensitiveData() ? passwordPropertiesEncryptor.encryptSensitiveData(properties, cluster, type) : properties; + this.properties = properties; + if (serverConfiguration.shouldEncryptSensitiveData()) { + configPropertiesEncryptor.encryptSensitiveData(this); + } // only set this if it's non-null this.propertiesAttributes = null == propertiesAttributes ? null @@ -170,13 +170,13 @@ public class ConfigImpl implements Config { persist(entity); configId = entity.getConfigId(); + configPropertiesEncryptor.decryptSensitiveData(this); } @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher, - LockFactory lockFactory, - @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { + LockFactory lockFactory) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.cluster = cluster; @@ -194,8 +194,6 @@ public class ConfigImpl implements Config { propertiesTypes = cluster.getConfigPropertiesTypes(type); - this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; - // incur the hit on deserialization since this business object is stored locally try { Map deserializedProperties = gson.> fromJson( @@ -242,14 +240,12 @@ public class ConfigImpl implements Config { @Assisted("tag") @Nullable String tag, @Assisted Map properties, @Assisted @Nullable Map> propertiesAttributes, ClusterDAO clusterDAO, - Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory, - @Named("PasswordPropertiesEncryptor") Encryptor> passwordPropertiesEncryptor) { + Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.tag = tag; this.type = type; - this.passwordPropertiesEncryptor = passwordPropertiesEncryptor; this.properties = new HashMap<>(properties); this.propertiesAttributes = null == propertiesAttributes ? null : new HashMap<>(propertiesAttributes); @@ -300,7 +296,7 @@ public Long getVersion() { public Map getProperties() { propertyLock.readLock().lock(); try { - return properties == null ? new HashMap<>() : passwordPropertiesEncryptor.decryptSensitiveData(properties); + return properties == null ? new HashMap<>() : new HashMap<>(properties); } finally { propertyLock.readLock().unlock(); } @@ -337,6 +333,11 @@ public void updateProperties(Map propertiesToUpdate) { } } + @Override + public Cluster getCluster() { + return cluster; + } + @Override public List getServiceConfigVersions() { return serviceConfigDAO.getServiceConfigVersionsByConfig(cluster.getClusterId(), type, version); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java index ac6bba8505f..27b46787a17 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java @@ -245,7 +245,7 @@ void updateHostsInConfigurations() throws AmbariException { for (Map.Entry property : config.getProperties().entrySet()) { updatedPropertyValue = replaceHosts(property.getValue(), currentHostNames, hostMapping); - + if (updatedPropertyValue != null) { Map propertiesWithUpdates = config.getProperties(); propertiesWithUpdates.put(property.getKey(), updatedPropertyValue); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java index 83a9d6bec8f..d992d4a67ac 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java @@ -23,8 +23,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.util.Map; - import javax.persistence.EntityManager; import javax.ws.rs.core.MediaType; @@ -62,11 +60,11 @@ import org.apache.ambari.server.security.SecurityHelper; import org.apache.ambari.server.security.SecurityHelperImpl; import org.apache.ambari.server.security.encryption.AESEncryptionService; +import org.apache.ambari.server.security.encryption.ConfigPropertiesEncryptor; import org.apache.ambari.server.security.encryption.CredentialStoreService; import org.apache.ambari.server.security.encryption.CredentialStoreServiceImpl; import org.apache.ambari.server.security.encryption.EncryptionService; import org.apache.ambari.server.security.encryption.Encryptor; -import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.stack.upgrade.orchestrate.UpgradeContextFactory; import org.apache.ambari.server.stageplanner.RoleGraphFactory; @@ -360,7 +358,7 @@ protected void configure() { bind(KerberosHelper.class).toInstance(createNiceMock(KerberosHelper.class)); bind(MpackManagerFactory.class).toInstance(createNiceMock(MpackManagerFactory.class)); bind(EncryptionService.class).to(AESEncryptionService.class); - bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).to(PasswordPropertiesEncryptor.class); + bind(new TypeLiteral>() {}).annotatedWith(Names.named("ConfigPropertiesEncryptor")).to(ConfigPropertiesEncryptor.class); } private void installDependencies() { diff --git a/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java b/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java index 19cd83c13af..94d64ac72fe 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/testutils/PartialNiceMockBinder.java @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Map; import javax.persistence.EntityManager; @@ -138,10 +137,6 @@ public Builder addAlertDefinitionBinding() { } public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagementController) { - return addAmbariMetaInfoBinding(ambariManagementController, easyMockSupport.createNiceMock(Encryptor.class)); - } - - public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagementController, Encryptor> passwordEncryptor) { configurers.add((Binder binder) -> { binder.bind(PersistedState.class).toInstance(easyMockSupport.createNiceMock(PersistedState.class)); binder.bind(HostRoleCommandFactory.class).to(HostRoleCommandFactoryImpl.class); @@ -161,7 +156,7 @@ public Builder addAmbariMetaInfoBinding(AmbariManagementController ambariManagem }); addConfigsBindings(); addFactoriesInstallBinding(); - addPasswordEncryptorBindings(passwordEncryptor); + addPasswordEncryptorBindings(); return this; } @@ -216,13 +211,9 @@ public Builder addConfigsBindings() { } public Builder addPasswordEncryptorBindings() { - return addPasswordEncryptorBindings(easyMockSupport.createNiceMock(Encryptor.class)); - } - - public Builder addPasswordEncryptorBindings(Encryptor> passwordEncryptor) { configurers.add((Binder binder) -> { binder.bind(EncryptionService.class).toInstance(easyMockSupport.createNiceMock(EncryptionService.class)); - binder.bind(new TypeLiteral>>() {}).annotatedWith(Names.named("PasswordPropertiesEncryptor")).toInstance(passwordEncryptor); + binder.bind(new TypeLiteral>() {}).annotatedWith(Names.named("ConfigPropertiesEncryptor")).toInstance(easyMockSupport.createNiceMock(Encryptor.class)); }); return this; } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java index dc167cb10e7..028f888d778 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/update/HostUpdateHelperTest.java @@ -67,7 +67,6 @@ import org.apache.ambari.server.scheduler.ExecutionScheduler; import org.apache.ambari.server.scheduler.ExecutionSchedulerImpl; import org.apache.ambari.server.security.encryption.CredentialStoreService; -import org.apache.ambari.server.security.encryption.PasswordPropertiesEncryptor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; @@ -87,7 +86,6 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.StandardPasswordEncoder; -import com.google.common.collect.ImmutableMap; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import com.google.inject.AbstractModule; @@ -248,7 +246,6 @@ public void testUpdateHostsInConfigurations() throws AmbariException { ClusterConfigEntity mockClusterConfigEntity1 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); ClusterConfigEntity mockClusterConfigEntity2 = easyMockSupport.createNiceMock(ClusterConfigEntity.class); StackEntity mockStackEntity = easyMockSupport.createNiceMock(StackEntity.class); - final PasswordPropertiesEncryptor passwordEncryptorMock = easyMockSupport.createNiceMock(PasswordPropertiesEncryptor.class); Map> clusterHostsToChange = new HashMap<>(); Map hosts = new HashMap<>(); List clusterConfigEntities1 = new ArrayList<>(); @@ -256,7 +253,7 @@ public void testUpdateHostsInConfigurations() throws AmbariException { final Injector mockInjector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - PartialNiceMockBinder.newBuilder().addAmbariMetaInfoBinding(mockAmbariManagementController, passwordEncryptorMock).build().configure(binder()); + PartialNiceMockBinder.newBuilder().addClustersBinding(mockAmbariManagementController).build().configure(binder()); bind(StackManagerFactory.class).toInstance(easyMockSupport.createNiceMock(StackManagerFactory.class)); bind(DBAccessor.class).toInstance(dbAccessor); bind(EntityManager.class).toInstance(entityManager); @@ -304,9 +301,6 @@ protected void configure() { expect(mockClusterConfigEntity1.getType()).andReturn("testType1").atLeastOnce(); expect(mockClusterConfigEntity1.getVersion()).andReturn(1L).atLeastOnce(); expect(mockClusterDAO.findConfig(1L)).andReturn(mockClusterConfigEntity1).atLeastOnce(); - final Map expectedMap1 = new HashMap<>(ImmutableMap.of("testProperty1", "testValue_host1", "testProperty2", "testValue_host5", "testProperty3", - "testValue_host11", "testProperty4", "testValue_host55")); - expect(passwordEncryptorMock.decryptSensitiveData(expectedMap1)).andReturn(expectedMap1).atLeastOnce(); expect(mockClusterConfigEntity2.getClusterId()).andReturn(1L).atLeastOnce(); expect(mockClusterConfigEntity2.getConfigId()).andReturn(2L).anyTimes(); @@ -316,8 +310,6 @@ protected void configure() { expect(mockClusterConfigEntity2.getType()).andReturn("testType2").atLeastOnce(); expect(mockClusterConfigEntity2.getVersion()).andReturn(2L).atLeastOnce(); expect(mockClusterDAO.findConfig(2L)).andReturn(mockClusterConfigEntity2).atLeastOnce(); - final Map expectedMap2 = new HashMap<>(ImmutableMap.of("testProperty5", "test_host1_test_host5_test_host11_test_host55")); - expect(passwordEncryptorMock.decryptSensitiveData(expectedMap2)).andReturn(expectedMap2).atLeastOnce(); Capture dataCapture = EasyMock.newCapture(); mockClusterConfigEntity1.setData(EasyMock.capture(dataCapture)); @@ -325,7 +317,7 @@ protected void configure() { mockClusterConfigEntity2.setData("{\"testProperty5\":\"test_host5_test_host1_test_host55_test_host11\"}"); expectLastCall(); - + HostUpdateHelper hostUpdateHelper = new HostUpdateHelper(null, null, mockInjector); hostUpdateHelper.setHostChangesFileMap(clusterHostsToChange); From 587d796267ba8818504285c9064d7080a0714c0a Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Fri, 19 Oct 2018 14:09:03 +0200 Subject: [PATCH 6/6] AMBARI-24742. Missing decryption when loading data from DB --- .../main/java/org/apache/ambari/server/state/ConfigImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java index 2e3fbb383ba..e04a6bbf0ed 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java @@ -176,7 +176,7 @@ public class ConfigImpl implements Config { @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher, - LockFactory lockFactory) { + LockFactory lockFactory, @Named("ConfigPropertiesEncryptor") Encryptor configPropertiesEncryptor) { propertyLock = lockFactory.newReadWriteLock(PROPERTY_LOCK_LABEL); this.cluster = cluster; @@ -204,6 +204,7 @@ public class ConfigImpl implements Config { } properties = deserializedProperties; + configPropertiesEncryptor.decryptSensitiveData(this); } catch (JsonSyntaxException e) { LOG.error("Malformed configuration JSON stored in the database for {}/{}", entity.getType(), entity.getTag());