CLOUDSTACK-9593: userdata: enforce data is a multiple of 4 characters (#1760)

Python base64 requires that the string is a multiple of 4 characters but
the Apache codec does not. RFC states is not mandatory so the data should
not fail the VR script (vmdata.py).

Signed-off-by: Marc-Aurèle Brothier <m@brothier.org>
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Author: marcaurele

engine/schema/src/com/cloud/upgrade/dao/Upgrade41000to41100.java

@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.codec.binary.Base64;
 import org.apache.log4j.Logger;
 
 import com.cloud.hypervisor.Hypervisor;
@@ -64,9 +65,49 @@ public InputStream[] getPrepareScripts() {
 
     @Override
     public void performDataMigration(Connection conn) {
+        validateUserDataInBase64(conn);
         updateSystemVmTemplates(conn);
     }
 
+    private void validateUserDataInBase64(Connection conn) {
+        try (final PreparedStatement selectStatement = conn.prepareStatement("SELECT `id`, `user_data` FROM `cloud`.`user_vm` WHERE `user_data` IS NOT NULL;");
+             final ResultSet selectResultSet = selectStatement.executeQuery()) {
+            while (selectResultSet.next()) {
+                final Long userVmId = selectResultSet.getLong(1);
+                final String userData = selectResultSet.getString(2);
+                if (Base64.isBase64(userData)) {
+                    final String newUserData = Base64.encodeBase64String(Base64.decodeBase64(userData.getBytes()));
+                    if (!userData.equals(newUserData)) {
+                        try (final PreparedStatement updateStatement = conn.prepareStatement("UPDATE `cloud`.`user_vm` SET `user_data` = ? WHERE `id` = ? ;")) {
+                            updateStatement.setString(1, newUserData);
+                            updateStatement.setLong(2, userVmId);
+                            updateStatement.executeUpdate();
+                        } catch (SQLException e) {
+                            LOG.error("Failed to update cloud.user_vm user_data for id:" + userVmId + " with exception: " + e.getMessage());
+                            throw new CloudRuntimeException("Exception while updating cloud.user_vm for id " + userVmId, e);
+                        }
+                    }
+                } else {
+                    // Update to NULL since it's invalid
+                    LOG.warn("Removing user_data for vm id " + userVmId + " because it's invalid");
+                    LOG.warn("Removed data was: " + userData);
+                    try (final PreparedStatement updateStatement = conn.prepareStatement("UPDATE `cloud`.`user_vm` SET `user_data` = NULL WHERE `id` = ? ;")) {
+                        updateStatement.setLong(1, userVmId);
+                        updateStatement.executeUpdate();
+                    } catch (SQLException e) {
+                        LOG.error("Failed to update cloud.user_vm user_data for id:" + userVmId + " to NULL with exception: " + e.getMessage());
+                        throw new CloudRuntimeException("Exception while updating cloud.user_vm for id " + userVmId + " to NULL", e);
+                    }
+                }
+            }
+        } catch (SQLException e) {
+            throw new CloudRuntimeException("Exception while validating existing user_vm table's user_data column to be base64 valid with padding", e);
+        }
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Done validating base64 content of user data");
+        }
+    }
+
     @SuppressWarnings("serial")
     private void updateSystemVmTemplates(final Connection conn) {
         LOG.debug("Updating System Vm template IDs");

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -2525,7 +2525,7 @@ public UserVm updateVirtualMachine(long id, String displayName, String group, Bo
         if (userData != null) {
             // check and replace newlines
             userData = userData.replace("\\n", "");
-            validateUserData(userData, httpMethod);
+            userData = validateUserData(userData, httpMethod);
             // update userData on domain router.
             updateUserdata = true;
         } else {
@@ -3396,7 +3396,7 @@ protected UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOf
         _accountMgr.checkAccess(owner, AccessType.UseEntry, false, template);
 
         // check if the user data is correct
-        validateUserData(userData, httpmethod);
+        userData = validateUserData(userData, httpmethod);
 
         // Find an SSH public key corresponding to the key pair name, if one is
         // given
@@ -3943,7 +3943,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
         }
     }
 
-    private void validateUserData(String userData, HTTPMethod httpmethod) {
+    protected String validateUserData(String userData, HTTPMethod httpmethod) {
         byte[] decodedUserData = null;
         if (userData != null) {
             if (!Base64.isBase64(userData)) {
@@ -3971,7 +3971,10 @@ private void validateUserData(String userData, HTTPMethod httpmethod) {
             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

server/test/com/cloud/vm/UserVmManagerTest.java

@@ -48,6 +48,7 @@
 import com.cloud.event.dao.UsageEventDao;
 import com.cloud.uservm.UserVm;
 import org.junit.Assert;
+import org.apache.cloudstack.api.BaseCmd;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -1056,4 +1057,16 @@ public void testPersistDeviceBusInfo() {
         _userVmMgr.persistDeviceBusInfo(_vmMock, "lsilogic");
         verify(_vmDao, times(1)).saveDetails(any(UserVmVO.class));
     }
+
+    @Test
+    public void testValideBase64WithoutPadding() {
+        // fo should be encoded in base64 either as Zm8 or Zm8=
+        String encodedUserdata = "Zm8";
+        String encodedUserdataWithPadding = "Zm8=";
+
+        // Verify that we accept both but return the padded version
+        assertTrue("validate return the value with padding", encodedUserdataWithPadding.equals(_userVmMgr.validateUserData(encodedUserdata, BaseCmd.HTTPMethod.GET)));
+        assertTrue("validate return the value with padding", encodedUserdataWithPadding.equals(_userVmMgr.validateUserData(encodedUserdataWithPadding, BaseCmd.HTTPMethod.GET)));
+    }
+
 }