Skip to content

Commit

Permalink
Normalize encryption on global configurations values (#6812)
Browse files Browse the repository at this point in the history
  • Loading branch information
@BryanMLima
BryanMLima committed Sep 28, 2023
1 parent 8939ebb commit ae5dda8
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 29 deletions.
58 changes: 42 additions & 16 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -150,25 +151,50 @@ public void execute() {
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;
}
}
2 changes: 0 additions & 2 deletions engine/schema/src/main/java/com/cloud/domain/DomainDetailVO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import javax.persistence.Id;
import javax.persistence.Table;

import com.cloud.utils.db.Encrypt;
import org.apache.cloudstack.api.InternalIdentity;

@Entity
Expand All @@ -40,7 +39,6 @@ public class DomainDetailVO implements InternalIdentity {
@Column(name = "name")
private String name;

@Encrypt
@Column(name = "value")
private String value;

Expand Down
61 changes: 61 additions & 0 deletions engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")};
Expand Down Expand Up @@ -66,6 +74,7 @@ public InputStream[] getPrepareScripts() {

@Override
public void performDataMigration(Connection conn) {
decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn);
migrateBackupDates(conn);
}

Expand Down Expand Up @@ -95,6 +104,37 @@ public void updateSystemVmTemplates(Connection conn) {
}
}

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.");

Expand Down Expand Up @@ -125,6 +165,27 @@ private void modifyDateColumnNameAndCreateNewOne(Connection conn) {
}
}

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;
Expand Down
3 changes: 0 additions & 3 deletions engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

import org.apache.cloudstack.api.InternalIdentity;

import com.cloud.utils.db.Encrypt;

@Entity
@Table(name = "account_details")
public class AccountDetailVO implements InternalIdentity {
Expand All @@ -41,7 +39,6 @@ public class AccountDetailVO implements InternalIdentity {
@Column(name = "name")
private String name;

@Encrypt
@Column(name = "value", length=4096)
private String value;

Expand Down
2 changes: 1 addition & 1 deletion ...ork/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public void setValue(String value) {

@Override
public boolean isEncrypted() {
return "Hidden".equals(getCategory()) || "Secure".equals(getCategory());
return StringUtils.equalsAny(getCategory(), "Hidden", "Secure");
}

@Override
Expand Down
26 changes: 26 additions & 0 deletions framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ private boolean migrateData(String oldDBKey, String newDBKey, String oldEncrypto

// migrate resource details values
migrateHostDetails(conn);
migrateEncryptedAccountDetails(conn);
migrateEncryptedDomainDetails(conn);
migrateClusterDetails(conn);
migrateImageStoreDetails(conn);
migrateStoragePoolDetails(conn);
Expand All @@ -497,6 +499,30 @@ private boolean migrateData(String oldDBKey, String newDBKey, String oldEncrypto
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;
Expand Down
33 changes: 26 additions & 7 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@


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;
Expand Down Expand Up @@ -664,7 +665,7 @@ public boolean stop() {

@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) {
Expand All @@ -677,6 +678,11 @@ public String updateConfiguration(final long userId, final String name, final St
// 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);
Expand Down Expand Up @@ -767,7 +773,8 @@ public String updateConfiguration(final long userId, final String name, final St
default:
throw new InvalidParameterValueException("Scope provided is invalid");
}
return value;

return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
}

// Execute all updates in a single transaction
Expand Down Expand Up @@ -864,6 +871,10 @@ public String updateConfiguration(final long userId, final String name, final St
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
*/
Expand All @@ -890,10 +901,11 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
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())) {
Expand All @@ -912,9 +924,9 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
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);
Expand Down Expand Up @@ -971,14 +983,21 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
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 {
throw new CloudRuntimeException("Unable to update configuration parameter " + name);
}
}

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;
Expand Down

0 comments on commit ae5dda8

Please sign in to comment.