diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index 5d76393d81e..cc17e48fe07 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -85,6 +85,7 @@ import org.apache.cloudstack.utils.security.DigestHelper; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.io.FileUtils; import org.apache.commons.io.FilenameUtils; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; @@ -201,6 +202,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S private static final String POST_UPLOAD_KEY_LOCATION = "/etc/cloudstack/agent/ms-psk"; private static final String ORIGINAL_FILE_EXTENSION = ".orig"; + private static final String USE_HTTPS_TO_UPLOAD = "useHttpsToUpload"; + private static final Map updatableConfigData = Maps.newHashMap(); static { @@ -3537,7 +3540,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S return null; } - private String getPostUploadPSK() { + protected String getPostUploadPSK() { if (_ssvmPSK == null) { try { _ssvmPSK = FileUtils.readFileToString(new File(POST_UPLOAD_KEY_LOCATION), "utf-8"); @@ -3573,14 +3576,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S throw new InvalidParameterValueException("content length is not set in the request or has invalid value."); } - //validate signature - String fullUrl = "https://" + hostname + "/upload/" + uuid; - String computedSignature = EncryptionUtil.generateSignature(metadata + fullUrl + timeout, getPostUploadPSK()); - boolean isSignatureValid = computedSignature.equals(signature); - if (!isSignatureValid) { - updateStateMapWithError(uuid, "signature validation failed."); - throw new InvalidParameterValueException("signature validation failed."); - } + validatePostUploadRequestSignature(signature, hostname, uuid, metadata, timeout); //validate timeout DateTime timeoutDateTime = DateTime.parse(timeout, ISODateTimeFormat.dateTime()); @@ -3590,6 +3586,48 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S } } + /** + * Validates whether the provided signature matches the signature generated from the other parameters; + * throws an InvalidParameterValueException if it does not. + */ + protected void validatePostUploadRequestSignature(String signature, String hostname, String uuid, String metadata, String timeout) { + s_logger.trace(String.format("Validating signature [%s] for post upload request [%s].", signature, uuid)); + String protocol = getUploadProtocol(); + String fullUrl = String.format("%s://%s/upload/%s", protocol, hostname, uuid); + String data = String.format("%s%s%s", metadata, fullUrl, timeout); + + String computedSignature = EncryptionUtil.generateSignature(data, getPostUploadPSK()); + s_logger.debug(String.format("Computed signature for post upload request [%s] is [%s].", uuid, computedSignature)); + + boolean isSignatureValid = computedSignature.equals(signature); + if (!isSignatureValid) { + s_logger.debug(String.format("Signature for post upload request [%s] is invalid.", uuid)); + String errorMsg = "signature validation failed."; + updateStateMapWithError(uuid, errorMsg); + throw new InvalidParameterValueException(errorMsg); + } + s_logger.debug(String.format("Signature for post upload request [%s] is valid.", uuid)); + } + + /** + * Returns the protocol used for uploads as a string. + */ + protected String getUploadProtocol() { + if (useHttpsToUpload()) { + s_logger.debug(String.format("Param [%s] is set to true; therefore, HTTPS is being used.", USE_HTTPS_TO_UPLOAD)); + return NetUtils.HTTPS_PROTO; + } + s_logger.debug(String.format("Param [%s] is set to false; therefore, HTTP is being used.", USE_HTTPS_TO_UPLOAD)); + return NetUtils.HTTP_PROTO; + } + + /** + * Retrieves the value of "useHttpsToUpload" from the params as a boolean + */ + protected boolean useHttpsToUpload() { + return BooleanUtils.toBoolean((String) _params.get(USE_HTTPS_TO_UPLOAD)); + } + private TemplateOrVolumePostUploadCommand getTemplateOrVolumePostUploadCmd(String metadata) { TemplateOrVolumePostUploadCommand cmd = null; try { diff --git a/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java index cd6444a8b4b..72b9f5abe19 100644 --- a/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java +++ b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java @@ -28,6 +28,9 @@ import java.nio.file.Path; import java.util.List; import java.util.stream.Stream; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.EncryptionUtil; +import com.cloud.utils.net.NetUtils; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyAnswer; import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyCommand; @@ -51,6 +54,22 @@ public class NfsSecondaryStorageResourceTest { @Spy private NfsSecondaryStorageResource resource; + private static final String HOSTNAME = "hostname"; + + private static final String UUID = "uuid"; + + private static final String METADATA = "metadata"; + + private static final String TIMEOUT = "timeout"; + + private static final String PSK = "6HyGMx9Vat7rZw1pMZrM4OlD4FFwLUPznTsFqVFSOIvk0mAWMRCVZ6UCq42gZvhp"; + + private static final String PROTOCOL = NetUtils.HTTP_PROTO; + + private static final String EXPECTED_SIGNATURE = "expectedSignature"; + + private static final String COMPUTED_SIGNATURE = "computedSignature"; + @Test public void testSwiftWriteMetadataFile() throws Exception { String metaFileName = "test_metadata_file"; @@ -141,4 +160,86 @@ public class NfsSecondaryStorageResourceTest { Assert.assertEquals(dir + File.separator + fileName + ".ovf", result.get(1)); } } + + private void prepareForValidatePostUploadRequestSignatureTests(MockedStatic encryptionUtilMock) { + Mockito.doReturn(PROTOCOL).when(resource).getUploadProtocol(); + Mockito.doReturn(PSK).when(resource).getPostUploadPSK(); + encryptionUtilMock.when(() -> EncryptionUtil.generateSignature(Mockito.anyString(), Mockito.anyString())).thenReturn(COMPUTED_SIGNATURE); + String fullUrl = String.format("%s://%s/upload/%s", PROTOCOL, HOSTNAME, UUID); + String data = String.format("%s%s%s", METADATA, fullUrl, TIMEOUT); + encryptionUtilMock.when(() -> EncryptionUtil.generateSignature(data, PSK)).thenReturn(EXPECTED_SIGNATURE); + } + + @Test(expected = InvalidParameterValueException.class) + public void validatePostUploadRequestSignatureTestThrowExceptionWhenProtocolDiffers() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + Mockito.doReturn(NetUtils.HTTPS_PROTO).when(resource).getUploadProtocol(); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, TIMEOUT); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void validatePostUploadRequestSignatureTestThrowExceptionWhenHostnameDiffers() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, "test", UUID, METADATA, TIMEOUT); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void validatePostUploadRequestSignatureTestThrowExceptionWhenUuidDiffers() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, "test", METADATA, TIMEOUT); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void validatePostUploadRequestSignatureTestThrowExceptionWhenMetadataDiffers() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, "test", TIMEOUT); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void validatePostUploadRequestSignatureTestThrowExceptionWhenTimeoutDiffers() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, "test"); + } + } + + @Test + public void validatePostUploadRequestSignatureTestSuccessWhenDataIsTheSame() { + try (MockedStatic encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) { + prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock); + + resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, TIMEOUT); + } + } + + @Test + public void getUploadProtocolTestReturnHttpsWhenUseHttpsToUploadIsTrue() { + Mockito.doReturn(true).when(resource).useHttpsToUpload(); + + String result = resource.getUploadProtocol(); + + Assert.assertEquals(NetUtils.HTTPS_PROTO, result); + } + + @Test + public void getUploadProtocolTestReturnHttpWhenUseHttpsToUploadIsFalse() { + Mockito.doReturn(false).when(resource).useHttpsToUpload(); + + String result = resource.getUploadProtocol(); + + Assert.assertEquals(NetUtils.HTTP_PROTO, result); + } } diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index 137cebb516a..018912a2140 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -80,6 +80,7 @@ public class NetUtils { public static final String ICMP6_PROTO = "icmp6"; public final static String ALL_PROTO = "all"; public final static String HTTP_PROTO = "http"; + public final static String HTTPS_PROTO = "https"; public final static String SSL_PROTO = "ssl"; public final static String ALL_IP4_CIDRS = "0.0.0.0/0";