CLOUDSTACK-10046 checksum validation for any java supported Digests-t…

…ype (apache#2246) * CLOUDSTACK-10046 digest helper for calculating checksums * CLOUDSTACK-10046 cleanup unused checksum code * CLOUDSTACK-10046 padding method proof of concept * CLOUDSTACK-10046 only compare checksums if old value is valid * Adding positive and negative tests for md5, sha-1 and sha-256, for xen, vmware and kvm hypervisors. KVM Results: Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 189, in test_02_1_create_template_with_checksum_sha1_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{sha-1}bf580a13f791d86acf3449a7b457a91a14389264" didn\'t match the given value, "{sha-1}someInvalidValue"\n'] === TestName: test_02_1_create_template_with_checksum_sha1_negative | Status : SUCCESS === === TestName: test_02_create_template_with_checksum_sha1 | Status : SUCCESS ===. Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 203, in test_03_1_create_template_with_checksum_sha256_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{SHA-256}efc03633f2b8f5db08acbcc5dc1be9028572dfd8f1c6c8ea663f0ef94b458c5" didn\'t match the given value, "{SHA-256}someInvalidValue"\n'] === TestName: test_03_1_create_template_with_checksum_sha256_negative | Status : SUCCESS === === TestName: test_03_create_template_with_checksum_sha256 | Status : SUCCESS === Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 217, in test_04_1_create_template_with_checksum_md5_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{md5}ada77653dcf1e59495a9e1ac670ad95f" didn\'t match the given value, "{md5}someInvalidValue"\n'] === TestName: test_04_1_create_template_with_checksum_md5_negative | Status : SUCCESS === === TestName: test_04_create_template_with_checksum_md5 | Status : SUCCESS === * CLOUDSTACK-10046 digest helper for calculating checksums * CLOUDSTACK-10046 cleanup unused checksum code * CLOUDSTACK-10046 padding method proof of concept * CLOUDSTACK-10046 only compare checksums if old value is valid * Adding positive and negative tests for md5, sha-1 and sha-256, for xen, vmware and kvm hypervisors. KVM Results: Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 189, in test_02_1_create_template_with_checksum_sha1_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{sha-1}bf580a13f791d86acf3449a7b457a91a14389264" didn\'t match the given value, "{sha-1}someInvalidValue"\n'] === TestName: test_02_1_create_template_with_checksum_sha1_negative | Status : SUCCESS === === TestName: test_02_create_template_with_checksum_sha1 | Status : SUCCESS ===. Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 203, in test_03_1_create_template_with_checksum_sha256_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{SHA-256}efc03633f2b8f5db08acbcc5dc1be9028572dfd8f1c6c8ea663f0ef94b458c5" didn\'t match the given value, "{SHA-256}someInvalidValue"\n'] === TestName: test_03_1_create_template_with_checksum_sha256_negative | Status : SUCCESS === === TestName: test_03_create_template_with_checksum_sha256 | Status : SUCCESS === Negative Test Passed - Exception Occurred Under template download ['Traceback (most recent call last):\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 217, in test_04_1_create_template_with_checksum_md5_negative\n self.download(self.apiclient, template.id)\n', ' File "/Users/bstoyanov/Documents/sb2/cloudstack/test/integration/smoke/test_templates.py", line 260, in download\n template.status)\n', 'Exception: Failed to download template: status - Failed post download script: checksum "{md5}ada77653dcf1e59495a9e1ac670ad95f" didn\'t match the given value, "{md5}someInvalidValue"\n'] === TestName: test_04_1_create_template_with_checksum_md5_negative | Status : SUCCESS === === TestName: test_04_create_template_with_checksum_md5 | Status : SUCCESS === * Adding additional test with no checksum added when registering template Result: test_05_create_template_with_no_checksum (integration.smoke.test_templates.TestCreateTemplateWithChecksum) ... === TestName: test_05_create_template_with_no_checksum | Status : SUCCESS === ok ---------------------------------------------------------------------- Ran 1 test in 42.320s OK * Fixing negative tests exception handling * Adding tests for ISO checksum validation and fixing a zero prefix failure test in templates * CLOUDSTACK-10046 padding * CLOUDSTACK-10046 usability additions * yet another IDE artifact hindering checkstyle
alokhom · Oct 11, 2017 · ed7811a · ed7811a
1 parent 7ca5b53
commit ed7811a
Show file tree

Hide file tree

Showing 23 changed files with 774 additions and 237 deletions.
diff --git a/api/src/org/apache/cloudstack/api/APICommand.java b/api/src/org/apache/cloudstack/api/APICommand.java
@@ -16,18 +16,19 @@
 // under the License.
 package org.apache.cloudstack.api;
 
-import static java.lang.annotation.ElementType.TYPE;
-
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.ResponseObject.ResponseView;
 
+import static java.lang.annotation.ElementType.TYPE;
+
 @Retention(RetentionPolicy.RUNTIME)
 @Target({TYPE})
 public @interface APICommand {
+
     Class<? extends BaseResponse> responseObject();
 
     String name() default "";

diff --git a/api/src/org/apache/cloudstack/api/AbstractGetUploadParamsCmd.java b/api/src/org/apache/cloudstack/api/AbstractGetUploadParamsCmd.java
@@ -18,15 +18,15 @@
  */
 package org.apache.cloudstack.api;
 
+import java.net.URL;
+import java.util.UUID;
+
 import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.GetUploadParamsResponse;
 import org.apache.cloudstack.api.response.ProjectResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.log4j.Logger;
 
-import java.net.URL;
-import java.util.UUID;
-
 public abstract class AbstractGetUploadParamsCmd extends BaseCmd {
 
     public static final Logger s_logger = Logger.getLogger(AbstractGetUploadParamsCmd.class.getName());
@@ -42,7 +42,7 @@ public abstract class AbstractGetUploadParamsCmd extends BaseCmd {
             + "to be hosted on")
     private Long zoneId;
 
-    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the MD5 checksum value of this volume/template")
+    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the checksum value of this volume/template " + ApiConstants.CHECKSUM_PARAMETER_PREFIX_DESCRIPTION)
     private String checksum;
 
     @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "an optional accountName. Must be used with domainId.")

diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java
@@ -673,6 +673,12 @@ public class ApiConstants {
     public static final String ZONE_ID_LIST = "zoneids";
     public static final String DESTINATION_ZONE_ID_LIST = "destzoneids";
     public static final String ADMIN = "admin";
+    public static final String CHECKSUM_PARAMETER_PREFIX_DESCRIPTION = "The parameter containing the checksum will be considered a MD5sum if it is not prefixed\n"
+            + " and just a plain ascii/utf8 representation of a hexadecimal string. If it is required to\n"
+            + " use another algorithm the hexadecimal string is to be prefixed with a string of the form,\n"
+            + " \"{<algorithm>}\", not including the double quotes. In this <algorithm> is the exact string\n"
+            + " representing the java supported algorithm, i.e. MD5 or SHA-256. Note that java does not\n"
+            + " contain an algorithm called SHA256 or one called sha-256, only SHA-256.";
 
     public enum HostDetails {
         all, capacity, events, stats, min;

diff --git a/api/src/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java b/api/src/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java
@@ -94,7 +94,7 @@ public class RegisterIsoCmd extends BaseCmd {
     @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "an optional account name. Must be used with domainId.")
     private String accountName;
 
-    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the MD5 checksum value of this ISO")
+    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the checksum value of this ISO. " + ApiConstants.CHECKSUM_PARAMETER_PREFIX_DESCRIPTION)
     private String checksum;
 
     @Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, description = "Register ISO for the project")

diff --git a/api/src/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java b/api/src/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java
@@ -122,7 +122,7 @@ public class RegisterTemplateCmd extends BaseCmd {
     @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "an optional accountName. Must be used with domainId.")
     private String accountName;
 
-    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the MD5 checksum value of this template")
+    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the checksum value of this template. " + ApiConstants.CHECKSUM_PARAMETER_PREFIX_DESCRIPTION)
     private String checksum;
 
     @Parameter(name = ApiConstants.TEMPLATE_TAG, type = CommandType.STRING, description = "the tag for this template.")

diff --git a/api/src/org/apache/cloudstack/api/command/user/volume/UploadVolumeCmd.java b/api/src/org/apache/cloudstack/api/command/user/volume/UploadVolumeCmd.java
@@ -82,7 +82,7 @@ public class UploadVolumeCmd extends BaseAsyncCmd {
     @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "an optional accountName. Must be used with domainId.")
     private String accountName;
 
-    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the MD5 checksum value of this volume")
+    @Parameter(name = ApiConstants.CHECKSUM, type = CommandType.STRING, description = "the checksum value of this volume. " + ApiConstants.CHECKSUM_PARAMETER_PREFIX_DESCRIPTION)
     private String checksum;
 
     @Parameter(name = ApiConstants.IMAGE_STORE_UUID, type = CommandType.STRING, description = "Image store uuid")

diff --git a/core/src/com/cloud/agent/api/ComputeChecksumCommand.java b/core/src/com/cloud/agent/api/ComputeChecksumCommand.java
@@ -25,6 +25,7 @@
 public class ComputeChecksumCommand extends SsCommand {
     private DataStoreTO store;
     private String templatePath;
+    private String algorithm = "MD5";
 
     public ComputeChecksumCommand() {
         super();
@@ -35,6 +36,11 @@ public ComputeChecksumCommand(DataStoreTO store, String templatePath) {
         this.setStore(store);
     }
 
+    public ComputeChecksumCommand(DataStoreTO store, String templatePath, String algorithm) {
+        this(store,templatePath);
+        this.algorithm = algorithm;
+    }
+
     public String getTemplatePath() {
         return templatePath;
     }
@@ -43,8 +49,12 @@ public DataStoreTO getStore() {
         return store;
     }
 
-    public void setStore(DataStoreTO store) {
-        this.store = store;
+
+    public String getAlgorithm() {
+        return algorithm;
     }
 
+    void setStore(DataStoreTO store) {
+        this.store = store;
+    }
 }
diff --git a/engine/components-api/src/com/cloud/template/TemplateManager.java b/engine/components-api/src/com/cloud/template/TemplateManager.java
@@ -115,7 +115,7 @@ public interface TemplateManager {
 
     DataStore getImageStore(String storeUuid, Long zoneId);
 
-    String getChecksum(DataStore store, String templatePath);
+    String getChecksum(DataStore store, String templatePath, String algorithm);
 
     List<DataStore> getImageStoreByTemplate(long templateId, Long zoneId);
 

diff --git a/scripts/installer/createtmplt.sh b/scripts/installer/createtmplt.sh
@@ -39,6 +39,7 @@ fi
 
 verify_cksum() {
   digestalgo=""
+# NOTE this will only work with 0-padded checksums
   case ${#1} in
         32) digestalgo="md5sum" ;;
         40) digestalgo="sha1sum" ;;

diff --git a/scripts/installer/createvolume.sh b/scripts/installer/createvolume.sh
@@ -40,6 +40,7 @@ fi
 
 verify_cksum() {
   digestalgo=""
+# NOTE this will only work with 0-padded checksums
   case ${#1} in
         32) digestalgo="md5sum" ;;
         40) digestalgo="sha1sum" ;;

diff --git a/scripts/storage/qcow2/createtmplt.sh b/scripts/storage/qcow2/createtmplt.sh
@@ -21,7 +21,7 @@
 # createtmplt.sh -- install a template
 
 usage() {
-  printf "Usage: %s: -t <template-fs> -n <templatename> -f <root disk file> -s <size in Gigabytes> -c <md5 cksum> -d <descr> -h  [-u]\n" $(basename $0) >&2
+  printf "Usage: %s: -t <template-fs> -n <templatename> -f <root disk file> -s <size in Gigabytes> -c <snapshot name> -d <descr> -h  [-u]\n" $(basename $0) >&2
 }
 
 
@@ -37,27 +37,6 @@ then
    fi
 fi
 
-
-verify_cksum() {
-  digestalgo=""
-  case ${#1} in
-        32) digestalgo="md5sum" ;;
-        40) digestalgo="sha1sum" ;;
-        56) digestalgo="sha224sum" ;;
-        64) digestalgo="sha256sum" ;;
-        96) digestalgo="sha384sum" ;;
-        128) digestalgo="sha512sum" ;;
-        *) echo "Please provide valid cheksum" ; exit 3 ;;
-  esac
-  echo  "$1  $2" | $digestalgo  -c --status
-  #printf "$1\t$2" | $digestalgo  -c --status
-  if [ $? -gt 0 ] 
-  then
-    printf "Checksum failed, not proceeding with install\n"
-    exit 3
-  fi
-}
-
 untar() {
   local ft=$(file $1| awk -F" " '{print $2}')
   local basedir=$(dirname $1)
@@ -166,7 +145,6 @@ do
 		tmpltimg="$OPTARG"
 		;;
   s)	sflag=1
-		sflag=1
 		;;
   c)	cflag=1
 		snapshotName="$OPTARG"

diff --git a/scripts/storage/qcow2/createvolume.sh b/scripts/storage/qcow2/createvolume.sh
@@ -22,7 +22,7 @@
 # createvol.sh -- install a volume
 
 usage() {
-  printf "Usage: %s: -t <volume-fs> -n <volumename> -f <root disk file> -s <size in Gigabytes> -c <md5 cksum> -d <descr> -h  [-u]\n" $(basename $0) >&2
+  printf "Usage: %s: -t <volume-fs> -n <volumename> -f <root disk file> -s <size in Gigabytes> -c <snapshot name> -d <descr> -h  [-u]\n" $(basename $0) >&2
 }
 
 
@@ -38,27 +38,6 @@ then
    fi
 fi
 
-
-verify_cksum() {
-  digestalgo=""
-  case ${#1} in
-        32) digestalgo="md5sum" ;;
-        40) digestalgo="sha1sum" ;;
-        56) digestalgo="sha224sum" ;;
-        64) digestalgo="sha256sum" ;;
-        96) digestalgo="sha384sum" ;;
-        128) digestalgo="sha512sum" ;;
-        *) echo "Please provide valid cheksum" ; exit 3 ;;
-  esac
-  echo  "$1  $2" | $digestalgo  -c --status
-  #printf "$1\t$2" | $digestalgo  -c --status
-  if [ $? -gt 0 ] 
-  then
-    printf "Checksum failed, not proceeding with install\n"
-    exit 3
-  fi
-}
-
 untar() {
   local ft=$(file $1| awk -F" " '{print $2}')
   local basedir=$(dirname $1)
@@ -167,7 +146,6 @@ do
 		volimg="$OPTARG"
 		;;
   s)	sflag=1
-		sflag=1
 		;;
   c)	cflag=1
 		snapshotName="$OPTARG"

diff --git a/scripts/storage/secondary/createtmplt.sh b/scripts/storage/secondary/createtmplt.sh
@@ -22,7 +22,7 @@
 # createtmplt.sh -- install a template
 
 usage() {
-  printf "Usage: %s: -t <template-fs> -n <templatename> -f <root disk file> -c <md5 cksum> -d <descr> -h  [-u] [-v]\n" $(basename $0) >&2
+  printf "Usage: %s: -t <template-fs> -n <templatename> -f <root disk file> -d <descr> -h  [-u] [-v]\n" $(basename $0) >&2
 }
 
 
@@ -39,26 +39,6 @@ rollback_if_needed() {
 fi
 }
 
-verify_cksum() {
-  digestalgo=""
-  case ${#1} in
-        32) digestalgo="md5sum" ;;
-        40) digestalgo="sha1sum" ;;
-        56) digestalgo="sha224sum" ;;
-        64) digestalgo="sha256sum" ;;
-        96) digestalgo="sha384sum" ;;
-        128) digestalgo="sha512sum" ;;
-        *) echo "Please provide valid cheksum" ; exit 3 ;;
-  esac
-  echo  "$1  $2" | $digestalgo  -c --status
-  #printf "$1\t$2" | $digestalgo  -c --status
-  if [ $? -gt 0 ] 
-  then
-    printf "Checksum failed, not proceeding with install\n"
-    exit 3
-  fi
-}
-
 untar() {
   local ft=$(file $1| awk -F" " '{print $2}')
   case $ft in
@@ -138,9 +118,8 @@ hflag=
 hvm=false
 cleanup=false
 dflag=
-cflag=
 
-while getopts 'vuht:n:f:s:c:d:S:' OPTION
+while getopts 'vuht:n:f:s:d:S:' OPTION
 do
   case $OPTION in
   t)	tflag=1
@@ -154,9 +133,6 @@ do
 		;;
   s)	sflag=1
 		;;
-  c)	cflag=1
-		cksum="$OPTARG"
-		;;
   d)	dflag=1
 		descr="$OPTARG"
 		;;
@@ -200,10 +176,6 @@ then
   exit 3
 fi
 
-if [ -n "$cksum" ]
-then
-  verify_cksum $cksum $tmpltimg
-fi
 [ -n "$verbose" ] && is_compressed $tmpltimg
 tmpltimg2=$(uncompress $tmpltimg)
 rollback_if_needed $tmpltfs $? "failed to uncompress $tmpltimg\n"
@@ -236,6 +208,8 @@ echo -n "" > /$tmpltfs/template.properties
 today=$(date '+%m_%d_%Y')
 echo "filename=$tmpltname" > /$tmpltfs/template.properties
 echo "description=$descr" >> /$tmpltfs/template.properties
+# we need to rethink this property as it might get changed after download due to decompression
+# option is to recalcutate it here
 echo "checksum=$cksum" >> /$tmpltfs/template.properties
 echo "hvm=$hvm" >> /$tmpltfs/template.properties
 echo "size=$imgsize" >> /$tmpltfs/template.properties

diff --git a/scripts/storage/secondary/createvolume.sh b/scripts/storage/secondary/createvolume.sh
@@ -41,6 +41,7 @@ fi
 
 verify_cksum() {
   digestalgo=""
+# NOTE this will only work with 0-padded checksums
   case ${#1} in
         32) digestalgo="md5sum" ;;
         40) digestalgo="sha1sum" ;;

diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -673,9 +673,9 @@ public VMTemplateStoragePoolVO prepareTemplateForCreate(VMTemplateVO templ, Stor
     }
 
     @Override
-    public String getChecksum(DataStore store, String templatePath) {
+    public String getChecksum(DataStore store, String templatePath, String algorithm) {
         EndPoint ep = _epSelector.select(store);
-        ComputeChecksumCommand cmd = new ComputeChecksumCommand(store.getTO(), templatePath);
+        ComputeChecksumCommand cmd = new ComputeChecksumCommand(store.getTO(), templatePath, algorithm);
         Answer answer = null;
         if (ep == null) {
             String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";