From c1ef55249ad91ecab9d1855f816c92a6bd226b84 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Wed, 11 Oct 2023 15:10:24 -0700 Subject: [PATCH] types/ipproto: import and test string parsing for ipproto IPProto has been being converted to and from string formats in multiple locations with variations in behavior. TextMarshaller and JSONMarshaller implementations are now added, along with defined accepted and preferred formats to centralize the logic into a single cross compatible implementation. Updates tailscale/corp#15043 Fixes tailscale/corp#15141 Signed-off-by: James Tucker --- cmd/derper/depaware.txt | 3 +- cmd/tailscale/depaware.txt | 2 + cmd/tailscaled/depaware.txt | 2 + types/ipproto/ipproto.go | 102 ++++++++++++++++++++++++- types/ipproto/ipproto_test.go | 138 ++++++++++++++++++++++++++++++++++ 5 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 types/ipproto/ipproto_test.go diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index c7cb9b4da..61bf89f0b 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -147,10 +147,11 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa L tailscale.com/util/linuxfw from tailscale.com/net/netns tailscale.com/util/mak from tailscale.com/syncs+ tailscale.com/util/multierr from tailscale.com/health+ + tailscale.com/util/nocasemaps from tailscale.com/types/ipproto tailscale.com/util/set from tailscale.com/health+ tailscale.com/util/singleflight from tailscale.com/net/dnscache tailscale.com/util/slicesx from tailscale.com/cmd/derper+ - tailscale.com/util/vizerror from tailscale.com/tsweb + tailscale.com/util/vizerror from tailscale.com/tsweb+ W 💣 tailscale.com/util/winutil from tailscale.com/hostinfo+ tailscale.com/version from tailscale.com/derp+ tailscale.com/version/distro from tailscale.com/hostinfo+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 6a4415067..7b253378e 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -152,11 +152,13 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/util/mak from tailscale.com/net/netcheck+ tailscale.com/util/multierr from tailscale.com/control/controlhttp+ tailscale.com/util/must from tailscale.com/cmd/tailscale/cli+ + tailscale.com/util/nocasemaps from tailscale.com/types/ipproto tailscale.com/util/quarantine from tailscale.com/cmd/tailscale/cli tailscale.com/util/set from tailscale.com/health+ tailscale.com/util/singleflight from tailscale.com/net/dnscache tailscale.com/util/slicesx from tailscale.com/net/dnscache+ tailscale.com/util/testenv from tailscale.com/cmd/tailscale/cli + tailscale.com/util/vizerror from tailscale.com/types/ipproto 💣 tailscale.com/util/winutil from tailscale.com/hostinfo+ W 💣 tailscale.com/util/winutil/authenticode from tailscale.com/clientupdate tailscale.com/version from tailscale.com/cmd/tailscale/cli+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index c99601c0e..c0b94b113 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -346,6 +346,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/mak from tailscale.com/control/controlclient+ tailscale.com/util/multierr from tailscale.com/control/controlclient+ tailscale.com/util/must from tailscale.com/logpolicy+ + tailscale.com/util/nocasemaps from tailscale.com/types/ipproto 💣 tailscale.com/util/osdiag from tailscale.com/cmd/tailscaled+ W 💣 tailscale.com/util/osdiag/internal/wsc from tailscale.com/util/osdiag tailscale.com/util/osshare from tailscale.com/ipn/ipnlocal+ @@ -362,6 +363,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/testenv from tailscale.com/ipn/ipnlocal+ tailscale.com/util/uniq from tailscale.com/wgengine/magicsock+ + tailscale.com/util/vizerror from tailscale.com/types/ipproto 💣 tailscale.com/util/winutil from tailscale.com/control/controlclient+ W 💣 tailscale.com/util/winutil/authenticode from tailscale.com/util/osdiag+ W tailscale.com/util/winutil/policy from tailscale.com/ipn/ipnlocal diff --git a/types/ipproto/ipproto.go b/types/ipproto/ipproto.go index 001b54500..b5333eb56 100644 --- a/types/ipproto/ipproto.go +++ b/types/ipproto/ipproto.go @@ -4,7 +4,13 @@ // Package ipproto contains IP Protocol constants. package ipproto -import "fmt" +import ( + "fmt" + "strconv" + + "tailscale.com/util/nocasemaps" + "tailscale.com/util/vizerror" +) // Version describes the IP address version. type Version uint8 @@ -69,6 +75,7 @@ const ( Fragment Proto = 0xFF ) +// Deprecated: use MarshalText instead. func (p Proto) String() string { switch p { case Unknown: @@ -97,3 +104,96 @@ func (p Proto) String() string { return fmt.Sprintf("IPProto-%d", int(p)) } } + +// Prefer names from +// https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml +// unless otherwise noted. +var ( + // preferredNames is the set of protocol names that re produced by + // MarshalText, and are the preferred representation. + preferredNames = map[Proto]string{ + 51: "ah", + DCCP: "dccp", + 8: "egp", + 50: "esp", + 47: "gre", + ICMPv4: "icmp", + IGMP: "igmp", + 9: "igp", + 4: "ipv4", + ICMPv6: "ipv6-icmp", + SCTP: "sctp", + TCP: "tcp", + UDP: "udp", + } + + // acceptedNames is the set of protocol names that are accepted by + // UnmarshalText. + acceptedNames = map[string]Proto{ + "ah": 51, + "dccp": DCCP, + "egp": 8, + "esp": 50, + "gre": 47, + "icmp": ICMPv4, + "icmpv4": ICMPv4, + "icmpv6": ICMPv6, + "igmp": IGMP, + "igp": 9, + "ip-in-ip": 4, // IANA says "ipv4"; Wikipedia/popular use says "ip-in-ip" + "ipv4": 4, + "ipv6-icmp": ICMPv6, + "sctp": SCTP, + "tcp": TCP, + "tsmp": TSMP, + "udp": UDP, + } +) + +// UnmarshalText implements encoding.TextUnmarshaler. If the input is empty, p +// is set to 0. If an error occurs, p is unchanged. +func (p *Proto) UnmarshalText(b []byte) error { + if len(b) == 0 { + *p = 0 + return nil + } + + if u, err := strconv.ParseUint(string(b), 10, 8); err == nil { + *p = Proto(u) + return nil + } + + if newP, ok := nocasemaps.GetOk(acceptedNames, string(b)); ok { + *p = newP + return nil + } + + return vizerror.Errorf("proto name %q not known; use protocol number 0-255", b) +} + +// MarshalText implements encoding.TextMarshaler. +func (p Proto) MarshalText() ([]byte, error) { + if s, ok := preferredNames[p]; ok { + return []byte(s), nil + } + return []byte(strconv.Itoa(int(p))), nil +} + +// MarshalJSON implements json.Marshaler. +func (p Proto) MarshalJSON() ([]byte, error) { + return []byte(strconv.Itoa(int(p))), nil +} + +// UnmarshalJSON implements json.Unmarshaler. If the input is empty, p is set to +// 0. If an error occurs, p is unchanged. The input must be a JSON number or an +// accepted string name. +func (p *Proto) UnmarshalJSON(b []byte) error { + if len(b) == 0 { + *p = 0 + return nil + } + if b[0] == '"' { + b = b[1 : len(b)-1] + } + return p.UnmarshalText(b) +} diff --git a/types/ipproto/ipproto_test.go b/types/ipproto/ipproto_test.go new file mode 100644 index 000000000..1ebeeee9f --- /dev/null +++ b/types/ipproto/ipproto_test.go @@ -0,0 +1,138 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package ipproto + +import ( + "encoding" + "encoding/json" + "fmt" + "testing" + + "tailscale.com/util/must" +) + +// Ensure that the Proto type implements encoding.TextMarshaler and +// encoding.TextUnmarshaler. +var ( + _ encoding.TextMarshaler = (*Proto)(nil) + _ encoding.TextUnmarshaler = (*Proto)(nil) +) + +func TestHistoricalStringNames(t *testing.T) { + // A subset of supported protocols were described with their lowercase String() representations and must remain supported. + var historical = map[string]Proto{ + "icmpv4": ICMPv4, + "igmp": IGMP, + "tcp": TCP, + "udp": UDP, + "dccp": DCCP, + "gre": GRE, + "sctp": SCTP, + } + + for name, proto := range historical { + var p Proto + must.Do(p.UnmarshalText([]byte(name))) + if got, want := p, proto; got != want { + t.Errorf("Proto.UnmarshalText(%q) = %v, want %v", name, got, want) + } + } +} + +func TestAcceptedNamesContainsPreferredNames(t *testing.T) { + for proto, name := range preferredNames { + if _, ok := acceptedNames[name]; !ok { + t.Errorf("preferredNames[%q] = %v, but acceptedNames does not contain it", name, proto) + } + } +} + +func TestProtoTextEncodingRoundTrip(t *testing.T) { + for i := 0; i < 256; i++ { + text := must.Get(Proto(i).MarshalText()) + var p Proto + must.Do(p.UnmarshalText(text)) + + if got, want := p, Proto(i); got != want { + t.Errorf("Proto(%d) round-trip got %v, want %v", i, got, want) + } + } +} + +func TestProtoUnmarshalText(t *testing.T) { + var p Proto = 1 + err := p.UnmarshalText([]byte(nil)) + if err != nil || p != 0 { + t.Fatalf("empty input, got err=%v, p=%v, want nil, 0", err, p) + } + + for i := 0; i < 256; i++ { + var p Proto + must.Do(p.UnmarshalText([]byte(fmt.Sprintf("%d", i)))) + if got, want := p, Proto(i); got != want { + t.Errorf("Proto(%d) = %v, want %v", i, got, want) + } + } + + for name, wantProto := range acceptedNames { + var p Proto + must.Do(p.UnmarshalText([]byte(name))) + if got, want := p, wantProto; got != want { + t.Errorf("Proto(%q) = %v, want %v", name, got, want) + } + } + + for wantProto, name := range preferredNames { + var p Proto + must.Do(p.UnmarshalText([]byte(name))) + if got, want := p, wantProto; got != want { + t.Errorf("Proto(%q) = %v, want %v", name, got, want) + } + } +} + +func TestProtoMarshalText(t *testing.T) { + for i := 0; i < 256; i++ { + text := must.Get(Proto(i).MarshalText()) + + if wantName, ok := preferredNames[Proto(i)]; ok { + if got, want := string(text), wantName; got != want { + t.Errorf("Proto(%d).MarshalText() = %q, want preferred name %q", i, got, want) + } + continue + } + + if got, want := string(text), fmt.Sprintf("%d", i); got != want { + t.Errorf("Proto(%d).MarshalText() = %q, want %q", i, got, want) + } + } +} + +func TestProtoMarshalJSON(t *testing.T) { + for i := 0; i < 256; i++ { + j := must.Get(Proto(i).MarshalJSON()) + if got, want := string(j), fmt.Sprintf(`%d`, i); got != want { + t.Errorf("Proto(%d).MarshalJSON() = %q, want %q", i, got, want) + } + } +} + +func TestProtoUnmarshalJSON(t *testing.T) { + var p Proto + + for i := 0; i < 256; i++ { + j := []byte(fmt.Sprintf(`%d`, i)) + must.Do(json.Unmarshal(j, &p)) + if got, want := p, Proto(i); got != want { + t.Errorf("Proto(%d) = %v, want %v", i, got, want) + } + } + + for name, wantProto := range acceptedNames { + must.Do(json.Unmarshal([]byte(fmt.Sprintf(`"%s"`, name)), &p)) + if got, want := p, wantProto; got != want { + t.Errorf("Proto(%q) = %v, want %v", name, got, want) + } + } +}