From be33a205956c165dc559fda4e92ba790b5586a27 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Sun, 23 Jun 2024 16:51:21 -0700 Subject: [PATCH] fix: properly xml marshal byte array fields Go's encoding/xml package and vCenter marshal '[]byte' differently: - Go encodes the entire array in a single xml element, for example: hello - vCenter encodes each byte of the array in its own xml element, example with same data as above: 104 101 108 108 111 This behavior is hardwired, see the xml/encoding source for the handful of []byte special cases along the lines of: if reflect.Slice && slice element type == reflect.Uint8 While govmomi maintains a fork of Go's xml/encoding package, we prefer to further diverge. Proposed solution is to use a 'ByteArray' type that implements xml marshaling as vCenter does, but otherwise behaves just as '[]byte' does. Commits that follow enhance vcsim and govc support around various []byte fields. Fixes #1977 Fixes #3469 --- gen/gen_from_vmodl.rb | 3 ++ gen/vim_wsdl.rb | 4 +++ govc/object/collect.go | 18 +++++++--- govc/test/host.bats | 11 ++++++ govc/test/object.bats | 22 ++++++++++++ object/host_system_test.go | 34 +++++++++++++++++-- simulator/customization_spec_manager.go | 17 +++++++--- simulator/esx/host_config_info.go | 7 ++-- simulator/esx/host_storage_device_info.go | 14 ++++---- simulator/property_collector.go | 4 +++ vim25/mo/mo.go | 4 +-- vim25/types/helpers.go | 41 +++++++++++++++++++++-- vim25/types/helpers_test.go | 26 ++++++++++++-- vim25/types/types.go | 20 +++++------ 14 files changed, 190 insertions(+), 35 deletions(-) diff --git a/gen/gen_from_vmodl.rb b/gen/gen_from_vmodl.rb index 6f7609be4..ccfc31568 100644 --- a/gen/gen_from_vmodl.rb +++ b/gen/gen_from_vmodl.rb @@ -86,6 +86,9 @@ def var_type when "dateTime" type ="time.Time" when "byte" + if slice? + return "types.ByteArray" + end when "double" type ="float64" when "float" diff --git a/gen/vim_wsdl.rb b/gen/vim_wsdl.rb index daf631948..5e1713cca 100644 --- a/gen/vim_wsdl.rb +++ b/gen/vim_wsdl.rb @@ -321,6 +321,10 @@ def var_type self.need_omitempty = false end when "byte" + if slice? + prefix = "" + t = "#{pkg}ByteArray" + end when "double" t = "float64" when "float" diff --git a/govc/object/collect.go b/govc/object/collect.go index 972f1f337..48936dfc5 100644 --- a/govc/object/collect.go +++ b/govc/object/collect.go @@ -18,6 +18,7 @@ package object import ( "context" + "encoding/base64" "encoding/json" "flag" "fmt" @@ -111,8 +112,8 @@ Examples: govc object.collect -R create-filter-request.xml -O # convert filter to Go code govc object.collect -s vm/my-vm summary.runtime.host | xargs govc ls -L # inventory path of VM's host govc object.collect -dump -o "network/VM Network" # output Managed Object structure as Go code - govc object.collect -json $vm config | \ # use -json + jq to search array elements - jq -r '.[] | select(.val.hardware.device[].macAddress == "00:0c:29:0c:73:c0") | .val.name'`, atable) + govc object.collect -json -s $vm config | \ # use -json + jq to search array elements + jq -r 'select(.hardware.device[].macAddress == "00:50:56:99:c4:27") | .name'`, atable) } var stringer = reflect.TypeOf((*fmt.Stringer)(nil)).Elem() @@ -123,11 +124,11 @@ type change struct { } func (pc *change) MarshalJSON() ([]byte, error) { - if len(pc.cmd.kind) == 0 { + if len(pc.cmd.kind) == 0 && !pc.cmd.simple { return json.Marshal(pc.Update.ChangeSet) } - return json.Marshal(pc.Update) + return json.Marshal(pc.Dump()) } func (pc *change) output(name string, rval reflect.Value, rtype reflect.Type) { @@ -154,6 +155,15 @@ func (pc *change) output(name string, rval reflect.Value, rtype reflect.Type) { etype := rtype.Elem() + if etype.Kind() == reflect.Uint8 { + if v, ok := rval.Interface().(types.ByteArray); ok { + s = base64.StdEncoding.EncodeToString(v) // ArrayOfByte + } else { + s = fmt.Sprintf("%x", rval.Interface().([]byte)) // ArrayOfBase64Binary + } + break + } + if etype.Kind() != reflect.Interface && etype.Kind() != reflect.Struct || etype.Implements(stringer) { var val []string diff --git a/govc/test/host.bats b/govc/test/host.bats index 9364dc656..50c7cf932 100755 --- a/govc/test/host.bats +++ b/govc/test/host.bats @@ -159,6 +159,17 @@ load test_helper run govc host.storage.info -t hba assert_success + + names=$(govc host.storage.info -json | jq -r .storageDeviceInfo.scsiLun[].alternateName[].data) + # given data is hex encoded []byte and: + # [0] == encoding + # [1] == type + # [2] == ? + # [3] == length + # validate name is at least 2 char x 4 + for name in $names; do + [ "${#name}" -ge 8 ] + done } @test "host.options" { diff --git a/govc/test/object.bats b/govc/test/object.bats index 2da45061b..93133e8ad 100755 --- a/govc/test/object.bats +++ b/govc/test/object.bats @@ -338,6 +338,28 @@ load test_helper assert_success } +@test "object.collect bytes" { + vcsim_env + + host=$(govc find / -type h | head -1) + + # ArrayOfByte with PEM encoded cert + govc object.collect -s "$host" config.certificate | \ + base64 -d | openssl x509 -text + + # []byte field with PEM encoded cert + govc object.collect -s -json "$host" config | jq -r .certificate | \ + base64 -d | openssl x509 -text + + # ArrayOfByte with DER encoded cert + govc object.collect -s CustomizationSpecManager:CustomizationSpecManager encryptionKey | \ + base64 -d | openssl x509 -inform DER -text + + # []byte field with DER encoded cert + govc object.collect -o -json CustomizationSpecManager:CustomizationSpecManager | jq -r .encryptionKey | \ + base64 -d | openssl x509 -inform DER -text +} + @test "object.collect view" { vcsim_env -dc 2 -folder 1 diff --git a/object/host_system_test.go b/object/host_system_test.go index 31caf4c31..684a76b53 100644 --- a/object/host_system_test.go +++ b/object/host_system_test.go @@ -1,9 +1,12 @@ /* -Copyright (c) 2019 VMware, Inc. All Rights Reserved. +Copyright (c) 2019-2024 VMware, Inc. All Rights Reserved. + Licensed 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 + +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. @@ -14,12 +17,16 @@ limitations under the License. package object_test import ( + "bytes" "context" + "encoding/pem" "testing" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/simulator" + "github.com/vmware/govmomi/simulator/esx" "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/mo" ) func TestHostSystemManagementIPs(t *testing.T) { @@ -59,3 +66,26 @@ func TestHostSystemManagementIPs(t *testing.T) { } }) } + +func TestHostSystemConfig(t *testing.T) { + simulator.Test(func(ctx context.Context, c *vim25.Client) { + host, err := find.NewFinder(c).HostSystem(ctx, "DC0_C0_H0") + if err != nil { + t.Fatal(err) + } + + var props mo.HostSystem + if err := host.Properties(ctx, host.Reference(), []string{"config"}, &props); err != nil { + t.Fatal(err) + } + + if !bytes.Equal(props.Config.Certificate, esx.HostConfigInfo.Certificate) { + t.Errorf("certificate=%s", string(props.Config.Certificate)) + } + + b, _ := pem.Decode(props.Config.Certificate) + if b == nil { + t.Error("failed to parse certificate") + } + }) +} diff --git a/simulator/customization_spec_manager.go b/simulator/customization_spec_manager.go index b522680e5..4ef7a51ee 100644 --- a/simulator/customization_spec_manager.go +++ b/simulator/customization_spec_manager.go @@ -1,11 +1,11 @@ /* -Copyright (c) 2019 VMware, Inc. All Rights Reserved. +Copyright (c) 2019-2024 VMware, Inc. All Rights Reserved. Licensed 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 +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, @@ -17,10 +17,12 @@ limitations under the License. package simulator import ( + "encoding/pem" "fmt" "sync/atomic" "time" + "github.com/vmware/govmomi/simulator/internal" "github.com/vmware/govmomi/vim25/methods" "github.com/vmware/govmomi/vim25/mo" "github.com/vmware/govmomi/vim25/soap" @@ -169,7 +171,7 @@ var DefaultCustomizationSpec = []types.CustomizationSpecItem{ }, }, }, - EncryptionKey: []uint8{0x30}, + EncryptionKey: nil, }, }, { @@ -236,7 +238,7 @@ var DefaultCustomizationSpec = []types.CustomizationSpecItem{ }, }, }, - EncryptionKey: []uint8{0x30}, + EncryptionKey: nil, }, }, } @@ -249,6 +251,13 @@ type CustomizationSpecManager struct { func (m *CustomizationSpecManager) init(r *Registry) { m.items = DefaultCustomizationSpec + + // Real VC is different DN, X509v3 extensions, etc. + // This is still useful for testing []byte of DER encoded cert over SOAP + if len(m.EncryptionKey) == 0 { + block, _ := pem.Decode(internal.LocalhostCert) + m.EncryptionKey = block.Bytes + } } var customizeNameCounter uint64 diff --git a/simulator/esx/host_config_info.go b/simulator/esx/host_config_info.go index b98f4cf35..ced5a5046 100644 --- a/simulator/esx/host_config_info.go +++ b/simulator/esx/host_config_info.go @@ -16,7 +16,10 @@ limitations under the License. package esx -import "github.com/vmware/govmomi/vim25/types" +import ( + "github.com/vmware/govmomi/simulator/internal" + "github.com/vmware/govmomi/vim25/types" +) // HostConfigInfo is the default template for the HostSystem config property. // Capture method: @@ -998,7 +1001,7 @@ var HostConfigInfo = types.HostConfigInfo{ Ipmi: (*types.HostIpmiInfo)(nil), SslThumbprintInfo: (*types.HostSslThumbprintInfo)(nil), SslThumbprintData: nil, - Certificate: []uint8{0x31, 0x30}, + Certificate: internal.LocalhostCert, PciPassthruInfo: nil, AuthenticationManagerInfo: &types.HostAuthenticationManagerInfo{ AuthConfig: []types.BaseHostAuthenticationStoreInfo{ diff --git a/simulator/esx/host_storage_device_info.go b/simulator/esx/host_storage_device_info.go index 79033344f..43d1f2ecd 100644 --- a/simulator/esx/host_storage_device_info.go +++ b/simulator/esx/host_storage_device_info.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2017-2023 VMware, Inc. All Rights Reserved. +Copyright (c) 2017-2024 VMware, Inc. All Rights Reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -93,15 +93,15 @@ var HostStorageDeviceInfo = types.HostStorageDeviceInfo{ { Namespace: "GENERIC_VPD", NamespaceId: 0x5, - Data: []uint8{0x2d, 0x37, 0x39}, + Data: []uint8{0x0, 0x0, 0x0, 0x4, 0x0, 0xb0, 0xb1, 0xb2}, }, { Namespace: "GENERIC_VPD", NamespaceId: 0x5, - Data: []uint8{0x30}, + Data: []uint8{0x0, 0xb2, 0x0, 0x4, 0x1, 0x60, 0x0, 0x0}, }, }, - StandardInquiry: []uint8{0x30}, + StandardInquiry: []uint8{0x0, 0x0, 0x6, 0x2, 0x1f, 0x0, 0x0, 0x72}, QueueDepth: 0, OperationalState: []string{"ok"}, Capabilities: &types.ScsiLunCapabilities{}, @@ -143,15 +143,15 @@ var HostStorageDeviceInfo = types.HostStorageDeviceInfo{ { Namespace: "GENERIC_VPD", NamespaceId: 0x5, - Data: []uint8{0x2d, 0x37, 0x39}, + Data: []uint8{0x0, 0x0, 0x0, 0x4, 0x0, 0xb0, 0xb1, 0xb2}, }, { Namespace: "GENERIC_VPD", NamespaceId: 0x5, - Data: []uint8{0x30}, + Data: []uint8{0x0, 0xb2, 0x0, 0x4, 0x1, 0x60, 0x0, 0x0}, }, }, - StandardInquiry: []uint8{0x30}, + StandardInquiry: []uint8{0x0, 0x0, 0x6, 0x2, 0x1f, 0x0, 0x0, 0x72}, QueueDepth: 1024, OperationalState: []string{"ok"}, Capabilities: &types.ScsiLunCapabilities{}, diff --git a/simulator/property_collector.go b/simulator/property_collector.go index b50fc9a91..13f3e5188 100644 --- a/simulator/property_collector.go +++ b/simulator/property_collector.go @@ -131,6 +131,10 @@ func wrapValue(rval reflect.Value, rtype reflect.Type) interface{} { pval = &types.ArrayOfByte{ Byte: v, } + case types.ByteArray: + pval = &types.ArrayOfByte{ + Byte: v, + } case []int16: pval = &types.ArrayOfShort{ Short: v, diff --git a/vim25/mo/mo.go b/vim25/mo/mo.go index fc6c96ff6..a3df2174e 100644 --- a/vim25/mo/mo.go +++ b/vim25/mo/mo.go @@ -5,7 +5,7 @@ Licensed 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 +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, @@ -210,7 +210,7 @@ type CustomizationSpecManager struct { Self types.ManagedObjectReference `json:"self"` Info []types.CustomizationSpecInfo `json:"info"` - EncryptionKey []byte `json:"encryptionKey"` + EncryptionKey types.ByteArray `json:"encryptionKey"` } func (m CustomizationSpecManager) Reference() types.ManagedObjectReference { diff --git a/vim25/types/helpers.go b/vim25/types/helpers.go index cbcdb637e..f34fceea2 100644 --- a/vim25/types/helpers.go +++ b/vim25/types/helpers.go @@ -1,11 +1,11 @@ /* -Copyright (c) 2015-2022 VMware, Inc. All Rights Reserved. +Copyright (c) 2015-2024 VMware, Inc. All Rights Reserved. Licensed 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 +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, @@ -17,10 +17,14 @@ limitations under the License. package types import ( + "io" "net/url" "reflect" + "strconv" "strings" "time" + + "github.com/vmware/govmomi/vim25/xml" ) func EnumValuesAsStrings[T ~string](enumValues []T) []string { @@ -316,6 +320,39 @@ func (ci VirtualMachineConfigInfo) ToConfigSpec() VirtualMachineConfigSpec { return cs } +type ByteArray []byte + +func (b ByteArray) MarshalXML(e *xml.Encoder, field xml.StartElement) error { + start := xml.StartElement{ + Name: field.Name, + } + for i := range b { + if err := e.EncodeElement(b[i], start); err != nil { + return err + } + } + return nil +} + +func (b *ByteArray) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + for { + t, err := d.Token() + if err == io.EOF { + break + } + + if c, ok := t.(xml.CharData); ok { + x, err := strconv.ParseInt(string(c), 10, 16) + if err != nil { + return err + } + *b = append(*b, byte(x)) + } + } + + return nil +} + func init() { // Known 6.5 issue where this event type is sent even though it is internal. // This workaround allows us to unmarshal and avoid NPEs. diff --git a/vim25/types/helpers_test.go b/vim25/types/helpers_test.go index 960c4256e..94991df7f 100644 --- a/vim25/types/helpers_test.go +++ b/vim25/types/helpers_test.go @@ -1,11 +1,11 @@ /* -Copyright (c) 2022 VMware, Inc. All Rights Reserved. +Copyright (c) 2022-2024 VMware, Inc. All Rights Reserved. Licensed 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 +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, @@ -17,6 +17,7 @@ limitations under the License. package types import ( + "bytes" "testing" "github.com/vmware/govmomi/vim25/xml" @@ -306,3 +307,24 @@ func TestVirtualMachineConfigInfoToConfigSpec(t *testing.T) { }) } } + +func TestArrayOfByte(t *testing.T) { + in := &ArrayOfByte{ + Byte: []byte("xmhell"), + } + + res, err := xml.Marshal(in) + if err != nil { + t.Fatal(err) + } + + var out ArrayOfByte + if err := xml.Unmarshal(res, &out); err != nil { + t.Logf("%s", string(res)) + t.Fatal(err) + } + + if !bytes.Equal(in.Byte, out.Byte) { + t.Errorf("out=%#v", out.Byte) + } +} diff --git a/vim25/types/types.go b/vim25/types/types.go index a51550331..0b5c23c64 100644 --- a/vim25/types/types.go +++ b/vim25/types/types.go @@ -5,7 +5,7 @@ Licensed 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 +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, @@ -2431,7 +2431,7 @@ func init() { // A boxed array of `PrimitiveByte`. To be used in `Any` placeholders. type ArrayOfByte struct { - Byte []byte `xml:"byte,omitempty" json:"_value"` + Byte ByteArray `xml:"byte,omitempty" json:"_value"` } func init() { @@ -18385,7 +18385,7 @@ type CustomizationSpec struct { // Both the client and the server can use this to determine if // stored passwords can be decrypted by the server or if the passwords need to be // re-entered and re-encrypted before the specification can be used. - EncryptionKey []byte `xml:"encryptionKey,omitempty" json:"encryptionKey,omitempty"` + EncryptionKey ByteArray `xml:"encryptionKey,omitempty" json:"encryptionKey,omitempty"` } func init() { @@ -34873,7 +34873,7 @@ type HostConfigInfo struct { // SSL Thumbprints registered on this host. SslThumbprintData []HostSslThumbprintInfo `xml:"sslThumbprintData,omitempty" json:"sslThumbprintData,omitempty"` // Full Host Certificate in PEM format, if known - Certificate []byte `xml:"certificate,omitempty" json:"certificate,omitempty"` + Certificate ByteArray `xml:"certificate,omitempty" json:"certificate,omitempty"` // PCI passthrough information. PciPassthruInfo []BaseHostPciPassthruInfo `xml:"pciPassthruInfo,omitempty,typeattr" json:"pciPassthruInfo,omitempty"` // Current authentication configuration. @@ -36351,7 +36351,7 @@ type HostDigestInfo struct { DigestMethod string `xml:"digestMethod" json:"digestMethod"` // The variable length byte array containing the digest value calculated by // the specified digestMethod. - DigestValue []byte `xml:"digestValue" json:"digestValue"` + DigestValue ByteArray `xml:"digestValue" json:"digestValue"` // The name of the object from which this digest value is calcaulated. ObjectName string `xml:"objectName,omitempty" json:"objectName,omitempty"` } @@ -44877,7 +44877,7 @@ type HostSubSpecification struct { // Time at which the host sub specification was created. CreatedTime time.Time `xml:"createdTime" json:"createdTime"` // The host sub specification data - Data []byte `xml:"data,omitempty" json:"data,omitempty"` + Data ByteArray `xml:"data,omitempty" json:"data,omitempty"` // The host sub specification data in Binary for wire efficiency. BinaryData []byte `xml:"binaryData,omitempty" json:"binaryData,omitempty"` } @@ -45374,7 +45374,7 @@ type HostTpmEventDetails struct { DynamicData // Value of the Platform Configuration Register (PCR) for this event. - DataHash []byte `xml:"dataHash" json:"dataHash"` + DataHash ByteArray `xml:"dataHash" json:"dataHash"` // Method in which the digest hash is calculated. // // The set of possible @@ -45434,7 +45434,7 @@ type HostTpmOptionEventDetails struct { // This array exposes the raw contents of the settings file (or files) that were // passed to kernel during the boot up process, and, therefore, should be treated // accordingly. - BootOptions []byte `xml:"bootOptions,omitempty" json:"bootOptions,omitempty"` + BootOptions ByteArray `xml:"bootOptions,omitempty" json:"bootOptions,omitempty"` } func init() { @@ -71958,7 +71958,7 @@ type ScsiLun struct { // For a SCSI-3 compliant device this property is derived from the // standard inquiry data. For devices that are not SCSI-3 compliant this // property is not defined. - StandardInquiry []byte `xml:"standardInquiry,omitempty" json:"standardInquiry,omitempty"` + StandardInquiry ByteArray `xml:"standardInquiry,omitempty" json:"standardInquiry,omitempty"` // The queue depth of SCSI device. QueueDepth int32 `xml:"queueDepth,omitempty" json:"queueDepth,omitempty"` // The operational states of the LUN. @@ -72078,7 +72078,7 @@ type ScsiLunDurableName struct { // along with the payload for data obtained from page 83h, and is the // payload for data obtained from page 80h of the Vital Product Data // (VPD). - Data []byte `xml:"data,omitempty" json:"data,omitempty"` + Data ByteArray `xml:"data,omitempty" json:"data,omitempty"` } func init() {