Skip to content

Normalize encryption on global configurations values #6812

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 8 commits into from
Sep 28, 2023
Merged

Normalize encryption on global configurations values #6812

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
272f2ee
Normalize global configs to non encrypted values, except those in the…
BryanMLima Jul 7, 2023
1f8b9c4
Fix db migration and address legacy configs
BryanMLima Aug 28, 2023
1957962
Revert getValue
BryanMLima Aug 28, 2023
6112b56
Change upgrade path to 4.19
BryanMLima Sep 18, 2023
4a16141
Merge branch 'main' of github.com:apache/cloudstack into normalize-gl…
BryanMLima Sep 18, 2023
4e28683
Fix double encryption of values
BryanMLima Sep 19, 2023
b836fc4
Fix - encrypt scoped configurations
BryanMLima Sep 19, 2023
88d32c7
fix return value
BryanMLima Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Normalize global configs to non encrypted values, except those in the… 
… Secure/Hidden categories
  • Loading branch information
@BryanMLima
BryanMLima committed Sep 18, 2023
commit 272f2ee9a50899cbb3be128e155316fa06a8f6dc
58 changes: 42 additions & 16 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java
View file
Open in desktop
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);
Comment thread
BryanMLima marked this conversation as resolved.
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(cfg.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
Open in desktop
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
64 changes: 64 additions & 0 deletions engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,29 @@
package com.cloud.upgrade.dao;

import com.cloud.upgrade.SystemVmTemplateRegistration;
import com.cloud.utils.crypt.DBEncryptionUtil;
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.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Upgrade41800to41810 implements DbUpgrade, DbUpgradeSystemVmTemplate {
final static Logger LOG = Logger.getLogger(Upgrade41800to41810.class);
private SystemVmTemplateRegistration systemVmTemplateRegistration;

static final String ACCOUNT_DETAILS = "account_details";

static final String DOMAIN_DETAILS = "domain_details";

@Override
public String[] getUpgradableVersionRange() {
return new String[] {"4.18.0.0", "4.18.1.0"};
Expand Down Expand Up @@ -58,6 +69,7 @@ public InputStream[] getPrepareScripts() {
@Override
public void performDataMigration(Connection conn) {
fixForeignKeyNames(conn);
decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn);
}

@Override
Expand Down Expand Up @@ -111,4 +123,56 @@ private void fixForeignKeyNames(Connection conn) {
DbUpgradeUtils.dropKeysIfExist(conn, "cloud.volumes", keys, false);
DbUpgradeUtils.addForeignKey(conn, "volumes", "passphrase_id","passphrase", "id");
}

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);
}
}

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);
}
}
}
}
3 changes: 0 additions & 3 deletions engine/schema/src/main/java/com/cloud/user/AccountDetailVO.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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");
Comment thread
BryanMLima marked this conversation as resolved.

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 NOT 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");
Comment thread
BryanMLima marked this conversation as resolved.
}

private void migrateEncryptedDomainDetails(Connection conn) {
System.out.println("Beginning migration of domain_details encrypted values");
Comment thread
BryanMLima marked this conversation as resolved.

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 NOT 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 domain details values");
Comment thread
BryanMLima marked this conversation as resolved.
}

protected String migrateValue(String value) {
if (StringUtils.isEmpty(value)) {
return value;
Expand Down
20 changes: 14 additions & 6 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import javax.naming.ConfigurationException;


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 @@ -865,17 +866,15 @@ public String updateConfiguration(final long userId, final String name, final St
public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidParameterValueException {
final Long userId = CallContext.current().getCallingUserId();
final String name = cmd.getCfgName();
String value = cmd.getValue();
final Long zoneId = cmd.getZoneId();
final Long clusterId = cmd.getClusterId();
final Long storagepoolId = cmd.getStoragepoolId();
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;

final Account caller = CallContext.current().getCallingAccount();
if (_accountMgr.isDomainAdmin(caller.getId())) {
Expand All @@ -894,11 +893,20 @@ 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();
}

String value = cmd.getValue();
boolean isConfigEncrypted = config != null && config.isEncrypted();
if (isConfigEncrypted) {
value = DBEncryptionUtil.encrypt(value);
}

String eventValue = isConfigEncrypted ? "*****" : Objects.requireNonNullElse(value, "");
CallContext.current().setEventDetails(String.format(" Name: %s New Value: %s", name, eventValue));

validateIpAddressRelatedConfigValues(name, value);

if (value == null) {
Expand Down Expand Up @@ -953,7 +961,7 @@ 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 {
Expand Down