Skip to content

Commit

Permalink
baremetal: validate that macs are EUI-48 and unicast
Browse files Browse the repository at this point in the history
When users specify a custom MAC address for the provisioning and
baremetal bridges of the libvirt bootstrap host, libvirt complains
if the user does not give a unicast MAC address:

```
020-11-13 10:52:18 level=debug msg=2020/11/13 10:52:18 [ERROR]
module.bootstrap: eval: *terraform.EvalApplyPost, err: Error defining
libvirt domain: virError(Code=27, Domain=20, Message='XML error:
expected unicast mac address, found multicast '7D:CE:E3:29:35:6F'')
')
```

This adds a function that validates whether a MAC address is unicast. A
multicast MAC address is one where the most significant byte has a least
significant bit of `1`.

      Example 1: Multicast MAC
      ------------------------
      7D:CE:E3:29:35:6F
       ^--> most significant byte

      0x7D is 0b11111101
                       ^--> this is a multicast MAC

      Example 2: Unicast MAC
      ----------------------
      7A:CE:E3:29:35:6F
       ^--> most significant byte

      0x7A is 0b11111010
                       ^--> this is a unicast MAC

Additionally, the built-in ParseMAC in go net accepts EUI-64 and
Infiniband macs. This updates our validator to make sure we have
EUI-48 (6-octet) MAC address as well.
  • Loading branch information
stbenjam committed Nov 13, 2020
1 parent 9be316b commit 1d91dff
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 8 deletions.
4 changes: 2 additions & 2 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ spec:
description: External bridge is used for external communication.
type: string
externalMACAddress:
description: ExternalMACAddress is used to allow setting a static MAC address for the bootstrap host on the external network. If left blank, libvirt will generate one for you.
description: ExternalMACAddress is used to allow setting a static unicast MAC address for the bootstrap host on the external network. Consider using the QEMU vendor prefix `52:54:00`. If left blank, libvirt will generate one for you.
type: string
hosts:
description: Hosts is the information needed to create the objects in Ironic.
Expand Down Expand Up @@ -953,7 +953,7 @@ spec:
description: ClusterProvisioningIP is the IP on the dedicated provisioning network where the baremetal-operator pod runs provisioning services, and an http server to cache some downloaded content e.g RHCOS/IPA images
type: string
provisioningMACAddress:
description: ProvisioningMACAddress is used to allow setting a static MAC address for the bootstrap host on the provisioning network. If left blank, libvirt will generate one for you.
description: ProvisioningMACAddress is used to allow setting a static unicast MAC address for the bootstrap host on the provisioning network. Consider using the QEMU vendor prefix `52:54:00`. If left blank, libvirt will generate one for you.
type: string
provisioningNetwork:
default: Managed
Expand Down
10 changes: 6 additions & 4 deletions pkg/types/baremetal/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ type Platform struct {
// +optional
ExternalBridge string `json:"externalBridge,omitempty"`

// ExternalMACAddress is used to allow setting a static MAC address for
// the bootstrap host on the external network. If left blank, libvirt will
// ExternalMACAddress is used to allow setting a static unicast MAC
// address for the bootstrap host on the external network. Consider
// using the QEMU vendor prefix `52:54:00`. If left blank, libvirt will
// generate one for you.
// +optional
ExternalMACAddress string `json:"externalMACAddress,omitempty"`
Expand All @@ -99,8 +100,9 @@ type Platform struct {
// +optional
ProvisioningBridge string `json:"provisioningBridge,omitempty"`

// ProvisioningMACAddress is used to allow setting a static MAC address for
// the bootstrap host on the provisioning network. If left blank, libvirt will
// ProvisioningMACAddress is used to allow setting a static unicast MAC
// address for the bootstrap host on the provisioning network. Consider
// using the QEMU vendor prefix `52:54:00`. If left blank, libvirt will
// generate one for you.
// +optional
ProvisioningMACAddress string `json:"provisioningMACAddress,omitempty"`
Expand Down
7 changes: 7 additions & 0 deletions pkg/types/baremetal/validation/platform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,13 @@ func TestValidateProvisioning(t *testing.T) {
ExternalMACAddress("CA:FE:CA:FE:CA:FE").
build(),
},
{
name: "invalid_multicast_mac",
platform: platform().
ExternalMACAddress("7D:CE:E3:29:35:6F").
build(),
expected: "expected unicast mac address, found multicast",
},
{
name: "invalid_bootstrapprovip_wrongCIDR",
platform: platform().
Expand Down
34 changes: 32 additions & 2 deletions pkg/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,39 @@ func IP(ip string) error {
return nil
}

// MAC validates that a value is a valid mac address
// MAC validates that a value is a valid unicast EUI-48 MAC address
func MAC(addr string) error {
_, err := net.ParseMAC(addr)
hwAddr, err := net.ParseMAC(addr)

// net.ParseMAC checks for any valid mac, including 20-octet infiniband
// MAC's. Let's make sure we have an EUI-48 MAC, consisting of 6 octets
if err == nil && len(hwAddr) != 6 {
return fmt.Errorf("invalid MAC address")
}

// We also need to check that the MAC is a valid unicast address. A multicast
// address is an address where the least significant bit of the most significant
// byte is 1.
//
// Example 1: Multicast MAC
// ------------------------
// 7D:CE:E3:29:35:6F
// ^--> most significant byte
//
// 0x7D is 0b11111101
// ^--> this is a multicast MAC
//
// Example 2: Unicast MAC
// ----------------------
// 7A:CE:E3:29:35:6F
// ^--> most significant byte
//
// 0x7A is 0b11111010
// ^--> this is a unicast MAC
if err == nil && hwAddr[0]&1 == 1 {
return fmt.Errorf("expected unicast mac address, found multicast: %q", addr)
}

return err
}

Expand Down
38 changes: 38 additions & 0 deletions pkg/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,3 +563,41 @@ func TestURI(t *testing.T) {
})
}
}

func TestMAC(t *testing.T) {
cases := []struct {
name string
addr string
expected string
}{
{
name: "valid_mac",
addr: "7A:CE:E3:29:35:6F",
},
{
name: "invalid_multicast",
addr: "7D:CE:E3:29:35:6F",
expected: "expected unicast mac address",
},
{
name: "invalid_infiniband",
addr: "00-00-00-00-fe-80-00-00-00-00-00-00-02-00-5e-10-00-00-00-01",
expected: "invalid MAC address",
},
{
name: "invalid_mac",
addr: "this is a bad mac",
expected: "invalid MAC address",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := MAC(tc.addr)
if tc.expected == "" {
assert.NoError(t, err)
} else {
assert.Regexp(t, tc.expected, err)
}
})
}
}

0 comments on commit 1d91dff

Please sign in to comment.