Repository: pires/go-proxyproto
Pull request #42 (2020-10-15 15:03:03)

Nuke TLV.Length
len(tlv.Value) can be used instead, and reduces the chances of a
mismatch between the actual length and the user-supplied Length field
when the user generates TLVs.

Obviously, this is a breaking change.

Closes: https://github.com/pires/go-proxyproto/issues/35


Patch:
diff --git a/tlv.go b/tlv.go
index 6873aa0..3bba207 100644
--- a/tlv.go
+++ b/tlv.go
@@ -44,9 +44,8 @@ type PP2Type byte
 
 // TLV is a uninterpreted Type-Length-Value for V2 protocol, see section 2.2
 type TLV struct {
-	Type   PP2Type
-	Length int
-	Value  []byte
+	Type  PP2Type
+	Value []byte
 }
 
 // SplitTLVs splits the Type-Length-Value vector, returns the vector or an error.
@@ -59,17 +58,17 @@ func SplitTLVs(raw []byte) ([]TLV, error) {
 		if len(raw)-i <= 3 {
 			return nil, ErrTruncatedTLV
 		}
-		tlv.Length = int(binary.BigEndian.Uint16(raw[i+1 : i+3])) // Max length = 65K
+		tlvLen := int(binary.BigEndian.Uint16(raw[i+1 : i+3])) // Max length = 65K
 		i += 3
-		if i+tlv.Length > len(raw) {
+		if i+tlvLen > len(raw) {
 			return nil, ErrTruncatedTLV
 		}
 		// Ignore no-op padding
 		if tlv.Type != PP2_TYPE_NOOP {
-			tlv.Value = make([]byte, tlv.Length)
-			copy(tlv.Value, raw[i:i+tlv.Length])
+			tlv.Value = make([]byte, tlvLen)
+			copy(tlv.Value, raw[i:i+tlvLen])
 		}
-		i += tlv.Length
+		i += tlvLen
 		tlvs = append(tlvs, tlv)
 	}
 	return tlvs, nil
@@ -79,14 +78,11 @@ func SplitTLVs(raw []byte) ([]TLV, error) {
 func JoinTLVs(tlvs []TLV) ([]byte, error) {
 	var raw []byte
 	for _, tlv := range tlvs {
-		if tlv.Length > math.MaxUint16 {
-			return nil, fmt.Errorf("proxyproto: cannot format TLV %v with length %d", tlv.Type, tlv.Length)
-		}
-		if tlv.Length != len(tlv.Value) {
-			return nil, fmt.Errorf("proxyproto: TLV %v length doesn't match value length", tlv.Type)
+		if len(tlv.Value) > math.MaxUint16 {
+			return nil, fmt.Errorf("proxyproto: cannot format TLV %v with length %d", tlv.Type, len(tlv.Value))
 		}
 		var length [2]byte
-		binary.BigEndian.PutUint16(length[:], uint16(tlv.Length))
+		binary.BigEndian.PutUint16(length[:], uint16(len(tlv.Value)))
 		raw = append(raw, byte(tlv.Type))
 		raw = append(raw, length[:]...)
 		raw = append(raw, tlv.Value...)
diff --git a/tlvparse/aws.go b/tlvparse/aws.go
index 0b43150..97d3d93 100644
--- a/tlvparse/aws.go
+++ b/tlvparse/aws.go
@@ -18,7 +18,7 @@ const (
 var vpceRe = regexp.MustCompile("^[A-Za-z0-9-]*$")
 
 func IsAWSVPCEndpointID(tlv proxyproto.TLV) bool {
-	return tlv.Type == PP2_TYPE_AWS && tlv.Length >= 1 && tlv.Value[0] == PP2_SUBTYPE_AWS_VPCE_ID
+	return tlv.Type == PP2_TYPE_AWS && len(tlv.Value) > 0 && tlv.Value[0] == PP2_SUBTYPE_AWS_VPCE_ID
 }
 
 func AWSVPCEndpointID(tlv proxyproto.TLV) (string, error) {
diff --git a/tlvparse/azure.go b/tlvparse/azure.go
index d1a151c..1803e42 100644
--- a/tlvparse/azure.go
+++ b/tlvparse/azure.go
@@ -17,7 +17,7 @@ const (
 
 // IsAzurePrivateEndpointLinkID returns true if given TLV matches Azure Private Endpoint LinkID format
 func isAzurePrivateEndpointLinkID(tlv proxyproto.TLV) bool {
-	return tlv.Type == PP2_TYPE_AZURE && tlv.Length == 5 && tlv.Value[0] == PP2_SUBTYPE_AZURE_PRIVATEENDPOINT_LINKID
+	return tlv.Type == PP2_TYPE_AZURE && len(tlv.Value) == 5 && tlv.Value[0] == PP2_SUBTYPE_AZURE_PRIVATEENDPOINT_LINKID
 }
 
 // AzurePrivateEndpointLinkID returns linkID if given TLV matches Azure Private Endpoint LinkID format
diff --git a/tlvparse/ssl.go b/tlvparse/ssl.go
index ba08af9..f09cbfd 100644
--- a/tlvparse/ssl.go
+++ b/tlvparse/ssl.go
@@ -78,9 +78,8 @@ func (s PP2SSL) Marshal() (proxyproto.TLV, error) {
 	v = append(v, tlvs...)
 
 	return proxyproto.TLV{
-		Type:   proxyproto.PP2_TYPE_SSL,
-		Length: len(v),
-		Value:  v,
+		Type:  proxyproto.PP2_TYPE_SSL,
+		Value: v,
 	}, nil
 }
 
@@ -97,7 +96,7 @@ func (s PP2SSL) ClientCN() (string, bool) {
 
 // SSLType is true if the TLV is type SSL
 func IsSSL(t proxyproto.TLV) bool {
-	return t.Type == proxyproto.PP2_TYPE_SSL && t.Length >= tlvSSLMinLen
+	return t.Type == proxyproto.PP2_TYPE_SSL && len(t.Value) >= tlvSSLMinLen
 }
 
 // SSL returns the pp2_tlv_ssl from section 2.2.5 or errors with ErrIncompatibleTLV or ErrMalformedTLV
@@ -106,7 +105,7 @@ func SSL(t proxyproto.TLV) (PP2SSL, error) {
 	if !IsSSL(t) {
 		return ssl, proxyproto.ErrIncompatibleTLV
 	}
-	if t.Length < tlvSSLMinLen {
+	if len(t.Value) < tlvSSLMinLen {
 		return ssl, proxyproto.ErrMalformedTLV
 	}
 	ssl.Client = t.Value[0]
@@ -127,7 +126,7 @@ func SSL(t proxyproto.TLV) (PP2SSL, error) {
 				appended at the end of the field in the TLV format using the type
 				PP2_SUBTYPE_SSL_VERSION.
 			*/
-			if tlv.Length == 0 || !isASCII(tlv.Value) {
+			if len(tlv.Value) == 0 || !isASCII(tlv.Value) {
 				return PP2SSL{}, proxyproto.ErrMalformedTLV
 			}
 			versionFound = true
@@ -137,7 +136,7 @@ func SSL(t proxyproto.TLV) (PP2SSL, error) {
 				(OID: 2.5.4.3) of the client certificate's Distinguished Name, is appended
 				using the TLV format and the type PP2_SUBTYPE_SSL_CN. E.g. "example.com".
 			*/
-			if tlv.Length == 0 || !utf8.Valid(tlv.Value) {
+			if len(tlv.Value) == 0 || !utf8.Valid(tlv.Value) {
 				return PP2SSL{}, proxyproto.ErrMalformedTLV
 			}
 			cnFound = true


Test patch:
diff --git a/header_test.go b/header_test.go
index 1c2bacc..b90366f 100644
--- a/header_test.go
+++ b/header_test.go
@@ -203,13 +203,12 @@ func TestSetTLVs(t *testing.T) {
 				DestinationPort:    2000,
 			},
 			tlvs: []TLV{{
-				Type:   PP2_TYPE_AUTHORITY,
-				Length: 11,
-				Value:  []byte("example.org"),
+				Type:  PP2_TYPE_AUTHORITY,
+				Value: []byte("example.org"),
 			}},
 		},
 		{
-			name: "add wrong length",
+			name: "add too long TLV",
 			header: &Header{
 				Version:            1,
 				Command:            PROXY,
@@ -220,9 +219,8 @@ func TestSetTLVs(t *testing.T) {
 				DestinationPort:    2000,
 			},
 			tlvs: []TLV{{
-				Type:   PP2_TYPE_AUTHORITY,
-				Length: 1,
-				Value:  []byte("example.org"),
+				Type:  PP2_TYPE_AUTHORITY,
+				Value: append(bytes.Repeat([]byte("a"), 0xFFFF), []byte(".example.org")...),
 			}},
 			expectErr: true,
 		},
diff --git a/tlv_test.go b/tlv_test.go
index 8968473..04a3fd9 100644
--- a/tlv_test.go
+++ b/tlv_test.go
@@ -131,18 +131,16 @@ func TestJoinTLVs(t *testing.T) {
 			name: "authority TLV",
 			raw:  append([]byte{byte(PP2_TYPE_AUTHORITY), 0x00, 0x0B}, []byte("example.org")...),
 			tlvs: []TLV{{
-				Type:   PP2_TYPE_AUTHORITY,
-				Length: 11,
-				Value:  []byte("example.org"),
+				Type:  PP2_TYPE_AUTHORITY,
+				Value: []byte("example.org"),
 			}},
 		},
 		{
 			name: "empty TLV",
 			raw:  []byte{byte(PP2_TYPE_NOOP), 0x00, 0x00},
 			tlvs: []TLV{{
-				Type:   PP2_TYPE_NOOP,
-				Length: 0,
-				Value:  nil,
+				Type:  PP2_TYPE_NOOP,
+				Value: nil,
 			}},
 		},
 	}
diff --git a/tlvparse/azure_test.go b/tlvparse/azure_test.go
index c71adfd..c418b3d 100644
--- a/tlvparse/azure_test.go
+++ b/tlvparse/azure_test.go
@@ -29,9 +29,8 @@ func TestFindAzurePrivateEndpointLinkID(t *testing.T) {
 			name: "AWS VPC endpoint ID",
 			tlvs: []proxyproto.TLV{
 				{
-					Type:   0xEA,
-					Length: 12,
-					Value:  []byte{0x01, 0x76, 0x70, 0x63, 0x65, 0x2d, 0x61, 0x62, 0x63, 0x31, 0x32, 0x33},
+					Type:  0xEA,
+					Value: []byte{0x01, 0x76, 0x70, 0x63, 0x65, 0x2d, 0x61, 0x62, 0x63, 0x31, 0x32, 0x33},
 				},
 			},
 			wantLinkID: 0,
@@ -41,9 +40,8 @@ func TestFindAzurePrivateEndpointLinkID(t *testing.T) {
 			name: "Azure but wrong subtype",
 			tlvs: []proxyproto.TLV{
 				{
-					Type:   0xEE,
-					Length: 5,
-					Value:  []byte{0x02, 0x01, 0x01, 0x01, 0x01},
+					Type:  0xEE,
+					Value: []byte{0x02, 0x01, 0x01, 0x01, 0x01},
 				},
 			},
 			wantLinkID: 0,
@@ -53,9 +51,8 @@ func TestFindAzurePrivateEndpointLinkID(t *testing.T) {
 			name: "Azure but wrong length",
 			tlvs: []proxyproto.TLV{
 				{
-					Type:   0xEE,
-					Length: 3,
-					Value:  []byte{0x02, 0x01, 0x01},
+					Type:  0xEE,
+					Value: []byte{0x02, 0x01, 0x01},
 				},
 			},
 			wantLinkID: 0,
@@ -65,9 +62,8 @@ func TestFindAzurePrivateEndpointLinkID(t *testing.T) {
 			name: "Azure link ID",
 			tlvs: []proxyproto.TLV{
 				{
-					Type:   0xEE,
-					Length: 5,
-					Value:  []byte{0x1, 0xc1, 0x45, 0x0, 0x21},
+					Type:  0xEE,
+					Value: []byte{0x1, 0xc1, 0x45, 0x0, 0x21},
 				},
 			},
 			wantLinkID: 0x210045c1,
@@ -77,29 +73,24 @@ func TestFindAzurePrivateEndpointLinkID(t *testing.T) {
 			name: "Multiple TLVs",
 			tlvs: []proxyproto.TLV{
 				{ // AWS
-					Type:   0xEA,
-					Length: 12,
-					Value:  []byte{0x01, 0x76, 0x70, 0x63, 0x65, 0x2d, 0x61, 0x62, 0x63, 0x31, 0x32, 0x33},
+					Type:  0xEA,
+					Value: []byte{0x01, 0x76, 0x70, 0x63, 0x65, 0x2d, 0x61, 0x62, 0x63, 0x31, 0x32, 0x33},
 				},
 				{ // Azure but wrong subtype
-					Type:   0xEE,
-					Length: 5,
-					Value:  []byte{0x02, 0x01, 0x01, 0x01, 0x01},
+					Type:  0xEE,
+					Value: []byte{0x02, 0x01, 0x01, 0x01, 0x01},
 				},
 				{ // Azure but wrong length
-					Type:   0xEE,
-					Length: 3,
-					Value:  []byte{0x02, 0x01, 0x01},
+					Type:  0xEE,
+					Value: []byte{0x02, 0x01, 0x01},
 				},
 				{ // Correct
-					Type:   0xEE,
-					Length: 5,
-					Value:  []byte{0x1, 0xc1, 0x45, 0x0, 0x21},
+					Type:  0xEE,
+					Value: []byte{0x1, 0xc1, 0x45, 0x0, 0x21},
 				},
 				{ // Also correct, but second in line
-					Type:   0xEE,
-					Length: 5,
-					Value:  []byte{0x1, 0xc1, 0x45, 0x0, 0x22},
+					Type:  0xEE,
+					Value: []byte{0x1, 0xc1, 0x45, 0x0, 0x22},
 				},
 			},
 			wantLinkID: 0x210045c1,
diff --git a/tlvparse/ssl_test.go b/tlvparse/ssl_test.go
index 0e2ce3b..e98f87b 100644
--- a/tlvparse/ssl_test.go
+++ b/tlvparse/ssl_test.go
@@ -98,23 +98,20 @@ func TestPP2SSLMarshal(t *testing.T) {
 		Verify: 0,
 		TLV: []proxyproto.TLV{
 			{
-				Type:   proxyproto.PP2_SUBTYPE_SSL_VERSION,
-				Length: len(ver),
-				Value:  []byte(ver),
+				Type:  proxyproto.PP2_SUBTYPE_SSL_VERSION,
+				Value: []byte(ver),
 			},
 			{
-				Type:   proxyproto.PP2_SUBTYPE_SSL_CN,
-				Length: len(cn),
-				Value:  []byte(cn),
+				Type:  proxyproto.PP2_SUBTYPE_SSL_CN,
+				Value: []byte(cn),
 			},
 		},
 	}
 
 	raw := []byte{0x1, 0x0, 0x0, 0x0, 0x0, 0x21, 0x0, 0x7, 0x54, 0x4c, 0x53, 0x76, 0x31, 0x2e, 0x33, 0x22, 0x0, 0xb, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x6f, 0x72, 0x67}
 	want := proxyproto.TLV{
-		Type:   proxyproto.PP2_TYPE_SSL,
-		Length: len(raw),
-		Value:  raw,
+		Type:  proxyproto.PP2_TYPE_SSL,
+		Value: raw,
 	}
 
 	tlv, err := pp2.Marshal()