From eda6e560c369ea39e286cd1d916bc7e369fd7e03 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 2 Aug 2021 22:51:50 +0100 Subject: [PATCH 1/4] debug logging --- api.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api.go b/api.go index 088c337f..97ec4d41 100644 --- a/api.go +++ b/api.go @@ -445,6 +445,7 @@ func (h *Headscale) handleAuthKey(c *gin.Context, db *gorm.DB, idKey wgkey.Key, log.Println(err) return } + log.Printf("Assigning %s to %s", ip, m.Name) m.AuthKeyID = uint(pak.ID) m.IPAddress = ip.String() From 73207decfd13b703370d3ea2b57460111363fa4e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 3 Aug 2021 07:42:11 +0100 Subject: [PATCH 2/4] Check that IP is set before parsing Machine is saved to db before it is assigned an ip, so we might have empty ip fields coming back. --- utils.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/utils.go b/utils.go index 404e3823..45d44562 100644 --- a/utils.go +++ b/utils.go @@ -121,12 +121,14 @@ func (h *Headscale) getUsedIPs() ([]netaddr.IP, error) { ips := make([]netaddr.IP, len(addresses)) for index, addr := range addresses { - ip, err := netaddr.ParseIP(addr) - if err != nil { - return nil, fmt.Errorf("failed to parse ip from database, %w", err) - } + if addr != "" { + ip, err := netaddr.ParseIP(addr) + if err != nil { + return nil, fmt.Errorf("failed to parse ip from database, %w", err) + } - ips[index] = ip + ips[index] = ip + } } return ips, nil From d3349aa4d13fca1ad6ef28424657d8e6219b1aab Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 3 Aug 2021 09:26:28 +0100 Subject: [PATCH 3/4] Add test to ensure we can deal with empty ips from database --- utils_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/utils_test.go b/utils_test.go index 439fdc19..8fbe1375 100644 --- a/utils_test.go +++ b/utils_test.go @@ -117,3 +117,39 @@ func (s *Suite) TestGetMultiIp(c *check.C) { c.Assert(nextIP2.String(), check.Equals, expectedNextIP.String()) } + +func (s *Suite) TestGetAvailableIpMachineWithoutIP(c *check.C) { + ip, err := h.getAvailableIP() + c.Assert(err, check.IsNil) + + expected := netaddr.MustParseIP("10.27.0.0") + + c.Assert(ip.String(), check.Equals, expected.String()) + + n, err := h.CreateNamespace("test_ip") + c.Assert(err, check.IsNil) + + pak, err := h.CreatePreAuthKey(n.Name, false, false, nil) + c.Assert(err, check.IsNil) + + _, err = h.GetMachine("test", "testmachine") + c.Assert(err, check.NotNil) + + m := Machine{ + ID: 0, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Name: "testmachine", + NamespaceID: n.ID, + Registered: true, + RegisterMethod: "authKey", + AuthKeyID: uint(pak.ID), + } + h.db.Save(&m) + + ip2, err := h.getAvailableIP() + c.Assert(err, check.IsNil) + + c.Assert(ip2.String(), check.Equals, expected.String()) +} From ea615e3a268126ec291e72481f016d3dfd3d7040 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 3 Aug 2021 10:06:42 +0100 Subject: [PATCH 4/4] Do not issue "network" or "broadcast" addresses (0 or 255) --- utils.go | 26 +++++++++++--------------- utils_test.go | 18 +++++++++--------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/utils.go b/utils.go index 45d44562..03dc673f 100644 --- a/utils.go +++ b/utils.go @@ -79,21 +79,6 @@ func (h *Headscale) getAvailableIP() (*netaddr.IP, error) { return nil, err } - // for _, ip := range usedIps { - // nextIP := ip.Next() - - // if !containsIPs(usedIps, nextIP) && ipPrefix.Contains(nextIP) { - // return &nextIP, nil - // } - // } - - // // If there are no IPs in use, we are starting fresh and - // // can issue IPs from the beginning of the prefix. - // ip := ipPrefix.IP() - // return &ip, nil - - // return nil, fmt.Errorf("failed to find any available IP in %s", ipPrefix) - // Get the first IP in our prefix ip := ipPrefix.IP() @@ -102,8 +87,19 @@ func (h *Headscale) getAvailableIP() (*netaddr.IP, error) { return nil, fmt.Errorf("could not find any suitable IP in %s", ipPrefix) } + // Some OS (including Linux) does not like when IPs ends with 0 or 255, which + // is typically called network or broadcast. Lets avoid them and continue + // to look when we get one of those traditionally reserved IPs. + ipRaw := ip.As4() + if ipRaw[3] == 0 || ipRaw[3] == 255 { + ip = ip.Next() + continue + } + if ip.IsZero() && ip.IsLoopback() { + + ip = ip.Next() continue } diff --git a/utils_test.go b/utils_test.go index 8fbe1375..f50cd117 100644 --- a/utils_test.go +++ b/utils_test.go @@ -10,7 +10,7 @@ func (s *Suite) TestGetAvailableIp(c *check.C) { c.Assert(err, check.IsNil) - expected := netaddr.MustParseIP("10.27.0.0") + expected := netaddr.MustParseIP("10.27.0.1") c.Assert(ip.String(), check.Equals, expected.String()) } @@ -46,7 +46,7 @@ func (s *Suite) TestGetUsedIps(c *check.C) { c.Assert(err, check.IsNil) - expected := netaddr.MustParseIP("10.27.0.0") + expected := netaddr.MustParseIP("10.27.0.1") c.Assert(ips[0], check.Equals, expected) @@ -91,20 +91,20 @@ func (s *Suite) TestGetMultiIp(c *check.C) { c.Assert(len(ips), check.Equals, 350) - c.Assert(ips[0], check.Equals, netaddr.MustParseIP("10.27.0.0")) - c.Assert(ips[9], check.Equals, netaddr.MustParseIP("10.27.0.9")) - c.Assert(ips[300], check.Equals, netaddr.MustParseIP("10.27.1.44")) + c.Assert(ips[0], check.Equals, netaddr.MustParseIP("10.27.0.1")) + c.Assert(ips[9], check.Equals, netaddr.MustParseIP("10.27.0.10")) + c.Assert(ips[300], check.Equals, netaddr.MustParseIP("10.27.1.47")) // Check that we can read back the IPs m1, err := h.GetMachineByID(1) c.Assert(err, check.IsNil) - c.Assert(m1.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.0").String()) + c.Assert(m1.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.1").String()) m50, err := h.GetMachineByID(50) c.Assert(err, check.IsNil) - c.Assert(m50.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.49").String()) + c.Assert(m50.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.50").String()) - expectedNextIP := netaddr.MustParseIP("10.27.1.94") + expectedNextIP := netaddr.MustParseIP("10.27.1.97") nextIP, err := h.getAvailableIP() c.Assert(err, check.IsNil) @@ -122,7 +122,7 @@ func (s *Suite) TestGetAvailableIpMachineWithoutIP(c *check.C) { ip, err := h.getAvailableIP() c.Assert(err, check.IsNil) - expected := netaddr.MustParseIP("10.27.0.0") + expected := netaddr.MustParseIP("10.27.0.1") c.Assert(ip.String(), check.Equals, expected.String())