From 8b02624e610eda16d28b34982c3a055ed802773d Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 18 Jun 2024 23:37:13 +0530 Subject: [PATCH] User data content size validation, and related code improvements (#8418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> --- .../CreateAutoScaleVmProfileCmd.java | 2 +- .../UpdateAutoScaleVmProfileCmd.java | 2 +- .../user/userdata/RegisterUserDataCmd.java | 9 +- .../api/command/user/vm/DeployVMCmd.java | 6 +- .../command/user/vm/ResetVMUserDataCmd.java | 2 +- .../api/command/user/vm/UpdateVMCmd.java | 2 +- .../response/AutoScaleVmProfileResponse.java | 2 +- .../api/response/VMUserDataResponse.java | 2 +- .../cloudstack/userdata/UserDataManager.java | 5 + .../configuration/ConfigurationManager.java | 4 - .../userdata/UserDataManagerImpl.java | 91 ++++++++++--------- .../ConfigurationManagerImpl.java | 9 +- .../network/as/AutoScaleManagerImpl.java | 7 +- .../main/java/com/cloud/vm/UserVmManager.java | 2 - .../java/com/cloud/vm/UserVmManagerImpl.java | 55 +---------- .../network/as/AutoScaleManagerImplTest.java | 10 +- 16 files changed, 92 insertions(+), 118 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java index db6ccd9ce53..8c1d38c1375 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java @@ -102,7 +102,7 @@ public class CreateAutoScaleVmProfileCmd extends BaseAsyncCreateCmd { description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. " + "This binary data must be base64 encoded before adding it to the request. " + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + - "Using HTTP POST(via POST body), you can send up to 1MB of data after base64 encoding." + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + "You also need to change vm.userdata.max.length value", length = 1048576, since = "4.18.0") diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/UpdateAutoScaleVmProfileCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/UpdateAutoScaleVmProfileCmd.java index 3e65d38e520..34c81bfc483 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/UpdateAutoScaleVmProfileCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/UpdateAutoScaleVmProfileCmd.java @@ -97,7 +97,7 @@ public class UpdateAutoScaleVmProfileCmd extends BaseAsyncCustomIdCmd { description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. " + "This binary data must be base64 encoded before adding it to the request. " + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + - "Using HTTP POST(via POST body), you can send up to 1MB of data after base64 encoding." + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + "You also need to change vm.userdata.max.length value", length = 1048576, since = "4.18.0") diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/RegisterUserDataCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/RegisterUserDataCmd.java index f294f7dd8e0..3d44230cac1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/RegisterUserDataCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/RegisterUserDataCmd.java @@ -74,7 +74,14 @@ public class RegisterUserDataCmd extends BaseCmd { @Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, description = "an optional project for the userdata") private Long projectId; - @Parameter(name = ApiConstants.USER_DATA, type = CommandType.STRING, required = true, description = "Userdata content", length = 1048576) + @Parameter(name = ApiConstants.USER_DATA, + type = CommandType.STRING, + required = true, + description = "Base64 encoded userdata content. " + + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + + "You also need to change vm.userdata.max.length value", + length = 1048576) private String userData; @Parameter(name = ApiConstants.PARAMS, type = CommandType.STRING, description = "comma separated list of variables declared in userdata content") diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java index 1cbe28f4dde..e02111af59f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java @@ -156,7 +156,11 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd implements SecurityG private String hypervisor; @Parameter(name = ApiConstants.USER_DATA, type = CommandType.STRING, - description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. This binary data must be base64 encoded before adding it to the request. Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. Using HTTP POST(via POST body), you can send up to 1MB of data after base64 encoding.", + description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. " + + "This binary data must be base64 encoded before adding it to the request. " + + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + + "You also need to change vm.userdata.max.length value", length = 1048576) private String userData; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ResetVMUserDataCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ResetVMUserDataCmd.java index 3ead67e2106..7e0aab98760 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ResetVMUserDataCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ResetVMUserDataCmd.java @@ -62,7 +62,7 @@ public class ResetVMUserDataCmd extends BaseCmd implements UserCmd { description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. " + "This binary data must be base64 encoded before adding it to the request. " + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + - "Using HTTP POST(via POST body), you can send up to 1MB of data after base64 encoding." + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + "You also need to change vm.userdata.max.length value", length = 1048576) private String userData; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index 32ce1f6db52..4527e152f18 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -86,7 +86,7 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, description = "an optional binary data that can be sent to the virtual machine upon a successful deployment. " + "This binary data must be base64 encoded before adding it to the request. " + "Using HTTP GET (via querystring), you can send up to 4KB of data after base64 encoding. " + - "Using HTTP POST(via POST body), you can send up to 1MB of data after base64 encoding." + + "Using HTTP POST (via POST body), you can send up to 1MB of data after base64 encoding. " + "You also need to change vm.userdata.max.length value", length = 1048576, since = "4.16.0") diff --git a/api/src/main/java/org/apache/cloudstack/api/response/AutoScaleVmProfileResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/AutoScaleVmProfileResponse.java index 9f238344730..678c4fcaaca 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/AutoScaleVmProfileResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/AutoScaleVmProfileResponse.java @@ -69,7 +69,7 @@ public class AutoScaleVmProfileResponse extends BaseResponse implements Controll private Map counterParams; @SerializedName(ApiConstants.USER_DATA) - @Param(description = "Base 64 encoded VM user data") + @Param(description = "Base64 encoded VM user data") private String userData; @SerializedName(ApiConstants.USER_DATA_ID) @Param(description="the id of userdata used for the VM", since = "4.18.1") diff --git a/api/src/main/java/org/apache/cloudstack/api/response/VMUserDataResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/VMUserDataResponse.java index 1b739e56442..cf819491c2c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/VMUserDataResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/VMUserDataResponse.java @@ -30,7 +30,7 @@ public class VMUserDataResponse extends BaseResponse { private String vmId; @SerializedName(ApiConstants.USER_DATA) - @Param(description = "Base 64 encoded VM user data") + @Param(description = "Base64 encoded VM user data") private String userData; public void setUserData(String userData) { diff --git a/api/src/main/java/org/apache/cloudstack/userdata/UserDataManager.java b/api/src/main/java/org/apache/cloudstack/userdata/UserDataManager.java index 4dfcd0a7de1..b4bede24890 100644 --- a/api/src/main/java/org/apache/cloudstack/userdata/UserDataManager.java +++ b/api/src/main/java/org/apache/cloudstack/userdata/UserDataManager.java @@ -17,11 +17,16 @@ package org.apache.cloudstack.userdata; import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import com.cloud.utils.component.Manager; public interface UserDataManager extends Manager, Configurable { + String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length"; + ConfigKey VM_USERDATA_MAX_LENGTH = new ConfigKey<>("Advanced", Integer.class, VM_USERDATA_MAX_LENGTH_STRING, "32768", + "Max length of vm userdata after base64 encoding. Default is 32768 and maximum is 1048576", true); + String concatenateUserData(String userdata1, String userdata2, String userdataProvider); String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod); } diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index 728511ba8d5..3341f6e6bb0 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -21,7 +21,6 @@ import java.util.Map; import java.util.Set; import com.cloud.dc.VlanVO; -import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO; import com.cloud.dc.ClusterVO; @@ -61,9 +60,6 @@ public interface ConfigurationManager { public static final String MESSAGE_CREATE_VLAN_IP_RANGE_EVENT = "Message.CreateVlanIpRange.Event"; public static final String MESSAGE_DELETE_VLAN_IP_RANGE_EVENT = "Message.DeleteVlanIpRange.Event"; - static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length"; - static final ConfigKey VM_USERDATA_MAX_LENGTH = new ConfigKey<>("Advanced", Integer.class, VM_USERDATA_MAX_LENGTH_STRING, "32768", - "Max length of vm userdata after base64 decoding. Default is 32768 and maximum is 1048576", true); /** * @param offering diff --git a/engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java b/engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java index 91f24fe7045..81a77fa5c0b 100644 --- a/engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java +++ b/engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java @@ -26,19 +26,18 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; +import org.apache.log4j.Logger; -import com.cloud.configuration.ConfigurationManager; import com.cloud.exception.InvalidParameterValueException; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; public class UserDataManagerImpl extends ManagerBase implements UserDataManager { - - + private static final Logger s_logger = Logger.getLogger(UserDataManagerImpl.class); private static final int MAX_USER_DATA_LENGTH_BYTES = 2048; - private static final int MAX_HTTP_GET_LENGTH = 2 * MAX_USER_DATA_LENGTH_BYTES; + private static final int MAX_HTTP_GET_LENGTH = 2 * MAX_USER_DATA_LENGTH_BYTES; // 4KB private static final int NUM_OF_2K_BLOCKS = 512; - private static final int MAX_HTTP_POST_LENGTH = NUM_OF_2K_BLOCKS * MAX_USER_DATA_LENGTH_BYTES; + private static final int MAX_HTTP_POST_LENGTH = NUM_OF_2K_BLOCKS * MAX_USER_DATA_LENGTH_BYTES; // 1MB private List userDataProviders; private static Map userDataProvidersMap = new HashMap<>(); @@ -67,7 +66,7 @@ public class UserDataManagerImpl extends ManagerBase implements UserDataManager @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {}; + return new ConfigKey[] {VM_USERDATA_MAX_LENGTH}; } protected UserDataProvider getUserdataProvider(String name) { @@ -90,49 +89,57 @@ public class UserDataManagerImpl extends ManagerBase implements UserDataManager @Override public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { - byte[] decodedUserData = null; - if (userData != null) { - - if (userData.contains("%")) { - try { - userData = URLDecoder.decode(userData, "UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new InvalidParameterValueException("Url decoding of userdata failed."); - } - } - - if (!Base64.isBase64(userData)) { - throw new InvalidParameterValueException("User data is not base64 encoded"); - } - // If GET, use 4K. If POST, support up to 1M. - if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) { - decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_GET_LENGTH, BaseCmd.HTTPMethod.GET); - } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) { - decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_POST_LENGTH, BaseCmd.HTTPMethod.POST); - } - - if (decodedUserData == null || decodedUserData.length < 1) { - throw new InvalidParameterValueException("User data is too short"); - } - // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR. - return Base64.encodeBase64String(decodedUserData); + s_logger.trace(String.format("Validating base64 encoded user data: [%s].", userData)); + if (StringUtils.isBlank(userData)) { + s_logger.debug("Null/empty base64 encoded user data set"); + return null; } - return null; + + if (userData.contains("%")) { + try { + userData = URLDecoder.decode(userData, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new InvalidParameterValueException("Url decoding of user data failed."); + } + } + + if (!Base64.isBase64(userData)) { + throw new InvalidParameterValueException("User data is not base64 encoded."); + } + + byte[] decodedUserData = null; + + // If GET, use 4K. If POST, support up to 1M. + if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) { + decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_GET_LENGTH, BaseCmd.HTTPMethod.GET); + } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) { + decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_POST_LENGTH, BaseCmd.HTTPMethod.POST); + } + + // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR. + return Base64.encodeBase64String(decodedUserData); } private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, BaseCmd.HTTPMethod httpMethod) { - byte[] decodedUserData = null; + byte[] decodedUserData = Base64.decodeBase64(userData.getBytes()); + if (decodedUserData == null || decodedUserData.length < 1) { + throw new InvalidParameterValueException("User data is too short."); + } - if (userData.length() >= maxHTTPLength) { - throw new InvalidParameterValueException(String.format("User data is too long for an http %s request", httpMethod.toString())); - } - if (userData.length() > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) { - throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()); - } - decodedUserData = Base64.decodeBase64(userData.getBytes()); - if (decodedUserData.length > maxHTTPLength) { + s_logger.trace(String.format("Decoded user data: [%s].", decodedUserData)); + int userDataLength = userData.length(); + int decodedUserDataLength = decodedUserData.length; + s_logger.info(String.format("Configured base64 encoded user data size: %d bytes, actual user data size: %d bytes", userDataLength, decodedUserDataLength)); + + if (userDataLength > maxHTTPLength) { + s_logger.warn(String.format("Base64 encoded user data (size: %d bytes) too long for http %s request (accepted size: %d bytes)", userDataLength, httpMethod.toString(), maxHTTPLength)); throw new InvalidParameterValueException(String.format("User data is too long for http %s request", httpMethod.toString())); } + if (userDataLength > VM_USERDATA_MAX_LENGTH.value()) { + s_logger.warn(String.format("Base64 encoded user data (size: %d bytes) has exceeded configurable max length of %d bytes", userDataLength, VM_USERDATA_MAX_LENGTH.value())); + throw new InvalidParameterValueException("User data has exceeded configurable max length: " + VM_USERDATA_MAX_LENGTH.value()); + } + return decodedUserData; } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 170f092ecc3..f65dffb18cd 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -132,6 +132,7 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.userdata.UserDataManager; import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.collections.CollectionUtils; @@ -555,7 +556,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati configValuesForValidation.add(StorageManager.STORAGE_POOL_DISK_WAIT.key()); configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_TIMEOUT.key()); configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.key()); - configValuesForValidation.add(VM_USERDATA_MAX_LENGTH_STRING); + configValuesForValidation.add(UserDataManager.VM_USERDATA_MAX_LENGTH_STRING); } private void weightBasedParametersForValidation() { @@ -1284,7 +1285,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException("Please enter a value less than 257 for the configuration parameter:" + name); } } - if (VM_USERDATA_MAX_LENGTH_STRING.equalsIgnoreCase(name)) { + if (UserDataManager.VM_USERDATA_MAX_LENGTH_STRING.equalsIgnoreCase(name)) { if (val > 1048576) { throw new InvalidParameterValueException("Please enter a value less than 1048576 for the configuration parameter:" + name); } @@ -7792,8 +7793,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, - BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, VM_SERVICE_OFFERING_MAX_CPU_CORES, - VM_SERVICE_OFFERING_MAX_RAM_SIZE, VM_USERDATA_MAX_LENGTH, MIGRATE_VM_ACROSS_CLUSTERS, + BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, + VM_SERVICE_OFFERING_MAX_CPU_CORES, VM_SERVICE_OFFERING_MAX_RAM_SIZE, MIGRATE_VM_ACROSS_CLUSTERS, ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS }; } diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index 468f238a0c5..5bbd85c2415 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -70,6 +70,7 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.userdata.UserDataManager; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.RandomStringUtils; @@ -254,6 +255,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage @Inject private UserVmManager userVmMgr; @Inject + private UserDataManager userDataMgr; + @Inject private UserVmDao userVmDao; @Inject private HostDao hostDao; @@ -573,7 +576,7 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage userDataDetails = cmd.getUserDataDetails().toString(); } userData = userVmMgr.finalizeUserData(userData, userDataId, template); - userData = userVmMgr.validateUserData(userData, cmd.getHttpMethod()); + userData = userDataMgr.validateUserData(userData, cmd.getHttpMethod()); if (userData != null) { profileVO.setUserData(userData); } @@ -652,7 +655,7 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage } VirtualMachineTemplate template = entityMgr.findByIdIncludingRemoved(VirtualMachineTemplate.class, templateId); userData = userVmMgr.finalizeUserData(userData, userDataId, template); - userData = userVmMgr.validateUserData(userData, cmd.getHttpMethod()); + userData = userDataMgr.validateUserData(userData, cmd.getHttpMethod()); vmProfile.setUserDataId(userDataId); vmProfile.setUserData(userData); vmProfile.setUserDataDetails(userDataDetails); diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java index b107a520205..7cd1a98fd74 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManager.java +++ b/server/src/main/java/com/cloud/vm/UserVmManager.java @@ -95,8 +95,6 @@ public interface UserVmManager extends UserVmService { String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template); - String validateUserData(String userData, HTTPMethod httpmethod); - void validateExtraConfig(long accountId, HypervisorType hypervisorType, String extraConfig); boolean isVMUsingLocalStorage(VMInstanceVO vm); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 7fd074e13e8..f68b3a6237a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.vm; -import static com.cloud.configuration.ConfigurationManagerImpl.VM_USERDATA_MAX_LENGTH; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import static org.apache.cloudstack.api.ApiConstants.MAX_IOPS; import static org.apache.cloudstack.api.ApiConstants.MIN_IOPS; @@ -132,7 +131,6 @@ import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.cloudstack.vm.UnmanagedVMsManager; import org.apache.cloudstack.vm.schedule.VMScheduleManager; -import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.math.NumberUtils; @@ -2782,6 +2780,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir userDataDetails = cmd.getUserdataDetails().toString(); } userData = finalizeUserData(userData, userDataId, template); + userData = userDataManager.validateUserData(userData, cmd.getHttpMethod()); long accountId = vmInstance.getAccountId(); @@ -4861,56 +4860,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } - @Override - public String validateUserData(String userData, HTTPMethod httpmethod) { - byte[] decodedUserData = null; - if (userData != null) { - - if (userData.contains("%")) { - try { - userData = URLDecoder.decode(userData, "UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new InvalidParameterValueException("Url decoding of userdata failed."); - } - } - - if (!Base64.isBase64(userData)) { - throw new InvalidParameterValueException("User data is not base64 encoded"); - } - // If GET, use 4K. If POST, support up to 1M. - if (httpmethod.equals(HTTPMethod.GET)) { - if (userData.length() >= MAX_HTTP_GET_LENGTH) { - throw new InvalidParameterValueException("User data is too long for an http GET request"); - } - if (userData.length() > VM_USERDATA_MAX_LENGTH.value()) { - throw new InvalidParameterValueException("User data has exceeded configurable max length : " + VM_USERDATA_MAX_LENGTH.value()); - } - decodedUserData = Base64.decodeBase64(userData.getBytes()); - if (decodedUserData.length > MAX_HTTP_GET_LENGTH) { - throw new InvalidParameterValueException("User data is too long for GET request"); - } - } else if (httpmethod.equals(HTTPMethod.POST)) { - if (userData.length() >= MAX_HTTP_POST_LENGTH) { - throw new InvalidParameterValueException("User data is too long for an http POST request"); - } - if (userData.length() > VM_USERDATA_MAX_LENGTH.value()) { - throw new InvalidParameterValueException("User data has exceeded configurable max length : " + VM_USERDATA_MAX_LENGTH.value()); - } - decodedUserData = Base64.decodeBase64(userData.getBytes()); - if (decodedUserData.length > MAX_HTTP_POST_LENGTH) { - throw new InvalidParameterValueException("User data is too long for POST request"); - } - } - - if (decodedUserData == null || decodedUserData.length < 1) { - throw new InvalidParameterValueException("User data is too short"); - } - // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR. - return Base64.encodeBase64String(decodedUserData); - } - return null; - } - @Override @ActionEvent(eventType = EventTypes.EVENT_VM_CREATE, eventDescription = "deploying Vm", async = true) public UserVm startVirtualMachine(DeployVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException, ConcurrentOperationException, ResourceAllocationException { @@ -6001,13 +5950,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } String userData = cmd.getUserData(); - userData = userDataManager.validateUserData(userData, cmd.getHttpMethod()); Long userDataId = cmd.getUserdataId(); String userDataDetails = null; if (MapUtils.isNotEmpty(cmd.getUserdataDetails())) { userDataDetails = cmd.getUserdataDetails().toString(); } userData = finalizeUserData(userData, userDataId, template); + userData = userDataManager.validateUserData(userData, cmd.getHttpMethod()); Account caller = CallContext.current().getCallingAccount(); Long callerId = caller.getId(); diff --git a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java index aaf0f254d41..60277740daa 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -122,6 +122,7 @@ import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.userdata.UserDataManager; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -189,6 +190,9 @@ public class AutoScaleManagerImplTest { @Mock UserVmManager userVmMgr; + @Mock + UserDataManager userDataMgr; + @Mock EntityManager entityManager; @@ -406,7 +410,7 @@ public class AutoScaleManagerImplTest { userDataDetails.put("0", new HashMap<>() {{ put("key1", "value1"); put("key2", "value2"); }}); Mockito.doReturn(userDataFinal).when(userVmMgr).finalizeUserData(any(), any(), any()); - Mockito.doReturn(userDataFinal).when(userVmMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); + Mockito.doReturn(userDataFinal).when(userDataMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); } @After @@ -760,7 +764,7 @@ public class AutoScaleManagerImplTest { Mockito.verify(autoScaleVmProfileDao).persist(Mockito.any()); Mockito.verify(userVmMgr).finalizeUserData(any(), any(), any()); - Mockito.verify(userVmMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); + Mockito.verify(userDataMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); } } @@ -821,7 +825,7 @@ public class AutoScaleManagerImplTest { Mockito.verify(autoScaleVmProfileDao).persist(Mockito.any()); Mockito.verify(userVmMgr).finalizeUserData(any(), any(), any()); - Mockito.verify(userVmMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); + Mockito.verify(userDataMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); } @Test