Skip to content

Commit 37a2350

Browse files
authored
Fix upload of volumes, templates and ISOs through HTTP (#8264)
When HTTP is used to upload volumes, templates and ISOs (configuration use.https.to.upload is set to false), an error occurs during the validation of the request's signature, preventing uploads from finishing successfully. This happens because an URL in the format String fullUrl = "https://" + hostname + "/upload/" + uuid is always created for the validation whether HTTP or HTTPS was being used. This PR addresses this issue, allowing users to upload volumes, templates and ISOs through HTTP.
1 parent cb62ce6 commit 37a2350

3 files changed

Lines changed: 149 additions & 9 deletions

File tree

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import org.apache.commons.codec.digest.DigestUtils;
8686
import org.apache.commons.io.FileUtils;
8787
import org.apache.commons.io.FilenameUtils;
88+
import org.apache.commons.lang3.BooleanUtils;
8889
import org.apache.commons.lang3.StringUtils;
8990
import org.apache.http.HttpEntity;
9091
import org.apache.http.HttpResponse;
@@ -201,6 +202,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
201202
private static final String POST_UPLOAD_KEY_LOCATION = "/etc/cloudstack/agent/ms-psk";
202203
private static final String ORIGINAL_FILE_EXTENSION = ".orig";
203204

205+
private static final String USE_HTTPS_TO_UPLOAD = "useHttpsToUpload";
206+
204207
private static final Map<String, String> updatableConfigData = Maps.newHashMap();
205208
static {
206209

@@ -3537,7 +3540,7 @@ public String postUpload(String uuid, String filename, long processTimeout) {
35373540
return null;
35383541
}
35393542

3540-
private String getPostUploadPSK() {
3543+
protected String getPostUploadPSK() {
35413544
if (_ssvmPSK == null) {
35423545
try {
35433546
_ssvmPSK = FileUtils.readFileToString(new File(POST_UPLOAD_KEY_LOCATION), "utf-8");
@@ -3573,14 +3576,7 @@ public void validatePostUploadRequest(String signature, String metadata, String
35733576
throw new InvalidParameterValueException("content length is not set in the request or has invalid value.");
35743577
}
35753578

3576-
//validate signature
3577-
String fullUrl = "https://" + hostname + "/upload/" + uuid;
3578-
String computedSignature = EncryptionUtil.generateSignature(metadata + fullUrl + timeout, getPostUploadPSK());
3579-
boolean isSignatureValid = computedSignature.equals(signature);
3580-
if (!isSignatureValid) {
3581-
updateStateMapWithError(uuid, "signature validation failed.");
3582-
throw new InvalidParameterValueException("signature validation failed.");
3583-
}
3579+
validatePostUploadRequestSignature(signature, hostname, uuid, metadata, timeout);
35843580

35853581
//validate timeout
35863582
DateTime timeoutDateTime = DateTime.parse(timeout, ISODateTimeFormat.dateTime());
@@ -3590,6 +3586,48 @@ public void validatePostUploadRequest(String signature, String metadata, String
35903586
}
35913587
}
35923588

3589+
/**
3590+
* Validates whether the provided signature matches the signature generated from the other parameters;
3591+
* throws an InvalidParameterValueException if it does not.
3592+
*/
3593+
protected void validatePostUploadRequestSignature(String signature, String hostname, String uuid, String metadata, String timeout) {
3594+
s_logger.trace(String.format("Validating signature [%s] for post upload request [%s].", signature, uuid));
3595+
String protocol = getUploadProtocol();
3596+
String fullUrl = String.format("%s://%s/upload/%s", protocol, hostname, uuid);
3597+
String data = String.format("%s%s%s", metadata, fullUrl, timeout);
3598+
3599+
String computedSignature = EncryptionUtil.generateSignature(data, getPostUploadPSK());
3600+
s_logger.debug(String.format("Computed signature for post upload request [%s] is [%s].", uuid, computedSignature));
3601+
3602+
boolean isSignatureValid = computedSignature.equals(signature);
3603+
if (!isSignatureValid) {
3604+
s_logger.debug(String.format("Signature for post upload request [%s] is invalid.", uuid));
3605+
String errorMsg = "signature validation failed.";
3606+
updateStateMapWithError(uuid, errorMsg);
3607+
throw new InvalidParameterValueException(errorMsg);
3608+
}
3609+
s_logger.debug(String.format("Signature for post upload request [%s] is valid.", uuid));
3610+
}
3611+
3612+
/**
3613+
* Returns the protocol used for uploads as a string.
3614+
*/
3615+
protected String getUploadProtocol() {
3616+
if (useHttpsToUpload()) {
3617+
s_logger.debug(String.format("Param [%s] is set to true; therefore, HTTPS is being used.", USE_HTTPS_TO_UPLOAD));
3618+
return NetUtils.HTTPS_PROTO;
3619+
}
3620+
s_logger.debug(String.format("Param [%s] is set to false; therefore, HTTP is being used.", USE_HTTPS_TO_UPLOAD));
3621+
return NetUtils.HTTP_PROTO;
3622+
}
3623+
3624+
/**
3625+
* Retrieves the value of "useHttpsToUpload" from the params as a boolean
3626+
*/
3627+
protected boolean useHttpsToUpload() {
3628+
return BooleanUtils.toBoolean((String) _params.get(USE_HTTPS_TO_UPLOAD));
3629+
}
3630+
35933631
private TemplateOrVolumePostUploadCommand getTemplateOrVolumePostUploadCmd(String metadata) {
35943632
TemplateOrVolumePostUploadCommand cmd = null;
35953633
try {

services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
import java.util.List;
2929
import java.util.stream.Stream;
3030

31+
import com.cloud.exception.InvalidParameterValueException;
32+
import com.cloud.utils.EncryptionUtil;
33+
import com.cloud.utils.net.NetUtils;
3134
import org.apache.cloudstack.storage.command.DeleteCommand;
3235
import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyAnswer;
3336
import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyCommand;
@@ -51,6 +54,22 @@ public class NfsSecondaryStorageResourceTest {
5154
@Spy
5255
private NfsSecondaryStorageResource resource;
5356

57+
private static final String HOSTNAME = "hostname";
58+
59+
private static final String UUID = "uuid";
60+
61+
private static final String METADATA = "metadata";
62+
63+
private static final String TIMEOUT = "timeout";
64+
65+
private static final String PSK = "6HyGMx9Vat7rZw1pMZrM4OlD4FFwLUPznTsFqVFSOIvk0mAWMRCVZ6UCq42gZvhp";
66+
67+
private static final String PROTOCOL = NetUtils.HTTP_PROTO;
68+
69+
private static final String EXPECTED_SIGNATURE = "expectedSignature";
70+
71+
private static final String COMPUTED_SIGNATURE = "computedSignature";
72+
5473
@Test
5574
public void testSwiftWriteMetadataFile() throws Exception {
5675
String metaFileName = "test_metadata_file";
@@ -141,4 +160,86 @@ public void testExecuteQuerySnapshotZoneCopyCommand() {
141160
Assert.assertEquals(dir + File.separator + fileName + ".ovf", result.get(1));
142161
}
143162
}
163+
164+
private void prepareForValidatePostUploadRequestSignatureTests(MockedStatic<EncryptionUtil> encryptionUtilMock) {
165+
Mockito.doReturn(PROTOCOL).when(resource).getUploadProtocol();
166+
Mockito.doReturn(PSK).when(resource).getPostUploadPSK();
167+
encryptionUtilMock.when(() -> EncryptionUtil.generateSignature(Mockito.anyString(), Mockito.anyString())).thenReturn(COMPUTED_SIGNATURE);
168+
String fullUrl = String.format("%s://%s/upload/%s", PROTOCOL, HOSTNAME, UUID);
169+
String data = String.format("%s%s%s", METADATA, fullUrl, TIMEOUT);
170+
encryptionUtilMock.when(() -> EncryptionUtil.generateSignature(data, PSK)).thenReturn(EXPECTED_SIGNATURE);
171+
}
172+
173+
@Test(expected = InvalidParameterValueException.class)
174+
public void validatePostUploadRequestSignatureTestThrowExceptionWhenProtocolDiffers() {
175+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
176+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
177+
Mockito.doReturn(NetUtils.HTTPS_PROTO).when(resource).getUploadProtocol();
178+
179+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, TIMEOUT);
180+
}
181+
}
182+
183+
@Test(expected = InvalidParameterValueException.class)
184+
public void validatePostUploadRequestSignatureTestThrowExceptionWhenHostnameDiffers() {
185+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
186+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
187+
188+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, "test", UUID, METADATA, TIMEOUT);
189+
}
190+
}
191+
192+
@Test(expected = InvalidParameterValueException.class)
193+
public void validatePostUploadRequestSignatureTestThrowExceptionWhenUuidDiffers() {
194+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
195+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
196+
197+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, "test", METADATA, TIMEOUT);
198+
}
199+
}
200+
201+
@Test(expected = InvalidParameterValueException.class)
202+
public void validatePostUploadRequestSignatureTestThrowExceptionWhenMetadataDiffers() {
203+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
204+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
205+
206+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, "test", TIMEOUT);
207+
}
208+
}
209+
210+
@Test(expected = InvalidParameterValueException.class)
211+
public void validatePostUploadRequestSignatureTestThrowExceptionWhenTimeoutDiffers() {
212+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
213+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
214+
215+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, "test");
216+
}
217+
}
218+
219+
@Test
220+
public void validatePostUploadRequestSignatureTestSuccessWhenDataIsTheSame() {
221+
try (MockedStatic<EncryptionUtil> encryptionUtilMock = Mockito.mockStatic(EncryptionUtil.class)) {
222+
prepareForValidatePostUploadRequestSignatureTests(encryptionUtilMock);
223+
224+
resource.validatePostUploadRequestSignature(EXPECTED_SIGNATURE, HOSTNAME, UUID, METADATA, TIMEOUT);
225+
}
226+
}
227+
228+
@Test
229+
public void getUploadProtocolTestReturnHttpsWhenUseHttpsToUploadIsTrue() {
230+
Mockito.doReturn(true).when(resource).useHttpsToUpload();
231+
232+
String result = resource.getUploadProtocol();
233+
234+
Assert.assertEquals(NetUtils.HTTPS_PROTO, result);
235+
}
236+
237+
@Test
238+
public void getUploadProtocolTestReturnHttpWhenUseHttpsToUploadIsFalse() {
239+
Mockito.doReturn(false).when(resource).useHttpsToUpload();
240+
241+
String result = resource.getUploadProtocol();
242+
243+
Assert.assertEquals(NetUtils.HTTP_PROTO, result);
244+
}
144245
}

utils/src/main/java/com/cloud/utils/net/NetUtils.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public class NetUtils {
8080
public static final String ICMP6_PROTO = "icmp6";
8181
public final static String ALL_PROTO = "all";
8282
public final static String HTTP_PROTO = "http";
83+
public final static String HTTPS_PROTO = "https";
8384
public final static String SSL_PROTO = "ssl";
8485

8586
public final static String ALL_IP4_CIDRS = "0.0.0.0/0";

0 commit comments

Comments
 (0)