User data content size validation, and related code improvements

engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java

@@ -26,15 +26,15 @@
 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 NUM_OF_2K_BLOCKS = 512;
@@ -90,49 +90,56 @@ 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.");
-                }
-            }
+        s_logger.trace(String.format("Validating user data: [%s].", userData));
+        if (StringUtils.isBlank(userData)) {
+            s_logger.debug("Null/empty user data set");
+            return null;
+        }
 
-            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 user data 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) {
+
+        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));
+
+        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());
+        }
+
         return decodedUserData;
     }
 }

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

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

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;
@@ -2787,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();
 
@@ -4860,56 +4859,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 {
@@ -5995,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();

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

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)