Skip to content

Commit

Permalink
fix(ipnet): tighten the checking of IP addresses
Browse files Browse the repository at this point in the history
  • Loading branch information
@favonia
favonia committed Sep 23, 2024
1 parent 2d95d69 commit 3189af9
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 68 deletions.
42 changes: 30 additions & 12 deletions internal/ipnet/ipnet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (t Type) UDPNetwork() string {

// Matches checks whether an IP belongs to it.
func (t Type) Matches(ip netip.Addr) bool {
ip = ip.Unmap()
switch t {
case IP4:
return ip.Is4()
Expand All @@ -79,9 +80,9 @@ func (t Type) Matches(ip netip.Addr) bool {
// NormalizeDetectedIP normalizes an IP into an IPv4 or IPv6 address.
func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool) {
if !ip.IsValid() {
ppfmt.Noticef(
pp.EmojiImpossible,
`Detected IP address is not valid`,
ppfmt.Noticef(pp.EmojiImpossible,
`Detected IP address is not valid; this should not happen and please report it at %s`,
pp.IssueReportingURL,
)
return netip.Addr{}, false
}
Expand Down Expand Up @@ -115,21 +116,38 @@ func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool)
return netip.Addr{}, false
}

if ip.IsUnspecified() {
ppfmt.Noticef(
pp.EmojiImpossible,
`Detected IP address %s is an unspecified %s address`,
ip.String(),
t.Describe(),
switch {
case ip.IsUnspecified():
ppfmt.Noticef(pp.EmojiError,
`Detected %s address %s is an unspecified address`,
t.Describe(), ip.String(),
)
return netip.Addr{}, false
case ip.IsLoopback():
ppfmt.Noticef(pp.EmojiError,
`Detected %s address %s is a loopback address`,
t.Describe(), ip.String(),
)
return netip.Addr{}, false
case ip.IsInterfaceLocalMulticast():
ppfmt.Noticef(pp.EmojiError,
`Detected %s address %s is an interface-local multicast address`,
t.Describe(), ip.String(),
)
return netip.Addr{}, false
case ip.IsLinkLocalMulticast(), ip.IsLinkLocalUnicast():
ppfmt.Noticef(pp.EmojiError,
`Detected %s address %s is a link-local address`,
t.Describe(), ip.String(),
)
return netip.Addr{}, false
}

if !ip.IsGlobalUnicast() {
ppfmt.Noticef(
pp.EmojiUserWarning,
`Detected IP address %s does not look like a global unicast IP address.`,
ip.String(),
pp.EmojiWarning,
`Detected %s address %s does not look like a global unicast address`,
t.Describe(), ip.String(),

Check warning on line 150 in internal/ipnet/ipnet.go

View check run for this annotation

Codecov / codecov/patch

internal/ipnet/ipnet.go#L148-L150 

Added lines #L148 - L150 were not covered by tests
)
}

Expand Down
67 changes: 53 additions & 14 deletions internal/ipnet/ipnet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ func TestNormalizeDetectedIP(t *testing.T) {
ok bool
prepareMockPP func(*mocks.MockPP)
}{
"4-invalid": {
ipnet.IP4,
netip.Addr{},
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible,
`Detected IP address is not valid; this should not happen and please report it at %s`,
pp.IssueReportingURL)
},
},
"4-1::2": {
ipnet.IP4, mustIP("1::2"),
netip.Addr{},
Expand All @@ -102,7 +113,43 @@ func TestNormalizeDetectedIP(t *testing.T) {
},
},
"4-::ffff:0a0a:0a0a": {ipnet.IP4, mustIP("::ffff:0a0a:0a0a"), mustIP("10.10.10.10"), true, nil},
"6-1::2": {ipnet.IP6, mustIP("1::2"), mustIP("1::2"), true, nil},
"4-0.0.0.0": {
ipnet.IP4, mustIP("0.0.0.0"),
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiError,
"Detected %s address %s is an unspecified address", "IPv4", "0.0.0.0")
},
},
"4-127.0.0.1": {
ipnet.IP4, mustIP("127.0.0.1"),
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiError, "Detected %s address %s is a loopback address", "IPv4", "127.0.0.1")
},
},
"4-224.0.0.1": {
ipnet.IP4, mustIP("224.0.0.1"),
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiError, "Detected %s address %s is a link-local address", "IPv4", "224.0.0.1")
},
},
"6-invalid": {
ipnet.IP6,
netip.Addr{},
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible,
`Detected IP address is not valid; this should not happen and please report it at %s`,
pp.IssueReportingURL)
},
},
"6-1::2": {ipnet.IP6, mustIP("1::2"), mustIP("1::2"), true, nil},
"6-10.10.10.10": {
ipnet.IP6, mustIP("10.10.10.10"),
netip.Addr{},
Expand All @@ -128,13 +175,14 @@ func TestNormalizeDetectedIP(t *testing.T) {
)
},
},
"6-invalid": {
ipnet.IP6,
netip.Addr{},
"6-ff01::1": {
ipnet.IP6, mustIP("ff01::1"),
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible, "Detected IP address is not valid")
m.EXPECT().Noticef(pp.EmojiError,
"Detected %s address %s is an interface-local multicast address",
"IPv6", "ff01::1")
},
},
"100-10.10.10.10": {
Expand All @@ -143,15 +191,6 @@ func TestNormalizeDetectedIP(t *testing.T) {
false,
nil,
},
"4-0.0.0.0": {
ipnet.IP4, mustIP("0.0.0.0"),
netip.Addr{},
false,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible,
"Detected IP address %s is an unspecified %s address", "0.0.0.0", "IPv4")
},
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/happy_eyeballs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (p happyEyeballs) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type)
queuedPP[r.method].Flush()

if r.method == MethodAlternative {
ppfmt.Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
ppfmt.Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
}

return r.ip, r.method, r.ok
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/happy_eyeballs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestHappyEyeballs(t *testing.T) {
return ip1001, true
}).Times(2)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
p.EXPECT().HasAlternative(ipNet).Return(true)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
},
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestHappyEyeballs(t *testing.T) {
return ip1001, true
}).Times(2)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
p.EXPECT().HasAlternative(ipNet).Return(true)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
},
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestHappyEyeballs(t *testing.T) {
return ip1001, true
}).Times(2)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
p.EXPECT().HasAlternative(ipNet).Return(true)
ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
},
Expand Down
61 changes: 23 additions & 38 deletions internal/provider/protocol/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,71 +28,56 @@ func TestLocalName(t *testing.T) {
func TestLocalGetIP(t *testing.T) {
t.Parallel()

ip4Loopback := netip.MustParseAddr("127.0.0.1")
ip6Loopback := netip.MustParseAddr("::1")
invalidIP := netip.Addr{}

for name, tc := range map[string]struct {
addrKey ipnet.Type
addr string
ipNet ipnet.Type
expected gomock.Matcher
ok bool
expected netip.Addr
prepareMockPP func(*mocks.MockPP)
}{
"4": {
ipnet.IP4, "127.0.0.1:80", ipnet.IP4, gomock.Eq(ip4Loopback), true,
"loopback/4": {
ipnet.IP4, "127.0.0.1:80", ipnet.IP4,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiUserWarning,
"Detected IP address %s does not look like a global unicast IP address.", "127.0.0.1")
m.EXPECT().Noticef(pp.EmojiError,
"Detected %s address %s is a loopback address", "IPv4", "127.0.0.1")
},
},
"6": {
"loopback/6": {
ipnet.IP6, "[::1]:80", ipnet.IP6,
gomock.AnyOf(
ip6Loopback,
gomock.Cond(func(x any) bool {
a, ok := x.(netip.Addr)
if !ok {
return false
}
return a.IsLinkLocalUnicast()
})),
true,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiUserWarning,
"Detected IP address %s does not look like a global unicast IP address.",
gomock.AnyOf(
"::1",
gomock.Cond(func(x any) bool {
s, ok := x.(string)
if !ok {
return false
}
return netip.MustParseAddr(s).IsLinkLocalUnicast()
})))
m.EXPECT().Noticef(pp.EmojiError,
"Detected %s address %s is a loopback address", "IPv6", "::1")
},
},
"4-nil1": {
ipnet.IP4, "", ipnet.IP4, gomock.Eq(invalidIP), false,
"empty/4": {
ipnet.IP4, "", ipnet.IP4,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiError, "Failed to detect a local %s address: %v", "IPv4", gomock.Any())
},
},
"6-nil1": {
ipnet.IP6, "", ipnet.IP6, gomock.Eq(invalidIP), false,
"empty/6": {
ipnet.IP6, "", ipnet.IP6,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiError, "Failed to detect a local %s address: %v", "IPv6", gomock.Any())
},
},
"4-nil2": {
ipnet.IP4, "127.0.0.1:80", ipnet.IP6, gomock.Eq(invalidIP), false,
"mismatch/6": {
ipnet.IP4, "127.0.0.1:80", ipnet.IP6,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible, "Unhandled IP network: %s", "IPv6")
},
},
"6-nil2": {
ipnet.IP6, "::1:80", ipnet.IP4, gomock.Eq(invalidIP), false,
"mismatch/4": {
ipnet.IP6, "::1:80", ipnet.IP4,
false, invalidIP,
func(m *mocks.MockPP) {
m.EXPECT().Noticef(pp.EmojiImpossible, "Unhandled IP network: %s", "IPv4")
},
Expand All @@ -114,7 +99,7 @@ func TestLocalGetIP(t *testing.T) {
tc.prepareMockPP(mockPP)
}
ip, method, ok := provider.GetIP(context.Background(), mockPP, tc.ipNet)
require.True(t, tc.expected.Matches(ip))
require.Equal(t, tc.expected, ip)
require.NotEqual(t, protocol.MethodAlternative, method)
require.Equal(t, tc.ok, ok)
})
Expand Down

0 comments on commit 3189af9

Please sign in to comment.