Libguestfs - Oct 2016 - [PATCH v5 0/2] Improve OVA manifest parsing

This series fixes and enhances parsing of the OVA manifest file.
The changes are:

- Added mandatory space to the regexp
- Process all lines in the file, not just one
- Warn on improperly formated lines
- Support SHA256 hashes

v4 -> v5:
- fix tests
- change one test to include the SHA256 checksum

Tomáš Golembiovský (2):
  v2v: ova: fix checking of the manifest file
  v2v: ova: support SHA256 hashes in manifest

 test-data/guestfs-hashsums.sh    | 13 +++++++++++++
 v2v/input_ova.ml                 | 18 ++++++++++++------
 v2v/test-v2v-i-ova-formats.sh    |  2 +-
 v2v/test-v2v-i-ova-gz.sh         |  2 +-
 v2v/test-v2v-i-ova-subfolders.sh |  2 +-
 v2v/test-v2v-i-ova-two-disks.sh  |  4 ++--
 v2v/test-v2v-i-ova.sh            |  4 ++--
 7 files changed, 32 insertions(+), 13 deletions(-)

-- 
2.10.0

The regular expression for parsing the manifest line was wrong. There is
a mandatory space between '=' and the hash.

Another problem was that only the first line of the manifest file was
actually processed.

Also added some debugging info and warning to catch problems with
parsing.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/input_ova.ml                 | 7 ++++++-
 v2v/test-v2v-i-ova-formats.sh    | 2 +-
 v2v/test-v2v-i-ova-gz.sh         | 2 +-
 v2v/test-v2v-i-ova-subfolders.sh | 2 +-
 v2v/test-v2v-i-ova-two-disks.sh  | 4 ++--
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 5731a45..0d04659 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -133,9 +133,10 @@ object
 
     (* Read any .mf (manifest) files and verify sha1. *)
     let mf = find_files exploded ".mf" in
-    let rex = Str.regexp "SHA1(\\(.*\\))=\\([0-9a-fA-F]+\\)\r?" in
+    let rex = Str.regexp "SHA1(\\(.*\\))= \\([0-9a-fA-F]+\\)\r?" in
     List.iter (
       fun mf ->
+        debug "processing manifest %s" mf;
         let mf_folder = Filename.dirname mf in
         let chan = open_in mf in
         let rec loop () @@ -149,6 +150,10 @@ object
               error (f_"checksum of disk %s does not match manifest %s
(actual sha1(%s) = %s, expected sha1 (%s) = %s)")
                 disk mf disk actual disk expected;
           )
+          else
+            warning (f_"unable to parse line from manifest file: %S")
line
+          ;
+          loop ()
         in
         (try loop () with End_of_file -> ());
         close_in chan
diff --git a/v2v/test-v2v-i-ova-formats.sh b/v2v/test-v2v-i-ova-formats.sh
index 3e2923d..d113994 100755
--- a/v2v/test-v2v-i-ova-formats.sh
+++ b/v2v/test-v2v-i-ova-formats.sh
@@ -58,7 +58,7 @@ pushd $d
 # conversion, so the contents of the disks doesn't matter.
 truncate -s 10k disk1.vmdk
 sha=`do_sha1 disk1.vmdk`
-echo -e "SHA1(disk1.vmdk)=$sha\r" > disk1.mf
+echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf
 
 for format in $formats; do
     case "$format" in
diff --git a/v2v/test-v2v-i-ova-gz.sh b/v2v/test-v2v-i-ova-gz.sh
index 086ff98..a38e1b4 100755
--- a/v2v/test-v2v-i-ova-gz.sh
+++ b/v2v/test-v2v-i-ova-gz.sh
@@ -45,7 +45,7 @@ pushd $d
 truncate -s 10k disk1.vmdk
 gzip disk1.vmdk
 sha=`do_sha1 disk1.vmdk.gz`
-echo -e "SHA1(disk1.vmdk.gz)=$sha\r" > disk1.mf
+echo -e "SHA1(disk1.vmdk.gz)= $sha\r" > disk1.mf
 
 tar -cf test.ova ../test-v2v-i-ova-gz.ovf disk1.vmdk.gz disk1.mf
 popd
diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova-subfolders.sh
index a590fcc..c49f7cb 100755
--- a/v2v/test-v2v-i-ova-subfolders.sh
+++ b/v2v/test-v2v-i-ova-subfolders.sh
@@ -46,7 +46,7 @@ pushd $d/subfolder
 
 truncate -s 10k disk1.vmdk
 sha=`do_sha1 disk1.vmdk`
-echo -e "SHA1(disk1.vmdk)=$sha\r" > disk1.mf
+echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf
 
 cd ..
 tar -cf test.ova subfolder
diff --git a/v2v/test-v2v-i-ova-two-disks.sh b/v2v/test-v2v-i-ova-two-disks.sh
index 4e7f38b..aefd90e 100755
--- a/v2v/test-v2v-i-ova-two-disks.sh
+++ b/v2v/test-v2v-i-ova-two-disks.sh
@@ -47,10 +47,10 @@ pushd $d
 # conversion, so the contents of the disks doesn't matter.
 truncate -s 10k disk1.vmdk
 sha=`do_sha1 disk1.vmdk`
-echo -e "SHA1(disk1.vmdk)=$sha\r" > disk1.mf
+echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf
 truncate -s 100k disk2.vmdk
 sha=`do_sha1 disk2.vmdk`
-echo -e "SHA1(disk2.vmdk)=$sha\r" > disk2.mf
+echo -e "SHA1(disk2.vmdk)= $sha\r" > disk2.mf
 
 tar -cf test.ova ../test-v2v-i-ova-two-disks.ovf disk1.vmdk disk1.mf disk2.vmdk
disk2.mf
 popd
-- 
2.10.0

The OVF standard allows the use of SHA256 hashes in the manifest file.
Adding support for this.

One of the tests was updated to put SHA256 into manifest file.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 test-data/guestfs-hashsums.sh | 13 +++++++++++++
 v2v/input_ova.ml              | 13 +++++++------
 v2v/test-v2v-i-ova.sh         |  4 ++--
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/test-data/guestfs-hashsums.sh b/test-data/guestfs-hashsums.sh
index 96e1238..86a5aaf 100755
--- a/test-data/guestfs-hashsums.sh
+++ b/test-data/guestfs-hashsums.sh
@@ -41,3 +41,16 @@ do_sha1 ()
       ;;
   esac
 }
+
+do_sha256 ()
+{
+  case "$(uname)" in
+    Linux)
+      sha256sum "$1" | awk '{print $1}'
+      ;;
+    *)
+      echo "$0: unknown method to calculate SHA256 of file on
$(uname)"
+      exit 1
+      ;;
+  esac
+}
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 0d04659..0eb37d4 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -133,7 +133,7 @@ object
 
     (* Read any .mf (manifest) files and verify sha1. *)
     let mf = find_files exploded ".mf" in
-    let rex = Str.regexp "SHA1(\\(.*\\))= \\([0-9a-fA-F]+\\)\r?" in
+    let rex = Str.regexp "\\(SHA1\\|SHA256\\)(\\(.*\\))=
\\([0-9a-fA-F]+\\)\r?" in
     List.iter (
       fun mf ->
         debug "processing manifest %s" mf;
@@ -142,13 +142,14 @@ object
         let rec loop ()            let line = input_line chan in
           if Str.string_match rex line 0 then (
-            let disk = Str.matched_group 1 line in
-            let expected = Str.matched_group 2 line in
-            let csum = Checksums.SHA1 expected in
+            let mode = Str.matched_group 1 line in
+            let disk = Str.matched_group 2 line in
+            let expected = Str.matched_group 3 line in
+            let csum = Checksums.of_string mode expected in
             try Checksums.verify_checksum csum (mf_folder // disk)
             with Checksums.Mismatched_checksum (_, actual) ->
-              error (f_"checksum of disk %s does not match manifest %s
(actual sha1(%s) = %s, expected sha1 (%s) = %s)")
-                disk mf disk actual disk expected;
+              error (f_"checksum of disk %s does not match manifest %s
(actual %s(%s) = %s, expected %s(%s) = %s)")
+                disk mf mode disk actual mode disk expected;
           )
           else
             warning (f_"unable to parse line from manifest file: %S")
line
diff --git a/v2v/test-v2v-i-ova.sh b/v2v/test-v2v-i-ova.sh
index ea4f3df..716cd33 100755
--- a/v2v/test-v2v-i-ova.sh
+++ b/v2v/test-v2v-i-ova.sh
@@ -57,8 +57,8 @@ qemu-img convert $f -O vmdk $d/$vmdk
 cp $ovf $d/$ovf
 sha1=`do_sha1 $d/$ovf`
 echo "SHA1($ovf)= $sha1" > $d/$mf
-sha1=`do_sha1 $d/$vmdk`
-echo "SHA1($vmdk)= $sha1" >> $d/$mf
+sha256=`do_sha256 $d/$vmdk`
+echo "SHA256($vmdk)= $sha256" >> $d/$mf
 
 pushd .
 cd $d
-- 
2.10.0

On Friday, 7 October 2016 14:12:20 CEST Tomáš Golembiovský
wrote:
> 
> This series fixes and enhances parsing of the OVA manifest file.
> The changes are:
> 
> - Added mandatory space to the regexp
> - Process all lines in the file, not just one
> - Warn on improperly formated lines
> - Support SHA256 hashes
LGTM -- pushed.

Thanks!
-- 
Pino Toscano