Merge pull request #137 from rancher-sandbox/validate-networks

Add validation to network.vde[*].name fields

Author: AkihiroSuda

pkg/cidata/cidata.go

@@ -15,7 +15,7 @@ import (
 	"github.com/AkihiroSuda/lima/pkg/iso9660util"
 	"github.com/AkihiroSuda/lima/pkg/limayaml"
 	"github.com/AkihiroSuda/lima/pkg/localpathutil"
-	"github.com/AkihiroSuda/lima/pkg/qemu"
+	"github.com/AkihiroSuda/lima/pkg/qemu/qemuconst"
 	"github.com/AkihiroSuda/lima/pkg/sshutil"
 	"github.com/AkihiroSuda/lima/pkg/store/filenames"
 	"github.com/opencontainers/go-digest"
@@ -72,7 +72,8 @@ func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML) error {
 		args.Mounts = append(args.Mounts, expanded)
 	}
 
-	args.Networks = append(args.Networks, Network{MACAddress: qemu.SlirpMACAddress, Name: "eth0"})
+	slirpMACAddress := limayaml.MACAddress(instDir)
+	args.Networks = append(args.Networks, Network{MACAddress: slirpMACAddress, Name: qemuconst.SlirpNICName})
 	for _, vde := range y.Network.VDE {
 		args.Networks = append(args.Networks, Network{MACAddress: vde.MACAddress, Name: vde.Name})
 	}

pkg/limayaml/defaults.go

@@ -10,6 +10,15 @@ import (
 	"github.com/AkihiroSuda/lima/pkg/guestagent/api"
 )
 
+func MACAddress(uniqueID string) string {
+	// TODO: combine the uniqueID with the host machineID to create a globally unique hash
+	sha := sha256.Sum256([]byte(uniqueID))
+	// According to https://gitlab.com/wireshark/wireshark/-/blob/master/manuf
+	// no well-known MAC addresses start with 0x22.
+	hw := append(net.HardwareAddr{0x22}, sha[0:5]...)
+	return hw.String()
+}
+
 func FillDefault(y *LimaYAML, filePath string) {
 	y.Arch = resolveArch(y.Arch)
 	for i := range y.Images {
@@ -62,12 +71,7 @@ func FillDefault(y *LimaYAML, filePath string) {
 		vde := &y.Network.VDE[i]
 		if vde.MACAddress == "" {
 			// every interface in every limayaml file must get its own unique MAC address
-			uniqueID := fmt.Sprintf("%s#%d", filePath, i)
-			sha := sha256.Sum256([]byte(uniqueID))
-			// According to https://gitlab.com/wireshark/wireshark/-/blob/master/manuf
-			// no well-known MAC addresses start with 0x22.
-			hw := append(net.HardwareAddr{0x22}, sha[0:5]...)
-			vde.MACAddress = hw.String()
+			vde.MACAddress = MACAddress(fmt.Sprintf("%s#%d", filePath, i))
 		}
 		if vde.Name == "" {
 			vde.Name = "vde" + strconv.Itoa(i)

pkg/limayaml/validate.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/AkihiroSuda/lima/pkg/localpathutil"
+	"github.com/AkihiroSuda/lima/pkg/qemu/qemuconst"
 	"github.com/docker/go-units"
 	"github.com/pkg/errors"
 )
@@ -159,6 +160,7 @@ func Validate(y LimaYAML) error {
 		// Not validating that the various GuestPortRanges and HostPortRanges are not overlapping. Rules will be
 		// processed sequentially and the first matching rule for a guest port determines forwarding behavior.
 	}
+	networkName := make(map[string]int)
 	for i, vde := range y.Network.VDE {
 		field := fmt.Sprintf("network.vde[%d]", i)
 		if vde.URL == "" {
@@ -190,6 +192,20 @@ func Validate(y LimaYAML) error {
 				return errors.Errorf("field `%s.macAddress` must be a 48 bit (6 bytes) MAC address; actual length of %q is %d bytes", field, vde.MACAddress, len(hw))
 			}
 		}
+		// FillDefault() will make sure that vde.Name is not the empty string
+		if len(vde.Name) >= 16 {
+			return errors.Errorf("field `%s.be less than 16 bytes, but is %d bytes: %q", field, len(vde.Name), vde.Name)
+		}
+		if strings.ContainsAny(vde.Name, " \t\n/") {
+			return errors.Errorf("field `%s.be must not contain whitespace or slashes", field)
+		}
+		if vde.Name == qemuconst.SlirpNICName {
+			return errors.Errorf("field `%s.name` must not be set to %q because it is reserved for slirp", field, qemuconst.SlirpNICName)
+		}
+		if prev, ok := networkName[vde.Name]; ok {
+			return errors.Errorf("field `%s.name` value %q has already been used by field `network.vde[%d].name`", field, vde.Name, prev)
+		}
+		networkName[vde.Name] = i
 	}
 	return nil
 }

pkg/qemu/qemu.go

@@ -130,11 +130,6 @@ func appendArgsIfNoConflict(args []string, k, v string) []string {
 	}
 	return append(args, k, v)
 }
-
-const (
-	SlirpMACAddress = "22:11:11:11:11:11"
-)
-
 func Cmdline(cfg Config) (string, []string, error) {
 	y := cfg.LimaYAML
 	exe, args, err := getExe(y.Arch)
@@ -200,10 +195,10 @@ func Cmdline(cfg Config) (string, []string, error) {
 	// Network
 	// CIDR is intentionally hardcoded to 192.168.5.0/24, as each of QEMU has its own independent slirp network.
 	args = append(args, "-netdev", fmt.Sprintf("user,id=net0,net=192.168.5.0/24,hostfwd=tcp:127.0.0.1:%d-:22", y.SSH.LocalPort))
-	args = append(args, "-device", "virtio-net-pci,netdev=net0,mac="+SlirpMACAddress)
+	args = append(args, "-device", "virtio-net-pci,netdev=net0,mac="+limayaml.MACAddress(cfg.InstanceDir))
 	for i, vde := range y.Network.VDE {
-			args = append(args, "-netdev", fmt.Sprintf("vde,id=net%d,sock=%s", i+1, vde.URL))
-			args = append(args, "-device", fmt.Sprintf("virtio-net-pci,netdev=net%d,mac=%s", i+1, vde.MACAddress))
+		args = append(args, "-netdev", fmt.Sprintf("vde,id=net%d,sock=%s", i+1, vde.URL))
+		args = append(args, "-device", fmt.Sprintf("virtio-net-pci,netdev=net%d,mac=%s", i+1, vde.MACAddress))
 	}
 
 	// virtio-rng-pci accelerates starting up the OS, according to https://wiki.gentoo.org/wiki/QEMU/Options

pkg/qemu/qemuconst/qemuconst.go

@@ -0,0 +1,5 @@
+package qemuconst
+
+const (
+	SlirpNICName = "eth0"
+)