govc: Add checksum validation to govc import.ova

This patch enhances govc import.ova command to validate the checksum
of an uploaded file (e.g. .vmdk file in .ova) - as computed by the
remote server with the checksum listed in the ova manifest (.mf file
in .ova). govc import.ova takes a new optional flag to indicate
strict checksum validation.
(This govc enhancement needed a small tweak in nfc/lease.go code
to actually return manifest data in GetManifest() API.)

This patch also modifies simulator to compute checksum of uploaded
file and return that metadata through HttpNfcLeaseGetManifest() call.

govc ova import test now has a new sub-test to test for ova with
valid checksum and ova with invalid checksum.

Closes: vmware#1978

govc/USAGE.md

@@ -3097,6 +3097,7 @@ Options:
   -ds=                   Datastore [GOVC_DATASTORE]
   -folder=               Inventory folder [GOVC_FOLDER]
   -host=                 Host system [GOVC_HOST]
+  -m=false               Verify checksum of uploaded files against manifest (.mf)
   -name=                 Name to use for new entity
   -options=              Options spec file path for VM deployment
   -pool=                 Resource pool [GOVC_RESOURCE_POOL]
@@ -3111,6 +3112,7 @@ Options:
   -ds=                   Datastore [GOVC_DATASTORE]
   -folder=               Inventory folder [GOVC_FOLDER]
   -host=                 Host system [GOVC_HOST]
+  -m=false               Verify checksum of uploaded files against manifest (.mf)
   -name=                 Name to use for new entity
   -options=              Options spec file path for VM deployment
   -pool=                 Resource pool [GOVC_RESOURCE_POOL]

govc/importx/archive.go

@@ -32,6 +32,7 @@ import (
 	"strings"
 
 	"github.com/vmware/govmomi/ovf"
+	"github.com/vmware/govmomi/vapi/library"
 	"github.com/vmware/govmomi/vim25"
 	"github.com/vmware/govmomi/vim25/soap"
 )
@@ -40,6 +41,8 @@ import (
 // only encapsulates some common archive related functionality.
 type ArchiveFlag struct {
 	Archive
+
+	manifest map[string]*library.Checksum
 }
 
 func newArchiveFlag(ctx context.Context) (*ArchiveFlag, context.Context) {
@@ -72,6 +75,22 @@ func (f *ArchiveFlag) ReadEnvelope(data []byte) (*ovf.Envelope, error) {
 	return e, nil
 }
 
+func (f *ArchiveFlag) readManifest(fpath string) error {
+	base := filepath.Base(fpath)
+	ext := filepath.Ext(base)
+	mfName := strings.Replace(base, ext, ".mf", 1)
+
+	mf, _, err := f.Open(mfName)
+	if err != nil {
+		msg := fmt.Sprintf("manifest %q: %s", mf, err)
+		fmt.Fprintln(os.Stderr, msg)
+		return errors.New(msg)
+	}
+	f.manifest, err = library.ReadManifest(mf)
+	_ = mf.Close()
+	return err
+}
+
 type Archive interface {
 	Open(string) (io.ReadCloser, int64, error)
 }

govc/importx/ovf.go

@@ -22,6 +22,7 @@ import (
 	"flag"
 	"fmt"
 	"path"
+	"strings"
 
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/govc/cli"
@@ -44,7 +45,8 @@ type ovfx struct {
 	*ArchiveFlag
 	*OptionsFlag
 
-	Name string
+	Name           string
+	VerifyManifest bool
 
 	Client       *vim25.Client
 	Datacenter   *object.Datacenter
@@ -74,6 +76,7 @@ func (cmd *ovfx) Register(ctx context.Context, f *flag.FlagSet) {
 	cmd.OptionsFlag.Register(ctx, f)
 
 	f.StringVar(&cmd.Name, "name", "", "Name to use for new entity")
+	f.BoolVar(&cmd.VerifyManifest, "m", false, "Verify checksum of uploaded files against manifest (.mf)")
 }
 
 func (cmd *ovfx) Process(ctx context.Context) error {
@@ -315,6 +318,13 @@ func (cmd *ovfx) Import(fpath string) (*types.ManagedObjectReference, error) {
 		}
 	}
 
+	if cmd.VerifyManifest {
+		err = cmd.readManifest(fpath)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	lease, err := cmd.ResourcePool.ImportVApp(ctx, spec.ImportSpec, folder, host)
 	if err != nil {
 		return nil, err
@@ -355,5 +365,74 @@ func (cmd *ovfx) Upload(ctx context.Context, lease *nfc.Lease, item nfc.FileItem
 		Progress:      logger,
 	}
 
-	return lease.Upload(ctx, item, f, opts)
+	err = lease.Upload(ctx, item, f, opts)
+	if err != nil {
+		return err
+	}
+
+	if cmd.VerifyManifest {
+		mapImportKeyToKey := func(urls []types.HttpNfcLeaseDeviceUrl, importKey string) string {
+			for _, url := range urls {
+				if url.ImportKey == importKey {
+					return url.Key
+				}
+			}
+			return ""
+		}
+		leaseInfo, err := lease.Wait(ctx, nil)
+		if err != nil {
+			return err
+		}
+		return cmd.validateChecksum(ctx, lease, file, mapImportKeyToKey(leaseInfo.DeviceUrl, item.DeviceId))
+	}
+	return nil
+}
+
+func (cmd *ovfx) validateChecksum(ctx context.Context, lease *nfc.Lease, file string, key string) error {
+	sum, found := cmd.manifest[file]
+	if !found {
+		msg := fmt.Sprintf("missing checksum for %v in manifest file", file)
+		return errors.New(msg)
+	}
+	// Perform the checksum match eagerly, after each file upload, instead
+	// of after uploading all the files, to provide fail-fast behavior.
+	// (Trade-off here is multiple GetManifest() API calls to the server.)
+	manifests, err := lease.GetManifest(ctx)
+	if err != nil {
+		return err
+	}
+	for _, m := range manifests {
+		if m.Key == key {
+			// Compare server-side computed checksum of uploaded file
+			// against the client's manifest entry (assuming client's
+			// manifest has correct checksums - client doesn't compute
+			// checksum of the file before uploading).
+
+			// Try matching sha1 first (newer versions have moved to sha256).
+			if strings.ToUpper(sum.Algorithm) == "SHA1" {
+				if sum.Checksum != m.Sha1 {
+					msg := fmt.Sprintf("manifest checksum %v mismatch with uploaded checksum %v for file %v",
+						sum.Checksum, m.Sha1, file)
+					return errors.New(msg)
+				}
+				// Uploaded file checksum computed by server matches with local manifest entry.
+				return nil
+			}
+			// If not sha1, check for other types (in a separate field).
+			if !strings.EqualFold(sum.Algorithm, m.ChecksumType) {
+				msg := fmt.Sprintf("manifest checksum type %v mismatch with uploaded checksum type %v for file %v",
+					sum.Algorithm, m.ChecksumType, file)
+				return errors.New(msg)
+			}
+			if !strings.EqualFold(sum.Checksum, m.Checksum) {
+				msg := fmt.Sprintf("manifest checksum %v mismatch with uploaded checksum %v for file %v",
+					sum.Checksum, m.Checksum, file)
+				return errors.New(msg)
+			}
+			// Uploaded file checksum computed by server matches with local manifest entry.
+			return nil
+		}
+	}
+	msg := fmt.Sprintf("missing manifest entry on server for uploaded file %v (key %v), manifests=%#v", file, key, manifests)
+	return errors.New(msg)
 }

govc/test/images/update.sh

@@ -7,6 +7,12 @@ pushd $(dirname $0)
 base_url=https://github.com/dougm/packer-ttylinux/releases/download/16.1u2
 ttylinux="ttylinux-pc_i486-16.1"
 files="${ttylinux}.iso ${ttylinux}-live.ova ${ttylinux}.ova"
+tarbin="tar"
+if [ "$(uname)" = "Darwin" ]; then
+  # macOS BSD tar creates additional entries that govc tar archive reader
+  # chokes on (instead of ignoring). Use GNU tar instead.
+  tarbin="gtar"
+fi
 
 for name in $files ; do
   wget -qO $name $base_url/$name
@@ -15,6 +21,17 @@ done
 wget -qN https://github.com/icebreaker/floppybird/raw/master/build/floppybird.img
 
 # extract ova so we can also use the .vmdk and .ovf files directly
-tar -xvf ${ttylinux}.ova
+$tarbin -xvf ${ttylinux}.ova
+
+# create an ova with "bad" checksum in manifest by
+# modifying/replacing .mf in copy of ${ttylinux}.ova
+mkdir -p "$(pwd)/tmp"
+$tarbin xv -C "$(pwd)/tmp" -f ${ttylinux}.ova
+pushd "$(pwd)/tmp"
+sed 's/=.*$/= 5e82716003a1bff5b1d27bbd7d1e83addc881503/g' < ${ttylinux}.mf > ${ttylinux}-bad-checksum.mf
+mv ${ttylinux}-bad-checksum.mf ${ttylinux}.mf
+popd
+$tarbin cv -C "$(pwd)/tmp" -f ${ttylinux}-bad-checksum.ova .
+rm -fr "$(pwd)/tmp"
 
 popd

govc/test/import.bats

@@ -42,6 +42,16 @@ load test_helper
   assert_success
 }
 
+@test "import.ova with checksum validation" {
+  vcsim_env -app 1
+
+  run govc import.ova -name=bad-checksum-vm -m "$GOVC_IMAGES/$TTYLINUX_NAME-bad-checksum.ova"
+  assert_failure # vmdk checksum mismatch
+
+  run govc import.ova -name=good-checksum-vm -m "$GOVC_IMAGES/$TTYLINUX_NAME.ova"
+  assert_success
+}
+
 @test "import.ova with iso" {
   vcsim_env
 

nfc/lease.go

@@ -71,17 +71,17 @@ func (l *Lease) Complete(ctx context.Context) error {
 }
 
 // GetManifest wraps methods.GetManifest
-func (l *Lease) GetManifest(ctx context.Context) error {
+func (l *Lease) GetManifest(ctx context.Context) ([]types.HttpNfcLeaseManifestEntry, error) {
 	req := types.HttpNfcLeaseGetManifest{
 		This: l.Reference(),
 	}
 
-	_, err := methods.HttpNfcLeaseGetManifest(ctx, l.c, &req)
+	res, err := methods.HttpNfcLeaseGetManifest(ctx, l.c, &req)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	return nil
+	return res.Returnval, nil
 }
 
 // Progress wraps methods.Progress

simulator/http_nfc_lease.go

@@ -17,9 +17,11 @@ limitations under the License.
 package simulator
 
 import (
+	"crypto/sha1"
+	"encoding/hex"
 	"fmt"
+	"hash"
 	"io"
-	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
@@ -32,9 +34,15 @@ import (
 	"github.com/vmware/govmomi/vim25/types"
 )
 
+type metadata struct {
+	sha1 []byte
+	size int64
+}
+
 type HttpNfcLease struct {
 	mo.HttpNfcLease
-	files map[string]string
+	files    map[string]string
+	metadata map[string]metadata
 }
 
 var (
@@ -62,12 +70,12 @@ func ServeNFC(w http.ResponseWriter, r *http.Request) {
 	}
 
 	status := http.StatusOK
-	var dst io.Writer
+	var dst hash.Hash
 	var src io.ReadCloser
 
 	switch r.Method {
 	case http.MethodPut, http.MethodPost:
-		dst = ioutil.Discard
+		dst = sha1.New()
 		src = r.Body
 	case http.MethodGet:
 		f, err := os.Open(file)
@@ -82,6 +90,12 @@ func ServeNFC(w http.ResponseWriter, r *http.Request) {
 
 	n, err := io.Copy(dst, src)
 	_ = src.Close()
+	if dst != nil {
+		lease.metadata[name] = metadata{
+			sha1: dst.Sum(nil),
+			size: n,
+		}
+	}
 
 	msg := fmt.Sprintf("transferred %d bytes", n)
 	if err != nil {
@@ -101,7 +115,8 @@ func NewHttpNfcLease(ctx *Context, entity types.ManagedObjectReference) *HttpNfc
 			},
 			State: types.HttpNfcLeaseStateReady,
 		},
-		files: make(map[string]string),
+		files:    make(map[string]string),
+		metadata: make(map[string]metadata),
 	}
 
 	ctx.Session.Put(lease)
@@ -135,3 +150,28 @@ func (l *HttpNfcLease) HttpNfcLeaseProgress(ctx *Context, req *types.HttpNfcLeas
 		Res: new(types.HttpNfcLeaseProgressResponse),
 	}
 }
+
+func (l *HttpNfcLease) getDeviceKey(name string) string {
+	for _, devUrl := range l.Info.DeviceUrl {
+		if name == devUrl.TargetId {
+			return devUrl.Key
+		}
+	}
+	return "unknown"
+}
+
+func (l *HttpNfcLease) HttpNfcLeaseGetManifest(ctx *Context, req *types.HttpNfcLeaseGetManifest) soap.HasFault {
+	entries := []types.HttpNfcLeaseManifestEntry{}
+	for name, md := range l.metadata {
+		entries = append(entries, types.HttpNfcLeaseManifestEntry{
+			Key:  l.getDeviceKey(name),
+			Sha1: hex.EncodeToString(md.sha1),
+			Size: md.size,
+		})
+	}
+	return &methods.HttpNfcLeaseGetManifestBody{
+		Res: &types.HttpNfcLeaseGetManifestResponse{
+			Returnval: entries,
+		},
+	}
+}