From 930cda56d48654d0cc484fd8e628962a0d6775f8 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 29 Dec 2023 13:46:16 +0530 Subject: [PATCH 1/9] Validate user data with actual length, and some code improvements --- .../userdata/UserDataManagerImpl.java | 65 ++++++++++--------- .../network/as/AutoScaleManagerImpl.java | 7 +- .../main/java/com/cloud/vm/UserVmManager.java | 2 - .../java/com/cloud/vm/UserVmManagerImpl.java | 52 --------------- .../network/as/AutoScaleManagerImplTest.java | 10 ++- 5 files changed, 45 insertions(+), 91 deletions(-) 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 91f24fe70458..5edc6748d1e2 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 @@ -90,49 +90,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use @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 (StringUtils.isBlank(userData)) { + throw new InvalidParameterValueException("User data is empty"); + } - 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 (userData.contains("%")) { + try { + userData = URLDecoder.decode(userData, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new InvalidParameterValueException("Url decoding of userdata failed."); } + } - 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); + if (!Base64.isBase64(userData)) { + throw new InvalidParameterValueException("User data is not base64 encoded"); } - return null; - } - private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, BaseCmd.HTTPMethod httpMethod) { byte[] decodedUserData = null; - if (userData.length() >= maxHTTPLength) { - throw new InvalidParameterValueException(String.format("User data is too long for an http %s request", httpMethod.toString())); + // 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 (userData.length() > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) { - throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()); + + // 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 = Base64.decodeBase64(userData.getBytes()); + if (decodedUserData == null || decodedUserData.length < 1) { + throw new InvalidParameterValueException("User data is too short"); } - decodedUserData = Base64.decodeBase64(userData.getBytes()); - if (decodedUserData.length > maxHTTPLength) { + + int userDataLength = decodedUserData.length; + + if (userDataLength > maxHTTPLength) { throw new InvalidParameterValueException(String.format("User data is too long for http %s request", httpMethod.toString())); } + if (userDataLength > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) { + throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()); + } + return decodedUserData; } } 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 468f238a0c54..5bbd85c2415d 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.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 AutoScaleVmProfile createAutoScaleVmProfile(CreateAutoScaleVmProfileCmd c 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 AutoScaleVmProfile updateAutoScaleVmProfile(UpdateAutoScaleVmProfileCmd c } 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 b107a520205b..7cd1a98fd743 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 d94847619982..83851a503dcb 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.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; @@ -4860,56 +4858,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } } - @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 { 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 aaf0f254d418..60277740daa2 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.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 void setUp() { 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 void testCreateAutoScaleVmProfile() { 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 void testUpdateAutoScaleVmProfile() { 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 From 5f9c7ac0a96df5b4d062caaedcaccae075d3aa49 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 4 Jan 2024 15:16:25 +0530 Subject: [PATCH 2/9] Ignore if user data is not set (don't fail) --- .../org/apache/cloudstack/userdata/UserDataManagerImpl.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 5edc6748d1e2..1297ea756878 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 @@ -33,8 +33,6 @@ import com.cloud.utils.exception.CloudRuntimeException; public class UserDataManagerImpl extends ManagerBase implements UserDataManager { - - 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 NUM_OF_2K_BLOCKS = 512; @@ -91,7 +89,7 @@ public String concatenateUserData(String userdata1, String userdata2, String use @Override public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { if (StringUtils.isBlank(userData)) { - throw new InvalidParameterValueException("User data is empty"); + return null; } if (userData.contains("%")) { From 8171710b7e8392c2a13d2fc3055fcb4c0de96d5a Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 4 Jan 2024 15:38:16 +0530 Subject: [PATCH 3/9] Validate user data after finalizing it --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 83851a503dcb..8aa55b6858a3 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2785,6 +2785,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx userDataDetails = cmd.getUserdataDetails().toString(); } userData = finalizeUserData(userData, userDataId, template); + userData = userDataManager.validateUserData(userData, cmd.getHttpMethod()); long accountId = vmInstance.getAccountId(); @@ -5943,13 +5944,13 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE } 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(); From dcc5410b8da3986a1c09a2a7371c51b35c50ffcd Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 10 Jan 2024 11:01:11 +0530 Subject: [PATCH 4/9] Removed registerUserData POST call changes here (separate PR here: https://github.com/apache/cloudstack/pull/8487) --- ui/src/views/compute/RegisterUserData.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/views/compute/RegisterUserData.vue b/ui/src/views/compute/RegisterUserData.vue index 990e59ff2772..a1322a128329 100644 --- a/ui/src/views/compute/RegisterUserData.vue +++ b/ui/src/views/compute/RegisterUserData.vue @@ -211,7 +211,7 @@ export default { params.params = userdataparams } - api('registerUserData', {}, 'POST', params).then(json => { + api('registerUserData', params).then(json => { this.$message.success(this.$t('message.success.register.user.data') + ' ' + values.name) }).catch(error => { this.$notifyError(error) From f05adf10a6f46104bbce19897d0e6ff859f34312 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 10 Jan 2024 12:10:40 +0530 Subject: [PATCH 5/9] Apply suggestions from code review 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> --- .../org/apache/cloudstack/userdata/UserDataManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 1297ea756878..bd4676dd8c20 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 @@ -101,7 +101,7 @@ public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { } if (!Base64.isBase64(userData)) { - throw new InvalidParameterValueException("User data is not base64 encoded"); + throw new InvalidParameterValueException("User data is not base64 encoded."); } byte[] decodedUserData = null; @@ -120,7 +120,7 @@ public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, BaseCmd.HTTPMethod httpMethod) { byte[] decodedUserData = Base64.decodeBase64(userData.getBytes()); if (decodedUserData == null || decodedUserData.length < 1) { - throw new InvalidParameterValueException("User data is too short"); + throw new InvalidParameterValueException("User data is too short."); } int userDataLength = decodedUserData.length; From 12ac93612db772527d079eb2461f33cbe66eb4d2 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 11 Jun 2024 17:34:07 +0530 Subject: [PATCH 6/9] Added logs for user data --- .../apache/cloudstack/userdata/UserDataManagerImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 bd4676dd8c20..47254d6eb3ba 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,6 +26,7 @@ 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; @@ -33,6 +34,7 @@ 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 NUM_OF_2K_BLOCKS = 512; @@ -89,6 +91,7 @@ public String concatenateUserData(String userdata1, String userdata2, String use @Override public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { if (StringUtils.isBlank(userData)) { + s_logger.debug("Null/empty user data set"); return null; } @@ -96,7 +99,7 @@ public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { try { userData = URLDecoder.decode(userData, "UTF-8"); } catch (UnsupportedEncodingException e) { - throw new InvalidParameterValueException("Url decoding of userdata failed."); + throw new InvalidParameterValueException("Url decoding of user data failed."); } } @@ -124,11 +127,14 @@ private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, } int userDataLength = decodedUserData.length; + s_logger.info(String.format("Configured user data size: %d bytes", userDataLength)); if (userDataLength > maxHTTPLength) { + s_logger.warn(String.format("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 > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) { + s_logger.warn(String.format("User data (size: %d bytes) has exceeded configurable max length of %d bytes", userDataLength, ConfigurationManager.VM_USERDATA_MAX_LENGTH.value())); throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()); } From 7c3780e4616601a613a4f48501ed0791c16ed0fd Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 13 Jun 2024 12:44:27 +0530 Subject: [PATCH 7/9] Addressed review comments --- .../org/apache/cloudstack/userdata/UserDataManagerImpl.java | 2 ++ 1 file changed, 2 insertions(+) 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 47254d6eb3ba..19d9b42afdb5 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 @@ -90,6 +90,7 @@ public String concatenateUserData(String userdata1, String userdata2, String use @Override public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { + s_logger.trace(String.format("Validating user data: [%s].", userData)); if (StringUtils.isBlank(userData)) { s_logger.debug("Null/empty user data set"); return null; @@ -126,6 +127,7 @@ private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, throw new InvalidParameterValueException("User data is too short."); } + s_logger.trace(String.format("Decoded user data: [%s].", decodedUserData)); int userDataLength = decodedUserData.length; s_logger.info(String.format("Configured user data size: %d bytes", userDataLength)); From 92006d09de25eab99ab7f08f2b7f4f5363fb54eb Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 13 Jun 2024 18:05:39 +0530 Subject: [PATCH 8/9] Use POST call to register user data --- ui/src/views/compute/RegisterUserData.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/views/compute/RegisterUserData.vue b/ui/src/views/compute/RegisterUserData.vue index a1322a128329..990e59ff2772 100644 --- a/ui/src/views/compute/RegisterUserData.vue +++ b/ui/src/views/compute/RegisterUserData.vue @@ -211,7 +211,7 @@ export default { params.params = userdataparams } - api('registerUserData', params).then(json => { + api('registerUserData', {}, 'POST', params).then(json => { this.$message.success(this.$t('message.success.register.user.data') + ' ' + values.name) }).catch(error => { this.$notifyError(error) From 49349e9f1b540078a9587e8b40a634c00328ce1e Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 14 Jun 2024 15:43:26 +0530 Subject: [PATCH 9/9] Check user data length with base64 encoded data, and some code improvements --- .../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 | 24 +++++++++---------- .../ConfigurationManagerImpl.java | 9 +++---- 12 files changed, 41 insertions(+), 28 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 db6ccd9ce53c..8c1d38c1375e 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 3e65d38e5201..34c81bfc4839 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 f294f7dd8e09..3d44230cac12 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 1cbe28f4ddee..e02111af59f7 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 3ead67e21064..7e0aab98760f 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 32ce1f6db524..4527e152f185 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 9f2383447301..678c4fcaaca4 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 1b739e564421..cf819491c2c0 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 4dfcd0a7de1b..b4bede248907 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 728511ba8d58..3341f6e6bb0c 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.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 19d9b42afdb5..81a77fa5c0b7 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 @@ -28,7 +28,6 @@ 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; @@ -36,9 +35,9 @@ 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 String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {}; + return new ConfigKey[] {VM_USERDATA_MAX_LENGTH}; } protected UserDataProvider getUserdataProvider(String name) { @@ -90,9 +89,9 @@ public String concatenateUserData(String userdata1, String userdata2, String use @Override public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { - s_logger.trace(String.format("Validating user data: [%s].", userData)); + s_logger.trace(String.format("Validating base64 encoded user data: [%s].", userData)); if (StringUtils.isBlank(userData)) { - s_logger.debug("Null/empty user data set"); + s_logger.debug("Null/empty base64 encoded user data set"); return null; } @@ -128,16 +127,17 @@ private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, } s_logger.trace(String.format("Decoded user data: [%s].", decodedUserData)); - int userDataLength = decodedUserData.length; - s_logger.info(String.format("Configured user data size: %d bytes", userDataLength)); + 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("User data (size: %d bytes) too long for http %s request (accepted size: %d bytes)", userDataLength, httpMethod.toString(), 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 > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) { - s_logger.warn(String.format("User data (size: %d bytes) has exceeded configurable max length of %d bytes", userDataLength, ConfigurationManager.VM_USERDATA_MAX_LENGTH.value())); - throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()); + 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 b59c8d018ee6..f990a9e060e3 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.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 @@ private void populateConfigValuesForValidationSet() { 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 @@ private String validateConfigurationValue(final String name, String value, final 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); } @@ -7786,8 +7787,8 @@ public String getConfigComponentName() { @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 }; }