volume upload: added validation for file formats

merged TemplateUtils and ImageStoreUtil to a singe ImageStoreUtil
also added a unittest for ImageStoreUtil

core/src/com/cloud/storage/template/HttpTemplateDownloader.java

@@ -28,6 +28,7 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Date;
 
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.commons.httpclient.Credentials;
 import org.apache.commons.httpclient.Header;
 import org.apache.commons.httpclient.HttpClient;
@@ -45,7 +46,6 @@ import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
-import org.apache.cloudstack.utils.template.TemplateUtils;
 
 import com.cloud.agent.api.storage.Proxy;
 import com.cloud.storage.StorageLayer;
@@ -259,7 +259,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
                         } catch (URISyntaxException e) {
                             s_logger.warn("Invalid download url: " + getDownloadUrl() + ", This should not happen since we have validated the url before!!");
                         }
-                        String unsupportedFormat = TemplateUtils.checkTemplateFormat(file.getAbsolutePath(), uripath);
+                        String unsupportedFormat = ImageStoreUtil.checkTemplateFormat(file.getAbsolutePath(), uripath);
                             if (unsupportedFormat == null || !unsupportedFormat.isEmpty()) {
                                  try {
                                      request.abort();

server/src/com/cloud/storage/VolumeApiServiceImpl.java

@@ -29,7 +29,6 @@ import java.util.concurrent.ExecutionException;
 import javax.inject.Inject;
 
 import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.ImageStoreUtil;
 import com.cloud.utils.db.TransactionCallbackWithException;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -39,6 +38,7 @@ import org.apache.cloudstack.api.response.GetUploadParamsResponse;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.log4j.Logger;
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;

server/src/com/cloud/template/TemplateManagerImpl.java

@@ -35,13 +35,13 @@ import javax.naming.ConfigurationException;
 
 import com.cloud.storage.ImageStoreUploadMonitorImpl;
 import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.ImageStoreUtil;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 
 import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
 import org.apache.cloudstack.api.response.GetUploadParamsResponse;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;

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

@@ -28,6 +28,8 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.exception.InvalidParameterValueException;
@@ -228,6 +230,14 @@ public class HttpUploadServerHandler extends SimpleChannelInboundHandler<HttpObj
                         FileUpload fileUpload = (FileUpload) data;
                         if (fileUpload.isCompleted()) {
                             fileReceived = true;
+                            String format = ImageStoreUtil.checkTemplateFormat(fileUpload.getFile().getAbsolutePath(), fileUpload.getFilename());
+                            if(StringUtils.isNotBlank(format)) {
+                                String errorString = "File type mismatch between the sent file and the actual content. Received: " + format;
+                                logger.error(errorString);
+                                responseContent.append(errorString);
+                                storageResource.updateStateMapWithError(uuid, errorString);
+                                return HttpResponseStatus.BAD_REQUEST;
+                            }
                             String status = storageResource.postUpload(uuid, fileUpload.getFile().getName());
                             if (status != null) {
                                 responseContent.append(status);

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

@@ -66,8 +66,10 @@ import io.netty.handler.logging.LogLevel;
 import io.netty.handler.logging.LoggingHandler;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
 import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
@@ -2663,6 +2665,18 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
 
         String fileSavedTempLocation = uploadEntity.getInstallPathPrefix() + "/" + filename;
 
+        String uploadedFileExtension = FilenameUtils.getExtension(filename);
+        String userSelectedFormat= "vhd";
+        if(uploadedFileExtension.equals("zip") || uploadedFileExtension.equals("bz2") || uploadedFileExtension.equals("gz")) {
+            userSelectedFormat += "." + uploadedFileExtension;
+        }
+        String formatError = ImageStoreUtil.checkTemplateFormat(fileSavedTempLocation, userSelectedFormat);
+        if(StringUtils.isNotBlank(formatError)) {
+            String errorString = "File type mismatch between uploaded file and selected format. Selected file format: " + uploadEntity.getFormat() + ". Received: " + formatError;
+            s_logger.error(errorString);
+            return errorString;
+        }
+
         int imgSizeGigs = getSizeInGB(_storage.getSize(fileSavedTempLocation));
         int maxSize = uploadEntity.getMaxSizeInGB();
         if(imgSizeGigs > maxSize) {

utils/src/com/cloud/utils/ImageStoreUtil.java

@@ -1,39 +0,0 @@
-/*
- * 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 com.cloud.utils;
-
-import org.apache.log4j.Logger;
-
-public class ImageStoreUtil {
-    public static final Logger s_logger = Logger.getLogger(ImageStoreUtil.class.getName());
-
-    public static String generatePostUploadUrl(String ssvmUrlDomain, String ipAddress, String uuid) {
-        String hostname = ipAddress;
-
-        //if ssvm url domain is present, use it to construct hostname in the format 1-2-3-4.domain
-        // if the domain name is not present, ssl validation fails and has to be ignored
-        if(StringUtils.isNotBlank(ssvmUrlDomain)) {
-            hostname = ipAddress.replace(".", "-");
-            hostname = hostname + ssvmUrlDomain.substring(1);
-        }
-
-        //only https works with postupload and url format is fixed
-        return "https://" + hostname + "/upload/" + uuid;
-    }
-}

utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java

@@ -1,30 +1,43 @@
-//
+/*
-// Licensed to the Apache Software Foundation (ASF) under one
+ * Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
+ * or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
+ * distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
+ * regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
+ * to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
+ * "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
+ * with the License.  You may obtain a copy of the License at
-//
+ *
-//   http://www.apache.org/licenses/LICENSE-2.0
+ *   http://www.apache.org/licenses/LICENSE-2.0
-//
+ *
-// Unless required by applicable law or agreed to in writing,
+ * Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
+ * software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
+ * KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
+ * specific language governing permissions and limitations
-// under the License.
+ * under the License.
-//
+ */
-
+package org.apache.cloudstack.utils.imagestore;
-package org.apache.cloudstack.utils.template;
-
-import org.apache.log4j.Logger;
 
 import com.cloud.utils.script.Script;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
 
-public class TemplateUtils {
+public class ImageStoreUtil {
-    public static final Logger s_logger = Logger.getLogger(TemplateUtils.class.getName());
+    public static final Logger s_logger = Logger.getLogger(ImageStoreUtil.class.getName());
+
+    public static String generatePostUploadUrl(String ssvmUrlDomain, String ipAddress, String uuid) {
+        String hostname = ipAddress;
+
+        //if ssvm url domain is present, use it to construct hostname in the format 1-2-3-4.domain
+        // if the domain name is not present, ssl validation fails and has to be ignored
+        if(StringUtils.isNotBlank(ssvmUrlDomain)) {
+            hostname = ipAddress.replace(".", "-");
+            hostname = hostname + ssvmUrlDomain.substring(1);
+        }
+
+        //only https works with postupload and url format is fixed
+        return "https://" + hostname + "/upload/" + uuid;
+    }
 
     // given a path, returns empty if path is supported image, and the file type if unsupported
     // this is meant to catch things like accidental upload of ASCII text .vmdk descriptor
@@ -75,7 +88,7 @@ public class TemplateUtils {
         return output;
     }
 
-    public static boolean isCorrectExtension(String path, String ext) {
+    private static boolean isCorrectExtension(String path, String ext) {
         if (path.toLowerCase().endsWith(ext)
             || path.toLowerCase().endsWith(ext + ".gz")
             || path.toLowerCase().endsWith(ext + ".bz2")
@@ -85,7 +98,7 @@ public class TemplateUtils {
         return false;
     }
 
-    public static boolean isCompressedExtension(String path) {
+    private static boolean isCompressedExtension(String path) {
         if (path.toLowerCase().endsWith(".gz")
             || path.toLowerCase().endsWith(".bz2")
             || path.toLowerCase().endsWith(".zip")) {

utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java

@@ -0,0 +1,38 @@
+package org.apache.cloudstack.utils.imagestore;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.UUID;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ImageStoreUtilTest {
+
+    @Test
+    public void testgeneratePostUploadUrl() throws MalformedURLException {
+        String ssvmdomain = "*.realhostip.com";
+        String ipAddress = "10.147.28.14";
+        String uuid = UUID.randomUUID().toString();
+
+        //ssvm domain is not set
+        String url = ImageStoreUtil.generatePostUploadUrl(null, ipAddress, uuid);
+        assertPostUploadUrl(url, ipAddress, uuid);
+
+        //ssvm domain is set to empty value
+        url = ImageStoreUtil.generatePostUploadUrl("", ipAddress, uuid);
+        assertPostUploadUrl(url, ipAddress, uuid);
+
+        //ssvm domain is set to a valid value
+        url = ImageStoreUtil.generatePostUploadUrl(ssvmdomain, ipAddress, uuid);
+        assertPostUploadUrl(url, ipAddress.replace(".", "-") + ssvmdomain.substring(1), uuid);
+    }
+
+    private void assertPostUploadUrl(String urlStr, String domain, String uuid) throws MalformedURLException {
+        URL url = new URL(urlStr);
+        Assert.assertNotNull(url);
+        Assert.assertEquals(url.getHost(), domain);
+        Assert.assertEquals(url.getPath(), "/upload/" + uuid);
+    }
+
+}