volume upload: added validation for file formats merged TemplateUtils and ImageStoreUtil to a singe ImageStoreUtil also added a unittest for ImageStoreUtil
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/018023c1 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/018023c1 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/018023c1 Branch: refs/heads/master Commit: 018023c1ef7fc5216a607e40811ff20f185d295c Parents: d5dffb5 Author: Rajani Karuturi <rajanikarut...@gmail.com> Authored: Thu Mar 19 11:54:51 2015 +0530 Committer: Rajani Karuturi <rajanikarut...@gmail.com> Committed: Fri Mar 20 16:25:13 2015 +0530 ---------------------------------------------------------------------- .../template/HttpTemplateDownloader.java | 4 +- .../com/cloud/storage/VolumeApiServiceImpl.java | 2 +- .../com/cloud/template/TemplateManagerImpl.java | 2 +- .../resource/HttpUploadServerHandler.java | 10 ++ .../resource/NfsSecondaryStorageResource.java | 14 +++ utils/src/com/cloud/utils/ImageStoreUtil.java | 39 ------- .../utils/imagestore/ImageStoreUtil.java | 110 +++++++++++++++++++ .../utils/template/TemplateUtils.java | 97 ---------------- .../utils/imagestore/ImageStoreUtilTest.java | 38 +++++++ 9 files changed, 176 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/core/src/com/cloud/storage/template/HttpTemplateDownloader.java ---------------------------------------------------------------------- diff --git a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java b/core/src/com/cloud/storage/template/HttpTemplateDownloader.java index 5644af4..632a809 100644 --- a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java +++ b/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(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/server/src/com/cloud/storage/VolumeApiServiceImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index 567a742..4ff7686 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/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; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/server/src/com/cloud/template/TemplateManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java index 9747ad0..10ff77f 100755 --- a/server/src/com/cloud/template/TemplateManagerImpl.java +++ b/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; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java ---------------------------------------------------------------------- diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java index 44a2b60..4a3fa86 100644 --- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java +++ b/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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java ---------------------------------------------------------------------- diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index f67015c..8ccbcb6 100755 --- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/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) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/com/cloud/utils/ImageStoreUtil.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/ImageStoreUtil.java b/utils/src/com/cloud/utils/ImageStoreUtil.java deleted file mode 100644 index 52f1324..0000000 --- a/utils/src/com/cloud/utils/ImageStoreUtil.java +++ /dev/null @@ -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; - } -} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java ---------------------------------------------------------------------- diff --git a/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java b/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java new file mode 100644 index 0000000..ed13360 --- /dev/null +++ b/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java @@ -0,0 +1,110 @@ +/* + * 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.utils.imagestore; + +import com.cloud.utils.script.Script; +import org.apache.commons.lang.StringUtils; +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; + } + + // 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 + public static String checkTemplateFormat(String path, String uripath) { + // note 'path' was generated by us so it should be safe on the cmdline, be wary of 'url' + String command = "file "; + if (isCompressedExtension(uripath)) { + command = "file -z "; + } + String output = Script.runSimpleBashScript(command + path + " | cut -d: -f2", 60000); + + // vmdk + if ((output.contains("VMware") || output.contains("data")) && isCorrectExtension(uripath, "vmdk")) { + s_logger.debug("File at path " + path + " looks like a vmware image :" + output); + return ""; + } + // raw + if ((output.contains("x86 boot") || output.contains("data")) && (isCorrectExtension(uripath, "raw") || isCorrectExtension(uripath, "img"))) { + s_logger.debug("File at path " + path + " looks like a raw image :" + output); + return ""; + } + // qcow2 + if (output.contains("QEMU QCOW") && isCorrectExtension(uripath, "qcow2")) { + s_logger.debug("File at path " + path + " looks like QCOW2 : " + output); + return ""; + } + // vhd + if (output.contains("Microsoft Disk Image") && (isCorrectExtension(uripath, "vhd") || isCorrectExtension(uripath, "vhdx"))) { + s_logger.debug("File at path " + path + " looks like vhd : " + output); + return ""; + } + // ova + if (output.contains("POSIX tar") && isCorrectExtension(uripath, "ova")) { + s_logger.debug("File at path " + path + " looks like ova : " + output); + return ""; + } + + //lxc + if (output.contains("POSIX tar") && isCorrectExtension(uripath, "tar")) { + s_logger.debug("File at path " + path + " looks like just tar : " + output); + return ""; + } + + if (output.contains("ISO 9660") && isCorrectExtension(uripath, "iso")) { + s_logger.debug("File at path " + path + " looks like an iso : " + output); + return ""; + } + return output; + } + + private static boolean isCorrectExtension(String path, String ext) { + if (path.toLowerCase().endsWith(ext) + || path.toLowerCase().endsWith(ext + ".gz") + || path.toLowerCase().endsWith(ext + ".bz2") + || path.toLowerCase().endsWith(ext + ".zip")) { + return true; + } + return false; + } + + private static boolean isCompressedExtension(String path) { + if (path.toLowerCase().endsWith(".gz") + || path.toLowerCase().endsWith(".bz2") + || path.toLowerCase().endsWith(".zip")) { + return true; + } + return false; + } +} + http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java ---------------------------------------------------------------------- diff --git a/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java b/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java deleted file mode 100644 index 53aa911..0000000 --- a/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java +++ /dev/null @@ -1,97 +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 org.apache.cloudstack.utils.template; - -import org.apache.log4j.Logger; - -import com.cloud.utils.script.Script; - -public class TemplateUtils { - public static final Logger s_logger = Logger.getLogger(TemplateUtils.class.getName()); - - // 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 - public static String checkTemplateFormat(String path, String uripath) { - // note 'path' was generated by us so it should be safe on the cmdline, be wary of 'url' - String command = "file "; - if (isCompressedExtension(uripath)) { - command = "file -z "; - } - String output = Script.runSimpleBashScript(command + path + " | cut -d: -f2", 60000); - - // vmdk - if ((output.contains("VMware") || output.contains("data")) && isCorrectExtension(uripath, "vmdk")) { - s_logger.debug("File at path " + path + " looks like a vmware image :" + output); - return ""; - } - // raw - if ((output.contains("x86 boot") || output.contains("data")) && (isCorrectExtension(uripath, "raw") || isCorrectExtension(uripath, "img"))) { - s_logger.debug("File at path " + path + " looks like a raw image :" + output); - return ""; - } - // qcow2 - if (output.contains("QEMU QCOW") && isCorrectExtension(uripath, "qcow2")) { - s_logger.debug("File at path " + path + " looks like QCOW2 : " + output); - return ""; - } - // vhd - if (output.contains("Microsoft Disk Image") && (isCorrectExtension(uripath, "vhd") || isCorrectExtension(uripath, "vhdx"))) { - s_logger.debug("File at path " + path + " looks like vhd : " + output); - return ""; - } - // ova - if (output.contains("POSIX tar") && isCorrectExtension(uripath, "ova")) { - s_logger.debug("File at path " + path + " looks like ova : " + output); - return ""; - } - - //lxc - if (output.contains("POSIX tar") && isCorrectExtension(uripath, "tar")) { - s_logger.debug("File at path " + path + " looks like just tar : " + output); - return ""; - } - - if (output.contains("ISO 9660") && isCorrectExtension(uripath, "iso")) { - s_logger.debug("File at path " + path + " looks like an iso : " + output); - return ""; - } - return output; - } - - public static boolean isCorrectExtension(String path, String ext) { - if (path.toLowerCase().endsWith(ext) - || path.toLowerCase().endsWith(ext + ".gz") - || path.toLowerCase().endsWith(ext + ".bz2") - || path.toLowerCase().endsWith(ext + ".zip")) { - return true; - } - return false; - } - - public static boolean isCompressedExtension(String path) { - if (path.toLowerCase().endsWith(".gz") - || path.toLowerCase().endsWith(".bz2") - || path.toLowerCase().endsWith(".zip")) { - return true; - } - return false; - } -} - http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java ---------------------------------------------------------------------- diff --git a/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java b/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java new file mode 100644 index 0000000..e1b5578 --- /dev/null +++ b/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); + } + +} \ No newline at end of file
