Skip to content

Commit

Permalink
Merge pull request #1411 from stgraber/cluster
Browse files Browse the repository at this point in the history 
incusd/cluster: Validate cluster HTTPS address on join too
  • Loading branch information
@hallyn
hallyn authored Nov 22, 2024
2 parents ebf13b0 + ad0d260 commit 21eee07
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 232 deletions.
8 changes: 7 additions & 1 deletion cmd/incusd/api_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,17 @@ func clusterPutJoin(d *Daemon, r *http.Request, req api.ClusterPut) response.Res
return response.BadRequest(fmt.Errorf("This server is already clustered"))
}

// The old pre 'clustering_join' join API approach is no longer supported.
// Validate server address.
if req.ServerAddress == "" {
return response.BadRequest(fmt.Errorf("No server address provided for this member"))
}

// Check that the provided address is an IP address or DNS, not wildcard and isn't required to specify a port.
err := validate.IsListenAddress(true, false, false)(req.ServerAddress)
if err != nil {
return response.BadRequest(fmt.Errorf("Invalid server address %q: %w", req.ServerAddress, err))
}

localHTTPSAddress := s.LocalConfig.HTTPSAddress()

var config *node.Config
Expand Down
2 changes: 1 addition & 1 deletion internal/server/endpoints/endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"github.com/lxc/incus/v6/internal/linux"
"github.com/lxc/incus/v6/internal/server/endpoints/listeners"
"github.com/lxc/incus/v6/internal/server/util"
"github.com/lxc/incus/v6/internal/util"
"github.com/lxc/incus/v6/shared/logger"
localtls "github.com/lxc/incus/v6/shared/tls"
)
Expand Down
148 changes: 0 additions & 148 deletions internal/server/util/net.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"net"
"os"

"github.com/lxc/incus/v6/internal/ports"
internalUtil "github.com/lxc/incus/v6/internal/util"
"github.com/lxc/incus/v6/shared/logger"
localtls "github.com/lxc/incus/v6/shared/tls"
)
Expand Down Expand Up @@ -92,152 +90,6 @@ func ServerTLSConfig(cert *localtls.CertInfo) *tls.Config {
return config
}

// NetworkInterfaceAddress returns the first global unicast address of any of the system network interfaces.
// Return the empty string if none is found.
func NetworkInterfaceAddress() string {
ifaces, err := net.Interfaces()
if err != nil {
return ""
}

for _, iface := range ifaces {
addrs, err := iface.Addrs()
if err != nil {
continue
}

if len(addrs) == 0 {
continue
}

for _, addr := range addrs {
ipNet, ok := addr.(*net.IPNet)
if !ok {
continue
}

if !ipNet.IP.IsGlobalUnicast() {
continue
}

return ipNet.IP.String()
}
}

return ""
}

// IsAddressCovered detects if network address1 is actually covered by
// address2, in the sense that they are either the same address or address2 is
// specified using a wildcard with the same port of address1.
func IsAddressCovered(address1, address2 string) bool {
address1 = internalUtil.CanonicalNetworkAddress(address1, ports.HTTPSDefaultPort)
address2 = internalUtil.CanonicalNetworkAddress(address2, ports.HTTPSDefaultPort)

if address1 == address2 {
return true
}

host1, port1, err := net.SplitHostPort(address1)
if err != nil {
return false
}

host2, port2, err := net.SplitHostPort(address2)
if err != nil {
return false
}

// If the ports are different, then address1 is clearly not covered by
// address2.
if port2 != port1 {
return false
}

// If address1 contains a host name, let's try to resolve it, in order
// to compare the actual IPs.
var addresses1 []net.IP
if host1 != "" {
ip := net.ParseIP(host1)
if ip != nil {
addresses1 = append(addresses1, ip)
} else {
ips, err := net.LookupHost(host1)
if err == nil && len(ips) > 0 {
for _, ipStr := range ips {
ip := net.ParseIP(ipStr)
if ip != nil {
addresses1 = append(addresses1, ip)
}
}
}
}
}

// If address2 contains a host name, let's try to resolve it, in order
// to compare the actual IPs.
var addresses2 []net.IP
if host2 != "" {
ip := net.ParseIP(host2)
if ip != nil {
addresses2 = append(addresses2, ip)
} else {
ips, err := net.LookupHost(host2)
if err == nil && len(ips) > 0 {
for _, ipStr := range ips {
ip := net.ParseIP(ipStr)
if ip != nil {
addresses2 = append(addresses2, ip)
}
}
}
}
}

for _, a1 := range addresses1 {
for _, a2 := range addresses2 {
if a1.Equal(a2) {
return true
}
}
}

// If address2 is using an IPv4 wildcard for the host, then address2 is
// only covered if it's an IPv4 address.
if host2 == "0.0.0.0" {
ip1 := net.ParseIP(host1)
if ip1 != nil && ip1.To4() != nil {
return true
}

return false
}

// If address2 is using an IPv6 wildcard for the host, then address2 is
// always covered.
if host2 == "::" || host2 == "" {
return true
}

return false
}

// IsWildCardAddress returns whether the given address is a wildcard.
func IsWildCardAddress(address string) bool {
address = internalUtil.CanonicalNetworkAddress(address, ports.HTTPSDefaultPort)

host, _, err := net.SplitHostPort(address)
if err != nil {
return false
}

if host == "0.0.0.0" || host == "::" || host == "" {
return true
}

return false
}

// SysctlGet retrieves the value of a sysctl file in /proc/sys.
func SysctlGet(path string) (string, error) {
// Read the current content
Expand Down
82 changes: 0 additions & 82 deletions internal/server/util/net_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
package util_test

import (
"fmt"
"io"
"net"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/lxc/incus/v6/internal/ports"
"github.com/lxc/incus/v6/internal/server/util"
internalUtil "github.com/lxc/incus/v6/internal/util"
)

// The connection returned by the dialer is paired with the one returned by the
Expand Down Expand Up @@ -43,81 +39,3 @@ func TestInMemoryNetwork(t *testing.T) {
_, err = client.Write([]byte("hello"))
assert.EqualError(t, err, "io: read/write on closed pipe")
}

func TestCanonicalNetworkAddress(t *testing.T) {
cases := map[string]string{
"127.0.0.1": "127.0.0.1:8443",
"127.0.0.1:": "127.0.0.1:8443",
"foo.bar": "foo.bar:8443",
"foo.bar:": "foo.bar:8443",
"foo.bar:8444": "foo.bar:8444",
"192.168.1.1:443": "192.168.1.1:443",
"f921:7358:4510:3fce:ac2e:844:2a35:54e": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]:": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8444": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8444",
}

for in, out := range cases {
t.Run(in, func(t *testing.T) {
assert.Equal(t, out, internalUtil.CanonicalNetworkAddress(in, ports.HTTPSDefaultPort))
})
}
}

func TestIsAddressCovered(t *testing.T) {
type testCase struct {
address1 string
address2 string
covered bool
}

cases := []testCase{
{"127.0.0.1:8443", "127.0.0.1:8443", true},
{"garbage", "127.0.0.1:8443", false},
{"127.0.0.1:8444", "garbage", false},
{"127.0.0.1:8444", "127.0.0.1:8443", false},
{"127.0.0.1:8443", "0.0.0.0:8443", true},
{"[::1]:8443", "0.0.0.0:8443", false},
{":8443", "0.0.0.0:8443", false},
{"127.0.0.1:8443", "[::]:8443", true},
{"[::1]:8443", "[::]:8443", true},
{"[::1]:8443", ":8443", true},
{":8443", "[::]:8443", true},
{"0.0.0.0:8443", "[::]:8443", true},
{"10.30.0.8:8443", "[::]", true},
{"localhost:8443", "127.0.0.1:8443", true},
}

// Test some localhost cases too
ips, err := net.LookupHost("localhost")
if err == nil && len(ips) > 0 && ips[0] == "127.0.0.1" {
cases = append(cases, testCase{"127.0.0.1:8443", "localhost:8443", true})
}

ips, err = net.LookupHost("ip6-localhost")
if err == nil && len(ips) > 0 && ips[0] == "::1" {
cases = append(cases, testCase{"[::1]:8443", "ip6-localhost:8443", true})
}

for _, c := range cases {
t.Run(fmt.Sprintf("%s-%s", c.address1, c.address2), func(t *testing.T) {
covered := internalUtil.IsAddressCovered(c.address1, c.address2)
if c.covered {
assert.True(t, covered)
} else {
assert.False(t, covered)
}
})
}
}

// This is a check against Go's stdlib to make sure that when listening to a port without specifying an address,
// then an IPv6 wildcard is assumed.
func TestListenImplicitIPv6Wildcard(t *testing.T) {
listener, err := net.Listen("tcp", ":9999")
require.NoError(t, err)
defer func() { _ = listener.Close() }()

assert.Equal(t, "[::]:9999", listener.Addr().String())
}
92 changes: 92 additions & 0 deletions internal/util/network_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package util_test

import (
"fmt"
"net"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/lxc/incus/v6/internal/ports"
internalUtil "github.com/lxc/incus/v6/internal/util"
)

func TestCanonicalNetworkAddress(t *testing.T) {
cases := map[string]string{
"127.0.0.1": "127.0.0.1:8443",
"127.0.0.1:": "127.0.0.1:8443",
"foo.bar": "foo.bar:8443",
"foo.bar:": "foo.bar:8443",
"foo.bar:8444": "foo.bar:8444",
"192.168.1.1:443": "192.168.1.1:443",
"f921:7358:4510:3fce:ac2e:844:2a35:54e": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]:": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8443",
"[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8444": "[f921:7358:4510:3fce:ac2e:844:2a35:54e]:8444",
}

for in, out := range cases {
t.Run(in, func(t *testing.T) {
assert.Equal(t, out, internalUtil.CanonicalNetworkAddress(in, ports.HTTPSDefaultPort))
})
}
}

func TestIsAddressCovered(t *testing.T) {
type testCase struct {
address1 string
address2 string
covered bool
}

cases := []testCase{
{"127.0.0.1:8443", "127.0.0.1:8443", true},
{"garbage", "127.0.0.1:8443", false},
{"127.0.0.1:8444", "garbage", false},
{"127.0.0.1:8444", "127.0.0.1:8443", false},
{"127.0.0.1:8443", "0.0.0.0:8443", true},
{"[::1]:8443", "0.0.0.0:8443", false},
{":8443", "0.0.0.0:8443", false},
{"127.0.0.1:8443", "[::]:8443", true},
{"[::1]:8443", "[::]:8443", true},
{"[::1]:8443", ":8443", true},
{":8443", "[::]:8443", true},
{"0.0.0.0:8443", "[::]:8443", true},
{"10.30.0.8:8443", "[::]", true},
{"localhost:8443", "127.0.0.1:8443", true},
{"localhost:8443", "[::]:8443", true},
}

// Test some localhost cases too
ips, err := net.LookupHost("localhost")
if err == nil && len(ips) > 0 && ips[0] == "127.0.0.1" {
cases = append(cases, testCase{"127.0.0.1:8443", "localhost:8443", true})
}

ips, err = net.LookupHost("ip6-localhost")
if err == nil && len(ips) > 0 && ips[0] == "::1" {
cases = append(cases, testCase{"[::1]:8443", "ip6-localhost:8443", true})
}

for _, c := range cases {
t.Run(fmt.Sprintf("%s-%s", c.address1, c.address2), func(t *testing.T) {
covered := internalUtil.IsAddressCovered(c.address1, c.address2)
if c.covered {
assert.True(t, covered)
} else {
assert.False(t, covered)
}
})
}
}

// This is a check against Go's stdlib to make sure that when listening to a port without specifying an address,
// then an IPv6 wildcard is assumed.
func TestListenImplicitIPv6Wildcard(t *testing.T) {
listener, err := net.Listen("tcp", ":9999")
require.NoError(t, err)
defer func() { _ = listener.Close() }()

assert.Equal(t, "[::]:9999", listener.Addr().String())
}

0 comments on commit 21eee07

Please sign in to comment.