This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new ae5dda867fd Normalize encryption on global configurations values
(#6812)
ae5dda867fd is described below
commit ae5dda867fdd9535d97086a96668d284c5d3fd03
Author: Bryan Lima <[email protected]>
AuthorDate: Thu Sep 28 11:44:36 2023 -0300
Normalize encryption on global configurations values (#6812)
---
.../api/command/admin/config/UpdateCfgCmd.java | 58 ++++++++++++++------
.../main/java/com/cloud/domain/DomainDetailVO.java | 2 -
.../com/cloud/upgrade/dao/Upgrade41810to41900.java | 61 ++++++++++++++++++++++
.../main/java/com/cloud/user/AccountDetailVO.java | 3 --
.../framework/config/impl/ConfigurationVO.java | 2 +-
.../utils/crypt/EncryptionSecretKeyChanger.java | 26 +++++++++
.../configuration/ConfigurationManagerImpl.java | 33 +++++++++---
7 files changed, 156 insertions(+), 29 deletions(-)
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
index abe7e31cfa2..63dc51452f0 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
@@ -16,6 +16,7 @@
// under the License.
package org.apache.cloudstack.api.command.admin.config;
+import com.cloud.utils.crypt.DBEncryptionUtil;
import org.apache.cloudstack.acl.RoleService;
import org.apache.cloudstack.api.response.DomainResponse;
import org.apache.log4j.Logger;
@@ -150,25 +151,50 @@ public class UpdateCfgCmd extends BaseCmd {
if (cfg != null) {
ConfigurationResponse response =
_responseGenerator.createConfigurationResponse(cfg);
response.setResponseName(getCommandName());
- if (getZoneId() != null) {
- response.setScope("zone");
- }
- if (getClusterId() != null) {
- response.setScope("cluster");
- }
- if (getStoragepoolId() != null) {
- response.setScope("storagepool");
- }
- if (getAccountId() != null) {
- response.setScope("account");
- }
- if (getDomainId() != null) {
- response.setScope("domain");
- }
- response.setValue(value);
+ response = setResponseScopes(response);
+ response = setResponseValue(response, cfg);
this.setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed
to update config");
}
}
+
+ /**
+ * Sets the configuration value in the response. If the configuration is
in the `Hidden` or `Secure` categories, the value is encrypted before being set
in the response.
+ * @param response to be set with the configuration `cfg` value
+ * @param cfg to be used in setting the response value
+ * @return the response with the configuration's value
+ */
+ public ConfigurationResponse setResponseValue(ConfigurationResponse
response, Configuration cfg) {
+ if (cfg.isEncrypted()) {
+ response.setValue(DBEncryptionUtil.encrypt(getValue()));
+ } else {
+ response.setValue(getValue());
+ }
+ return response;
+ }
+
+ /**
+ * Sets the scope for the Configuration response only if the field is not
null.
+ * @param response to be updated
+ * @return the response updated with the scopes
+ */
+ public ConfigurationResponse setResponseScopes(ConfigurationResponse
response) {
+ if (getZoneId() != null) {
+ response.setScope("zone");
+ }
+ if (getClusterId() != null) {
+ response.setScope("cluster");
+ }
+ if (getStoragepoolId() != null) {
+ response.setScope("storagepool");
+ }
+ if (getAccountId() != null) {
+ response.setScope("account");
+ }
+ if (getDomainId() != null) {
+ response.setScope("domain");
+ }
+ return response;
+ }
}
diff --git a/engine/schema/src/main/java/com/cloud/domain/DomainDetailVO.java
b/engine/schema/src/main/java/com/cloud/domain/DomainDetailVO.java
index 61eb6cfd28e..df5a2283baa 100644
--- a/engine/schema/src/main/java/com/cloud/domain/DomainDetailVO.java
+++ b/engine/schema/src/main/java/com/cloud/domain/DomainDetailVO.java
@@ -23,7 +23,6 @@ import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.Table;
-import com.cloud.utils.db.Encrypt;
import org.apache.cloudstack.api.InternalIdentity;
@Entity
@@ -40,7 +39,6 @@ public class DomainDetailVO implements InternalIdentity {
@Column(name = "name")
private String name;
- @Encrypt
@Column(name = "value")
private String value;
diff --git
a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
index 57f87d1f2cc..fd44e79e7cf 100644
--- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
+++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
@@ -17,15 +17,19 @@
package com.cloud.upgrade.dao;
import com.cloud.upgrade.SystemVmTemplateRegistration;
+import com.cloud.utils.crypt.DBEncryptionUtil;
import com.cloud.utils.DateUtil;
import com.cloud.utils.exception.CloudRuntimeException;
import org.apache.log4j.Logger;
+import org.jasypt.exceptions.EncryptionOperationNotPossibleException;
import java.io.InputStream;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
@@ -34,6 +38,10 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
final static Logger LOG = Logger.getLogger(Upgrade41810to41900.class);
private SystemVmTemplateRegistration systemVmTemplateRegistration;
+ private static final String ACCOUNT_DETAILS = "account_details";
+
+ private static final String DOMAIN_DETAILS = "domain_details";
+
private final SimpleDateFormat[] formats = {
new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"), new
SimpleDateFormat("MM/dd/yyyy HH:mm:ss"), new SimpleDateFormat("dd/MM/yyyy
HH:mm:ss"),
new SimpleDateFormat("EEE MMM dd HH:mm:ss z yyyy")};
@@ -66,6 +74,7 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
@Override
public void performDataMigration(Connection conn) {
+
decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn);
migrateBackupDates(conn);
}
@@ -95,6 +104,37 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
}
}
+ protected void
decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(Connection
conn) {
+ LOG.info("Decrypting global configuration values from the following
tables: account_details and domain_details.");
+
+ Map<Long, String> accountsMap = getConfigsWithScope(conn,
ACCOUNT_DETAILS);
+ updateConfigValuesWithScope(conn, accountsMap, ACCOUNT_DETAILS);
+ LOG.info("Successfully decrypted configurations from account_details
table.");
+
+ Map<Long, String> domainsMap = getConfigsWithScope(conn,
DOMAIN_DETAILS);
+ updateConfigValuesWithScope(conn, domainsMap, DOMAIN_DETAILS);
+ LOG.info("Successfully decrypted configurations from domain_details
table.");
+ }
+
+ protected Map<Long, String> getConfigsWithScope(Connection conn, String
table) {
+ Map<Long, String> configsToBeUpdated = new HashMap<>();
+ String selectDetails = String.format("SELECT details.id, details.value
from cloud.%s details, cloud.configuration c " +
+ "WHERE details.name = c.name AND c.category NOT IN ('Hidden',
'Secure') AND details.value <> \"\" ORDER BY details.id;", table);
+
+ try (PreparedStatement pstmt = conn.prepareStatement(selectDetails)) {
+ try (ResultSet result = pstmt.executeQuery()) {
+ while (result.next()) {
+ configsToBeUpdated.put(result.getLong("id"),
result.getString("value"));
+ }
+ }
+ return configsToBeUpdated;
+ } catch (SQLException e) {
+ String message = String.format("Unable to retrieve data from table
[%s] due to [%s].", table, e.getMessage());
+ LOG.error(message, e);
+ throw new CloudRuntimeException(message, e);
+ }
+ }
+
public void migrateBackupDates(Connection conn) {
LOG.info("Trying to convert backups' date column from varchar(255) to
datetime type.");
@@ -125,6 +165,27 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
}
}
+ protected void updateConfigValuesWithScope(Connection conn, Map<Long,
String> configsToBeUpdated, String table) {
+ String updateConfigValues = String.format("UPDATE cloud.%s SET value =
? WHERE id = ?;", table);
+
+ for (Map.Entry<Long, String> config : configsToBeUpdated.entrySet()) {
+ try (PreparedStatement pstmt =
conn.prepareStatement(updateConfigValues)) {
+ String decryptedValue =
DBEncryptionUtil.decrypt(config.getValue());
+
+ pstmt.setString(1, decryptedValue);
+ pstmt.setLong(2, config.getKey());
+
+ LOG.info(String.format("Updating config with ID [%s] to value
[%s].", config.getKey(), decryptedValue));
+ pstmt.executeUpdate();
+ } catch (SQLException | EncryptionOperationNotPossibleException e)
{
+ String message = String.format("Unable to update config value
with ID [%s] on table [%s] due to [%s]. The config value may already be
decrypted.",
+ config.getKey(), table, e);
+ LOG.error(message);
+ throw new CloudRuntimeException(message, e);
+ }
+ }
+ }
+
private void fetchDatesAndMigrateToNewColumn(Connection conn) {
String selectBackupDates = "SELECT `id`, `old_date` FROM
`cloud`.`backups` WHERE 1;";
String date;
diff --git a/engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
b/engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
index 71ad765e618..863f6c96008 100644
--- a/engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
+++ b/engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
@@ -25,8 +25,6 @@ import javax.persistence.Table;
import org.apache.cloudstack.api.InternalIdentity;
-import com.cloud.utils.db.Encrypt;
-
@Entity
@Table(name = "account_details")
public class AccountDetailVO implements InternalIdentity {
@@ -41,7 +39,6 @@ public class AccountDetailVO implements InternalIdentity {
@Column(name = "name")
private String name;
- @Encrypt
@Column(name = "value", length=4096)
private String value;
diff --git
a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java
b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java
index 08ec9bfe83f..c705cc64072 100644
---
a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java
+++
b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java
@@ -170,7 +170,7 @@ public class ConfigurationVO implements Configuration {
@Override
public boolean isEncrypted() {
- return "Hidden".equals(getCategory()) ||
"Secure".equals(getCategory());
+ return StringUtils.equalsAny(getCategory(), "Hidden", "Secure");
}
@Override
diff --git
a/framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java
b/framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java
index 88830b3e3f9..961c537d0da 100644
---
a/framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java
+++
b/framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java
@@ -475,6 +475,8 @@ public class EncryptionSecretKeyChanger {
// migrate resource details values
migrateHostDetails(conn);
+ migrateEncryptedAccountDetails(conn);
+ migrateEncryptedDomainDetails(conn);
migrateClusterDetails(conn);
migrateImageStoreDetails(conn);
migrateStoragePoolDetails(conn);
@@ -497,6 +499,30 @@ public class EncryptionSecretKeyChanger {
return true;
}
+ private void migrateEncryptedAccountDetails(Connection conn) {
+ System.out.println("Beginning migration of account_details encrypted
values");
+
+ String tableName = "account_details";
+ String selectSql = "SELECT details.id, details.value from
account_details details, cloud.configuration c " +
+ "WHERE details.name = c.name AND c.category IN ('Hidden',
'Secure') AND details.value <> \"\" ORDER BY details.id;";
+ String updateSql = "UPDATE cloud.account_details SET value = ? WHERE
id = ?;";
+ migrateValueAndUpdateDatabaseById(conn, tableName, selectSql,
updateSql, false);
+
+ System.out.println("End migration of account details values");
+ }
+
+ private void migrateEncryptedDomainDetails(Connection conn) {
+ System.out.println("Beginning migration of domain_details encrypted
values");
+
+ String tableName = "domain_details";
+ String selectSql = "SELECT details.id, details.value from
domain_details details, cloud.configuration c " +
+ "WHERE details.name = c.name AND c.category IN ('Hidden',
'Secure') AND details.value <> \"\" ORDER BY details.id;";
+ String updateSql = "UPDATE cloud.domain_details SET value = ? WHERE id
= ?;";
+ migrateValueAndUpdateDatabaseById(conn, tableName, selectSql,
updateSql, false);
+
+ System.out.println("End migration of domain details values");
+ }
+
protected String migrateValue(String value) {
if (StringUtils.isEmpty(value)) {
return value;
diff --git
a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index ef617ed4cb7..18cebff87fe 100644
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -47,6 +47,7 @@ import javax.naming.ConfigurationException;
import com.cloud.hypervisor.HypervisorGuru;
+import com.cloud.utils.crypt.DBEncryptionUtil;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -664,7 +665,7 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
@Override
@DB
- public String updateConfiguration(final long userId, final String name,
final String category, final String value, final String scope, final Long
resourceId) {
+ public String updateConfiguration(final long userId, final String name,
final String category, String value, final String scope, final Long resourceId)
{
final String validationMsg = validateConfigurationValue(name, value,
scope);
if (validationMsg != null) {
@@ -677,6 +678,11 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
// if scope is mentioned as global or not mentioned then it is normal
// global parameter updation
if (scope != null && !scope.isEmpty() &&
!ConfigKey.Scope.Global.toString().equalsIgnoreCase(scope)) {
+ boolean valueEncrypted = shouldEncryptValue(category);
+ if (valueEncrypted) {
+ value = DBEncryptionUtil.encrypt(value);
+ }
+
switch (ConfigKey.Scope.valueOf(scope)) {
case Zone:
final DataCenterVO zone = _zoneDao.findById(resourceId);
@@ -767,7 +773,8 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
default:
throw new InvalidParameterValueException("Scope provided is
invalid");
}
- return value;
+
+ return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
}
// Execute all updates in a single transaction
@@ -864,6 +871,10 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
return _configDao.getValue(name);
}
+ private boolean shouldEncryptValue(String category) {
+ return StringUtils.equalsAny(category, "Hidden", "Secure");
+ }
+
/**
* Updates the 'hypervisor.list' value to match the new custom hypervisor
name set as newValue if the previous value was set
*/
@@ -890,10 +901,11 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
final Long imageStoreId = cmd.getImageStoreId();
Long accountId = cmd.getAccountId();
Long domainId = cmd.getDomainId();
- CallContext.current().setEventDetails(" Name: " + name + " New Value:
" + (name.toLowerCase().contains("password") ? "*****" : value == null ? "" :
value));
// check if config value exists
final ConfigurationVO config = _configDao.findByName(name);
- String catergory = null;
+ String category = null;
+ String eventValue = encryptEventValueIfConfigIsEncrypted(config,
value);
+ CallContext.current().setEventDetails(String.format(" Name: %s New
Value: %s", name, eventValue));
final Account caller = CallContext.current().getCallingAccount();
if (_accountMgr.isDomainAdmin(caller.getId())) {
@@ -912,9 +924,9 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
s_logger.warn("Probably the component manager where
configuration variable " + name + " is defined needs to implement Configurable
interface");
throw new InvalidParameterValueException("Config parameter
with name " + name + " doesn't exist");
}
- catergory = _configDepot.get(name).category();
+ category = _configDepot.get(name).category();
} else {
- catergory = config.getCategory();
+ category = config.getCategory();
}
validateIpAddressRelatedConfigValues(name, value);
@@ -971,7 +983,7 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
value = (id == null) ? null : "";
}
- final String updatedValue = updateConfiguration(userId, name,
catergory, value, scope, id);
+ final String updatedValue = updateConfiguration(userId, name,
category, value, scope, id);
if (value == null && updatedValue == null ||
updatedValue.equalsIgnoreCase(value)) {
return _configDao.findByName(name);
} else {
@@ -979,6 +991,13 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
}
}
+ private String encryptEventValueIfConfigIsEncrypted(ConfigurationVO
config, String value) {
+ if (config != null && config.isEncrypted()) {
+ return "*****";
+ }
+ return Objects.requireNonNullElse(value, "");
+ }
+
private ParamCountPair getParamCount(Map<String, Long> scopeMap) {
Long id = null;
int paramCount = 0;
