Merge branch '4.22' into propagate-stopanswer-errors

Author: DaanHoogland

.github/linters/codespell.txt

@@ -187,6 +187,7 @@ environmnet
 equivalant
 erro
 erronous
+errorprone
 everthing
 everytime
 excute

agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java

@@ -249,7 +249,7 @@ public void freeVncPort(int port) {
     public MockVm createVmFromSpec(VirtualMachineTO vmSpec) {
         String vmName = vmSpec.getName();
         long ramSize = vmSpec.getMinRam();
-        int utilizationPercent = randSeed.nextInt() % 100;
+        int utilizationPercent = randSeed.nextInt(100);
         MockVm vm = null;
 
         synchronized (this) {

api/src/main/java/org/apache/cloudstack/acl/APIChecker.java

@@ -17,16 +17,22 @@
 package org.apache.cloudstack.acl;
 
 import com.cloud.exception.PermissionDeniedException;
+import com.cloud.exception.RequestLimitException;
 import com.cloud.user.Account;
 import com.cloud.user.User;
 import com.cloud.utils.component.Adapter;
 
+import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 /**
  * APICheckers is designed to verify the ownership of resources and to control the access to APIs.
  */
 public interface APIChecker extends Adapter {
+    Logger LOGGER = LogManager.getLogger(APIChecker.class);
     // Interface for checking access for a role using apiname
     // If true, apiChecker has checked the operation
     // If false, apiChecker is unable to handle the operation or not implemented
@@ -42,5 +48,27 @@ public interface APIChecker extends Adapter {
      * @return the list of allowed apis for the given user
      */
     List<String> getApisAllowedToUser(Role role, User user, List<String> apiNames) throws PermissionDeniedException;
+
+    default List<String> getApisAllowedToAccount(Account account, List<String> apiNames) {
+        List<String> allowedApis = new ArrayList<>();
+        for (String apiName : apiNames) {
+            try {
+                checkAccess(account, apiName);
+                allowedApis.add(apiName);
+            } catch (RequestLimitException e) {
+                // Non-ACL failure (e.g. rate limiting) should not be treated as simple "not allowed".
+                // Propagate as unchecked so callers are aware of the failure.
+                throw new RuntimeException("Failed to check access for API [" + apiName + "] due to request limits", e);
+            } catch (PermissionDeniedException e) {
+                LOGGER.trace("Account [" + account + "] is not allowed to access API [" + apiName + "]");
+            }
+        }
+        return allowedApis;
+    }
+
     boolean isEnabled();
+
+    default void refreshRoleCacheOnPermissionsChange(Role role) {
+        // Only applicable for dynamic role based checkers
+    }
 }

api/src/main/java/org/apache/cloudstack/api/ApiArgValidator.java

@@ -17,9 +17,18 @@
 
 package org.apache.cloudstack.api;
 
+import java.util.Locale;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.UuidUtils;
+
 public enum ApiArgValidator {
     /**
-     * Validates if the parameter is null or empty with the method {@link Strings#isNullOrEmpty(String)}.
+     * Validates if the parameter is null or empty with the method {@link StringUtils#isEmpty(CharSequence)}.
+     * Validation is currently done in the method ParamProcessWorker#validateNonEmptyString(String, String).
      */
     NotNullOrEmpty,
 
@@ -29,12 +38,72 @@ public enum ApiArgValidator {
     PositiveNumber,
 
     /**
-     * Validates if the parameter is an UUID with the method {@link UuidUtils#isUuid(String)}.
+     * Validates if the parameter is a UUID with the method {@link UuidUtils#isUuid(String)}.
+     * Validation is currently done in the method ParamProcessWorker#validateUuidString(String, String).
      */
     UuidString,
 
     /**
      * Validates if the parameter is a valid RFC Compliance domain name.
      */
     RFCComplianceDomainName,
+
+    /**
+     * Validates command option strings to avoid unsafe/code-like content.
+     */
+    SafeCommandOptions((param, annotation) -> {
+        if (BaseCmd.CommandType.STRING.equals(annotation.type())) {
+            validateSafeCommandOptions(param, annotation.name());
+        }
+    });
+
+    private static final Pattern SAFE_COMMAND_OPTIONS_PATTERN = Pattern.compile("^[A-Za-z0-9,._=:/+\\-\\s]*$");
+
+    private static final String[] UNSAFE_TOKENS = {
+            "$(", "`", "&&", "||", ";", "|", ">", "<"
+    };
+
+    private final ValidationRule rule;
+
+    ApiArgValidator() {
+        this(null);
+    }
+
+    ApiArgValidator(ValidationRule rule) {
+        this.rule = rule;
+    }
+
+    public void validate(final Object paramObj, final Parameter annotation) {
+        if (rule != null) {
+            rule.validate(paramObj, annotation);
+        }
+    }
+
+    private static void validateSafeCommandOptions(final Object param, final String argName) {
+        final String value = String.valueOf(param);
+        if (StringUtils.isBlank(value)) {
+            return;
+        }
+
+        if (!SAFE_COMMAND_OPTIONS_PATTERN.matcher(value).matches()) {
+            throwInvalidParameterValueException(argName, "contains unsupported or unsafe characters");
+        }
+
+        final String normalized = value.toLowerCase(Locale.ROOT);
+        for (String token : UNSAFE_TOKENS) {
+            if (normalized.contains(token)) {
+                throwInvalidParameterValueException(argName, "contains code-like or unsafe content");
+            }
+        }
+    }
+
+    private static void throwInvalidParameterValueException(final String argName, final String customMsg) {
+        throw new InvalidParameterValueException(String.format("Invalid value provided for API arg: %s%s", argName,
+                StringUtils.isBlank(customMsg) ? "" : " - " + customMsg));
+    }
+
+    @FunctionalInterface
+    interface ValidationRule {
+        void validate(Object paramObj, Parameter annotation);
+    }
 }

api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java

@@ -83,8 +83,8 @@ public Boolean getAllowUserDrivenBackups() {
     public void execute() {
         try {
             if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null) {
-                throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the",
-                        "following should be informed: name, description or allowUserDrivenBackups.", id));
+                throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated," +
+                        " at least one of the following should be passed: name, description or allowUserDrivenBackups.", id));
             }
 
             BackupOffering result = backupManager.updateBackupOffering(this);

api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ResetVMPasswordCmdByAdmin.java

@@ -27,5 +27,5 @@
 @APICommand(name = "resetPasswordForVirtualMachine", responseObject=UserVmResponse.class, description = "Resets the password for Instance. " +
                     "The Instance must be in a \"Stopped\" state and the Template must already " +
         "support this feature for this command to take effect. [async]", responseView = ResponseView.Full, entityType = {VirtualMachine.class},
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = true)
+        requestHasSensitiveInfo = true, responseHasSensitiveInfo = true)
 public class ResetVMPasswordCmdByAdmin extends ResetVMPasswordCmd implements AdminCmd {}

api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java

@@ -22,6 +22,7 @@
 import org.apache.commons.collections.MapUtils;
 
 import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.ACL;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -106,6 +107,7 @@ public class CreateAutoScaleVmProfileCmd extends BaseAsyncCreateCmd {
             since = "4.18.0")
     private String userData;
 
+    @ACL
     @Parameter(name = ApiConstants.USER_DATA_ID, type = CommandType.UUID, entityType = UserDataResponse.class, description = "the ID of the Userdata", since = "4.18.1")
     private Long userDataId;
 

api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/UpdateAutoScaleVmProfileCmd.java

@@ -101,6 +101,7 @@ public class UpdateAutoScaleVmProfileCmd extends BaseAsyncCustomIdCmd {
             since = "4.18.0")
     private String userData;
 
+    @ACL
     @Parameter(name = ApiConstants.USER_DATA_ID, type = CommandType.UUID, entityType = UserDataResponse.class, description = "the ID of the userdata",
             since = "4.18.1")
     private Long userDataId;

api/src/main/java/org/apache/cloudstack/api/command/user/backup/repository/AddBackupRepositoryCmd.java

@@ -20,6 +20,7 @@
 import com.cloud.utils.StringUtils;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseCmd;
@@ -57,7 +58,8 @@ public class AddBackupRepositoryCmd extends BaseCmd {
     @Parameter(name = ApiConstants.PROVIDER, type = CommandType.STRING, description = "backup repository provider")
     private String provider;
 
-    @Parameter(name = ApiConstants.MOUNT_OPTIONS, type = CommandType.STRING, description = "shared storage mount options")
+    @Parameter(name = ApiConstants.MOUNT_OPTIONS, type = CommandType.STRING, description = "shared storage mount options",
+            validations = {ApiArgValidator.SafeCommandOptions})
     private String mountOptions;
 
     @Parameter(name = ApiConstants.ZONE_ID,

api/src/main/java/org/apache/cloudstack/api/command/user/backup/repository/UpdateBackupRepositoryCmd.java

@@ -17,8 +17,11 @@
 
 package org.apache.cloudstack.api.command.user.backup.repository;
 
+import javax.inject.Inject;
+
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseCmd;
@@ -29,8 +32,6 @@
 import org.apache.cloudstack.backup.BackupRepositoryService;
 import org.apache.cloudstack.context.CallContext;
 
-import javax.inject.Inject;
-
 @APICommand(name = "updateBackupRepository",
         description = "Update a backup repository",
         responseObject = BackupRepositoryResponse.class, since = "4.22.0",
@@ -53,7 +54,8 @@ public class UpdateBackupRepositoryCmd extends BaseCmd {
     @Parameter(name = ApiConstants.ADDRESS, type = CommandType.STRING, description = "address of the backup repository")
     private String address;
 
-    @Parameter(name = ApiConstants.MOUNT_OPTIONS, type = CommandType.STRING, description = "shared storage mount options")
+    @Parameter(name = ApiConstants.MOUNT_OPTIONS, type = CommandType.STRING, description = "shared storage mount options",
+            validations = {ApiArgValidator.SafeCommandOptions})
     private String mountOptions;
 
     @Parameter(name = ApiConstants.CROSS_ZONE_INSTANCE_CREATION, type = CommandType.BOOLEAN, description = "backups in this repository can be used to create Instances on all Zones")