CLOUDSTACK-9705: Unauthenticated API allows Admin password reset

anshul-gangwar · Rohit Yadav · commit 75c81d918a35 · 2017-08-01T20:40:25.000+02:00
Now, Updating the password via UpdateUser API is not allowed via integration port (cherry picked from commit d206336) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
diff --git a/api/src/org/apache/cloudstack/api/Parameter.java b/api/src/org/apache/cloudstack/api/Parameter.java
@@ -51,4 +51,6 @@
     RoleType[] authorized() default {};
 
     ApiArgValidator[] validations() default {};
+
+    boolean acceptedOnAdminPort() default true;
 }
diff --git a/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java b/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java
@@ -18,8 +18,6 @@
 
 import javax.inject.Inject;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
@@ -29,6 +27,7 @@
 import org.apache.cloudstack.api.response.UserResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.region.RegionService;
+import org.apache.log4j.Logger;
 
 import com.cloud.user.Account;
 import com.cloud.user.User;
@@ -61,10 +60,12 @@ public class UpdateUserCmd extends BaseCmd {
     private String lastname;
 
     @Parameter(name = ApiConstants.PASSWORD,
-               type = CommandType.STRING,
-               description = "Clear text password (default hashed to SHA256SALT). If you wish to use any other hasing algorithm, you would need to write a custom authentication adapter")
+            type = CommandType.STRING,
+            description = "Clear text password (default hashed to SHA256SALT). If you wish to use any other hasing algorithm, you would need to write a custom authentication adapter. Can't be passed when command is executed via integration.api.port",
+            acceptedOnAdminPort = false)
     private String password;
 
+
     @Parameter(name = ApiConstants.SECRET_KEY, type = CommandType.STRING, description = "The secret key for the user. Must be specified with userSecretKey")
     private String secretKey;
 
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
@@ -44,6 +44,7 @@
 import com.cloud.utils.HttpUtils;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
+import com.cloud.utils.ReflectUtil;
 import com.cloud.utils.StringUtils;
 import com.cloud.utils.component.ComponentContext;
 import com.cloud.utils.component.ManagerBase;
@@ -65,6 +66,7 @@
 import org.apache.cloudstack.api.BaseAsyncCreateCmd;
 import org.apache.cloudstack.api.BaseCmd;
 import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ResponseObject;
 import org.apache.cloudstack.api.ResponseObject.ResponseView;
 import org.apache.cloudstack.api.ServerApiException;
@@ -150,6 +152,7 @@
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.lang.reflect.Type;
+import java.lang.reflect.Field;
 import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
@@ -430,8 +433,27 @@ public void handle(final HttpRequest request, final HttpResponse response, final
             if (!(responseType.equals(HttpUtils.RESPONSE_TYPE_JSON) || responseType.equals(HttpUtils.RESPONSE_TYPE_XML))) {
                 responseType = HttpUtils.RESPONSE_TYPE_XML;
             }
-
             try {
+                //verify that parameter is legit for passing via admin port
+                String[] command = (String[]) parameterMap.get("command");
+                if (command != null) {
+                    Class<?> cmdClass = getCmdClass(command[0]);
+                    if (cmdClass != null) {
+                        List<Field> fields = ReflectUtil.getAllFieldsForClass(cmdClass, BaseCmd.class);
+                        for (Field field : fields) {
+                            Parameter parameterAnnotation = field.getAnnotation(Parameter.class);
+                            if ((parameterAnnotation == null) || !parameterAnnotation.expose()) {
+                                continue;
+                            }
+                            Object paramObj = parameterMap.get(parameterAnnotation.name());
+                            if (paramObj != null) {
+                                if (!parameterAnnotation.acceptedOnAdminPort()) {
+                                    throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, "Parameter " + parameterAnnotation.name() + " can't be passed through the API integration port");
+                                }
+                            }
+                        }
+                    }
+                }
                 // always trust commands from API port, user context will always be UID_SYSTEM/ACCOUNT_ID_SYSTEM
                 CallContext.register(accountMgr.getSystemUser(), accountMgr.getSystemAccount());
                 sb.insert(0, "(userId=" + User.UID_SYSTEM + " accountId=" + Account.ACCOUNT_ID_SYSTEM + " sessionId=" + null + ") ");

Original file line number	Diff line number	Diff line change
`@@ -51,4 +51,6 @@`
`51`	`51`	`RoleType[] authorized() default {};`
`52`	`52`
`53`	`53`	`ApiArgValidator[] validations() default {};`
	`54`	`+`
	`55`	`+ boolean acceptedOnAdminPort() default true;`
`54`	`56`	`}`