Libguestfs - Oct 2016 - [PATCH v2 2/2] v2v: ova: support SHA256 hashes in manifest

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

v1 -> v2: rebased on to master to make use of the Checksums module


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

 v2v/input_ova.ml | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 5731a45..44e41ed 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,11 @@ 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. Line is
\"%s\"")
+              (String.replace line "\r" "")
+          ;
+          loop ()
         in
         (try loop () with End_of_file -> ());
         close_in chan
-- 
2.10.0

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

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/input_ova.ml | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 44e41ed..3c1a296 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. Line is
\"%s\"")
-- 
2.10.0

On Wed, Oct 05, 2016 at 12:51:05PM +0200, Tomáš Golembiovský
wrote:
> 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 | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> index 5731a45..44e41ed 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;
Don't capitalize the message.
>          let mf_folder = Filename.dirname mf in
>          let chan = open_in mf in
>          let rec loop () > @@ -149,6 +150,11 @@ 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. Line
is \"%s\"")
> +              (String.replace line "\r" "")
Use this instead:

     warning (f_"unable to parse line from manifest file: %S") line

Note %S _is_ supposed to be a capital letter.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Wed, Oct 05, 2016 at 12:51:06PM +0200, Tomáš Golembiovský
wrote:
> The OVF standard allows the use of SHA256 hashes in the manifest file.
> Adding support for this.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/input_ova.ml | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> index 44e41ed..3c1a296 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. Line
is \"%s\"")
This one looks OK, ACK for this patch.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html