diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/CreateVMFromBackupCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/CreateVMFromBackupCmdByAdmin.java index d95f17ef304c..f395335bc3cd 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/CreateVMFromBackupCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/CreateVMFromBackupCmdByAdmin.java @@ -34,7 +34,7 @@ responseObject = UserVmResponse.class, responseView = ResponseObject.ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, + requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.21.0", authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class CreateVMFromBackupCmdByAdmin extends CreateVMFromBackupCmd implements AdminCmd { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVMCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVMCmdByAdmin.java index e64c8b3f46c6..9f2abd076159 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVMCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVMCmdByAdmin.java @@ -31,7 +31,7 @@ @APICommand(name = "deployVirtualMachine", description = "Creates and automatically starts an Instance based on a service offering, disk offering, and Template.", responseObject = UserVmResponse.class, responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = true, responseHasSensitiveInfo = true) public class DeployVMCmdByAdmin extends DeployVMCmd implements AdminCmd { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVnfApplianceCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVnfApplianceCmdByAdmin.java index 5d2756c82fc6..ded569462f6c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVnfApplianceCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVnfApplianceCmdByAdmin.java @@ -28,7 +28,7 @@ responseObject = UserVmResponse.class, responseView = ResponseObject.ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, + requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.19.0") public class DeployVnfApplianceCmdByAdmin extends DeployVnfApplianceCmd implements AdminCmd { } 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 720df312fe7d..73823c5a2e37 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 @@ -47,7 +47,7 @@ @APICommand(name = "createAutoScaleVmProfile", description = "Creates a profile that contains information about the Instance which will be provisioned automatically by autoscale feature.", responseObject = AutoScaleVmProfileResponse.class, entityType = {AutoScaleVmProfile.class}, - requestHasSensitiveInfo = false, + requestHasSensitiveInfo = true, responseHasSensitiveInfo = false) @SuppressWarnings("rawtypes") public class CreateAutoScaleVmProfileCmd extends BaseAsyncCreateCmd { 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 0db0fdf6d775..91c2d46ced6b 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 @@ -43,7 +43,7 @@ import com.cloud.user.Account; @APICommand(name = "updateAutoScaleVmProfile", description = "Updates an existing autoscale Instance profile.", responseObject = AutoScaleVmProfileResponse.class, entityType = {AutoScaleVmProfile.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = true, responseHasSensitiveInfo = false) public class UpdateAutoScaleVmProfileCmd extends BaseAsyncCustomIdCmd { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java index 7390d4d2da71..fa76651e9f05 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java @@ -42,7 +42,7 @@ responseObject = UserVmResponse.class, responseView = ResponseObject.ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, + requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.21.0", authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class CreateVMFromBackupCmd extends BaseDeployVMCmd { 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 81ee00c98a25..8e25049b0a24 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 @@ -43,7 +43,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "deployVirtualMachine", description = "Creates and automatically starts an Instance based on a service offering, disk offering, and Template.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false) + requestHasSensitiveInfo = true) public class DeployVMCmd extends BaseDeployVMCmd { ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java index 92ddfd5b2357..86b8c608ddb2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java @@ -37,7 +37,7 @@ responseObject = UserVmResponse.class, responseView = ResponseObject.ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, + requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}, since = "4.19.0") public class DeployVnfApplianceCmd extends DeployVMCmd implements UserCmd { diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java index 81cc5d4f2a8c..06db6ac609e1 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java @@ -106,7 +106,6 @@ public AsyncJobDaoImpl() { asyncJobTypeSearch = createSearchBuilder(Long.class); asyncJobTypeSearch.select(null, SearchCriteria.Func.COUNT, asyncJobTypeSearch.entity().getId()); - asyncJobTypeSearch.and("job_info", asyncJobTypeSearch.entity().getCmdInfo(),Op.LIKE); asyncJobTypeSearch.and("job_cmd", asyncJobTypeSearch.entity().getCmd(), Op.IN); asyncJobTypeSearch.and("status", asyncJobTypeSearch.entity().getStatus(), SearchCriteria.Op.EQ); asyncJobTypeSearch.done(); @@ -290,12 +289,16 @@ public long countPendingNonPseudoJobs(Long... msIds) { @Override public long countPendingJobs(String havingInfo, String... cmds) { + if (havingInfo != null) { + // job_cmd_info is encrypted for commands with sensitive requests, so match on the decrypted value + SearchCriteria sc = failureMsidAsyncJobSearch.create(); + sc.setParameters("status", JobInfo.Status.IN_PROGRESS); + sc.setParameters("job_cmd", (Object[])cmds); + return listBy(sc).stream().filter(job -> StringUtils.contains(job.getCmdInfo(), havingInfo)).count(); + } SearchCriteria sc = asyncJobTypeSearch.create(); sc.setParameters("status", JobInfo.Status.IN_PROGRESS); sc.setParameters("job_cmd", (Object[])cmds); - if (havingInfo != null) { - sc.setParameters("job_info", "%" + havingInfo + "%"); - } List results = customSearch(sc, null); return results.get(0); } diff --git a/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/AsyncJobDataHelperTest.java b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/AsyncJobDataHelperTest.java index f4d74475a23d..32b407a6db77 100644 --- a/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/AsyncJobDataHelperTest.java +++ b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/AsyncJobDataHelperTest.java @@ -16,9 +16,19 @@ // under the License. package org.apache.cloudstack.framework.jobs; +import java.util.List; + import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.vm.CreateVMFromBackupCmdByAdmin; +import org.apache.cloudstack.api.command.admin.vm.DeployVMCmdByAdmin; +import org.apache.cloudstack.api.command.admin.vm.DeployVnfApplianceCmdByAdmin; +import org.apache.cloudstack.api.command.user.autoscale.CreateAutoScaleVmProfileCmd; +import org.apache.cloudstack.api.command.user.autoscale.UpdateAutoScaleVmProfileCmd; +import org.apache.cloudstack.api.command.user.vm.CreateVMFromBackupCmd; +import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; +import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd; import org.apache.cloudstack.api.response.SuccessResponse; import org.junit.Assert; import org.junit.Test; @@ -175,6 +185,21 @@ public void decryptResultIfNeededBlankReturnsBlank() { Assert.assertEquals("", AsyncJobDataHelper.decryptResultIfNeeded(SensitiveResponseAsyncCmd.class.getName(), "")); } + @Test + public void encryptInfoIfNeededEncryptsCommandsWithUserData() { + try (MockedStatic encryptionKeyCheckerMock = Mockito.mockStatic(EncryptionSecretKeyChecker.class); + MockedStatic dbEncryptionMock = Mockito.mockStatic(DBEncryptionUtil.class)) { + encryptionKeyCheckerMock.when(EncryptionSecretKeyChecker::useEncryption).thenReturn(true); + dbEncryptionMock.when(() -> DBEncryptionUtil.encrypt(RAW_CMD_INFO)).thenReturn("encrypted-value"); + + for (Class cmdClass : List.of(DeployVMCmd.class, DeployVMCmdByAdmin.class, DeployVnfApplianceCmd.class, + DeployVnfApplianceCmdByAdmin.class, CreateVMFromBackupCmd.class, CreateVMFromBackupCmdByAdmin.class, + CreateAutoScaleVmProfileCmd.class, UpdateAutoScaleVmProfileCmd.class)) { + Assert.assertEquals(cmdClass.getSimpleName(), "encrypted-value", AsyncJobDataHelper.encryptInfoIfNeeded(cmdClass.getName(), RAW_CMD_INFO)); + } + } + } + @APICommand(name = "testSensitiveAsyncCmd", responseObject = SuccessResponse.class, requestHasSensitiveInfo = true, responseHasSensitiveInfo = false) private static class SensitiveAsyncCmd extends BaseAsyncCmd { diff --git a/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImplTest.java b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImplTest.java new file mode 100644 index 000000000000..8b289341689d --- /dev/null +++ b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImplTest.java @@ -0,0 +1,41 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.framework.jobs.dao; + +import java.util.List; + +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; + +import com.cloud.utils.db.SearchCriteria; + +public class AsyncJobDaoImplTest { + + @Test + public void countPendingJobsMatchesOnDecryptedCmdInfo() { + AsyncJobDaoImpl asyncJobDao = Mockito.spy(new AsyncJobDaoImpl()); + AsyncJobVO matchingJob = Mockito.mock(AsyncJobVO.class); + Mockito.when(matchingJob.getCmdInfo()).thenReturn("{\"id\":\"backup-uuid\"}"); + AsyncJobVO otherJob = Mockito.mock(AsyncJobVO.class); + Mockito.when(otherJob.getCmdInfo()).thenReturn("{\"id\":\"other-uuid\"}"); + Mockito.doReturn(List.of(matchingJob, otherJob)).when(asyncJobDao).listBy(Mockito.any(SearchCriteria.class)); + + Assert.assertEquals(1L, asyncJobDao.countPendingJobs("backup-uuid", "SomeCmd")); + } +} diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index a0dfaddfe5fa..f478e4bbdcee 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -260,9 +260,14 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private static final Set sensitiveFields = new HashSet<>(Arrays.asList( "password", "secretkey", "apikey", "token", "sessionkey", "accesskey", "signature", - "authorization", "credential", "secret" + "authorization", "credential", "secret", "userdata" )); + private static boolean isSensitiveField(final String key) { + final String lowerKey = key.toLowerCase(); + return sensitiveFields.stream().anyMatch(lowerKey::contains); + } + private static final ConfigKey IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Integer.class , "integration.api.port" @@ -399,7 +404,7 @@ public void handleAsyncJobPublishEvent(String subject, String senderAddress, Obj if (info != null) { Type type = new TypeToken>(){}.getType(); Map cmdInfo = ApiGsonHelper.getBuilder().create().fromJson(info, type); - cmdInfoObj.putAll(cmdInfo); + cmdInfo.forEach((key, value) -> cmdInfoObj.put(key, isSensitiveField(key) ? "******" : value)); String eventTypeObj = cmdInfo.get("cmdEventType"); if (eventTypeObj != null) { cmdEventType = eventTypeObj; @@ -646,12 +651,8 @@ public String handleRequest(final Map params, final String responseType, final S final String keyStr = (String) key; final String[] value = (String[]) params.get(key); - String lowerKeyStr = keyStr.toLowerCase(); - boolean isSensitive = sensitiveFields.stream() - .anyMatch(lowerKeyStr::contains); - String logValue; - if (isSensitive) { + if (isSensitiveField(keyStr)) { logValue = "******"; // mask sensitive values } else { logValue = (value == null) ? "'null'" : value[0]; diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index 8ecdc458018c..29ddca78e573 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -24,8 +24,12 @@ import com.cloud.user.User; import com.cloud.user.UserAccount; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.Pair; import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.events.Event; +import org.apache.cloudstack.framework.events.EventDistributor; +import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.user.UserPasswordResetManager; import org.junit.After; import org.junit.AfterClass; @@ -34,6 +38,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedConstruction; @@ -245,4 +250,24 @@ public void testCheckCommandAvailableDeniesNonMatchingCidr() throws Exception { public void testCheckCommandAvailableThrowsWhenUserNull() throws Exception { ReflectionTestUtils.invokeMethod(apiServer, "checkCommandAvailable", null, "listVirtualMachines", InetAddress.getByName("127.0.0.1")); } + + @Test + public void handleAsyncJobPublishEventMasksSensitiveCmdInfo() { + AsyncJob job = Mockito.mock(AsyncJob.class); + Mockito.when(job.getDispatcher()).thenReturn("ApiAsyncJobDispatcher"); + Mockito.when(job.getCmdInfo()).thenReturn("{\"userdata\":\"I2Nsb3VkLWNvbmZpZw==\",\"name\":\"vm1\"}"); + User user = Mockito.mock(User.class); + Mockito.when(accountMgr.getUserIncludingRemoved(Mockito.anyLong())).thenReturn(user); + Mockito.when(accountMgr.getAccount(Mockito.anyLong())).thenReturn(Mockito.mock(Account.class)); + EventDistributor eventDistributor = Mockito.mock(EventDistributor.class); + apiServer.setEventDistributor(eventDistributor); + + apiServer.handleAsyncJobPublishEvent(null, null, new Pair<>(job, "test")); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(Event.class); + Mockito.verify(eventDistributor).publish(eventCaptor.capture()); + String description = eventCaptor.getValue().getDescription(); + Assert.assertFalse(description.contains("I2Nsb3VkLWNvbmZpZw==")); + Assert.assertTrue(description.contains("vm1")); + } }