Hash namespace+proxy ID when creating socket path (#17204)

UNIX domain socket paths are limited to 104-108 characters, depending on
the OS. This limit was quite easy to exceed when testing the feature on
Kubernetes, due to how proxy IDs encode the Pod ID eg:
metrics-collector-59467bcb9b-fkkzl-hcp-metrics-collector-sidecar-proxy

To ensure we stay under that character limit this commit makes a
couple changes:
- Use a b64 encoded SHA1 hash of the namespace + proxy ID to create a
  short and deterministic socket file name.
- Add validation to proxy registrations and proxy-defaults to enforce a
  limit on the socket directory length.
This commit is contained in:
Freddy 2023-05-09 12:20:26 -06:00 committed by GitHub
parent 270df96301
commit 0459069523
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 205 additions and 113 deletions

View File

@ -5,6 +5,8 @@ package proxycfg
import (
"context"
"crypto/sha1"
"encoding/base64"
"fmt"
"path"
"strings"
@ -660,8 +662,14 @@ func (s *handlerConnectProxy) maybeInitializeHCPMetricsWatches(ctx context.Conte
// The path includes the proxy ID so that when multiple proxies are on the same host
// they each have a distinct path to send their metrics.
sock := fmt.Sprintf("%s_%s.sock", s.proxyID.NamespaceOrDefault(), s.proxyID.ID)
path := path.Join(hcpCfg.HCPMetricsBindSocketDir, sock)
id := s.proxyID.NamespaceOrDefault() + "_" + s.proxyID.ID
// UNIX domain sockets paths have a max length of 108, so we take a hash of the compound ID
// to limit the length of the socket path.
h := sha1.New()
h.Write([]byte(id))
hash := base64.RawURLEncoding.EncodeToString(h.Sum(nil))
path := path.Join(hcpCfg.HCPMetricsBindSocketDir, hash+".sock")
upstream := structs.Upstream{
DestinationNamespace: acl.DefaultNamespaceName,

View File

@ -3722,7 +3722,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
DestinationNamespace: "default",
DestinationPartition: "default",
DestinationName: apimod.HCPMetricsCollectorName,
LocalBindSocketPath: "/tmp/consul/hcp-metrics/default_web-sidecar-proxy.sock",
LocalBindSocketPath: "/tmp/consul/hcp-metrics/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock",
Config: map[string]interface{}{
"protocol": "grpc",
},

View File

@ -468,6 +468,10 @@ func (e *ProxyConfigEntry) Validate() error {
return err
}
if err := validateOpaqueProxyConfig(e.Config); err != nil {
return fmt.Errorf("Config: %w", err)
}
if err := envoyextensions.ValidateExtensions(e.EnvoyExtensions.ToAPI()); err != nil {
return err
}
@ -1329,6 +1333,17 @@ func (c *ConfigEntryResponse) UnmarshalBinary(data []byte) error {
return nil
}
func validateOpaqueProxyConfig(config map[string]interface{}) error {
// This max is chosen to stay under the 104 character limit on OpenBSD, FreeBSD, MacOS.
// It assumes the socket's filename is fixed at 32 characters.
const maxSocketDirLen = 70
if path, _ := config["envoy_hcp_metrics_bind_socket_dir"].(string); len(path) > maxSocketDirLen {
return fmt.Errorf("envoy_hcp_metrics_bind_socket_dir length %d exceeds max %d", len(path), maxSocketDirLen)
}
return nil
}
func validateConfigEntryMeta(meta map[string]string) error {
var err error
if len(meta) > metaMaxKeyPairs {

View File

@ -3261,6 +3261,15 @@ func TestProxyConfigEntry(t *testing.T) {
EnterpriseMeta: *acl.DefaultEnterpriseMeta(),
},
},
"proxy config entry has invalid opaque config": {
entry: &ProxyConfigEntry{
Name: "global",
Config: map[string]interface{}{
"envoy_hcp_metrics_bind_socket_dir": "/Consul/is/a/networking/platform/that/enables/securing/your/networking/",
},
},
validateErr: "Config: envoy_hcp_metrics_bind_socket_dir length 71 exceeds max",
},
"proxy config has invalid failover policy": {
entry: &ProxyConfigEntry{
Name: "global",
@ -3445,3 +3454,33 @@ func uintPointer(v uint32) *uint32 {
func durationPointer(d time.Duration) *time.Duration {
return &d
}
func TestValidateOpaqueConfigMap(t *testing.T) {
tt := map[string]struct {
input map[string]interface{}
expectErr string
}{
"hcp metrics socket dir is valid": {
input: map[string]interface{}{
"envoy_hcp_metrics_bind_socket_dir": "/etc/consul.d/hcp"},
expectErr: "",
},
"hcp metrics socket dir is too long": {
input: map[string]interface{}{
"envoy_hcp_metrics_bind_socket_dir": "/Consul/is/a/networking/platform/that/enables/securing/your/networking/",
},
expectErr: "envoy_hcp_metrics_bind_socket_dir length 71 exceeds max 70",
},
}
for name, tc := range tt {
t.Run(name, func(t *testing.T) {
err := validateOpaqueProxyConfig(tc.input)
if tc.expectErr != "" {
require.ErrorContains(t, err, tc.expectErr)
return
}
require.NoError(t, err)
})
}
}

View File

@ -1508,6 +1508,10 @@ func (s *NodeService) ValidateForAgent() error {
"A Proxy cannot also be Connect Native, only typical services"))
}
if err := validateOpaqueProxyConfig(s.Proxy.Config); err != nil {
result = multierror.Append(result, fmt.Errorf("Proxy.Config: %w", err))
}
// ensure we don't have multiple upstreams for the same service
var (
upstreamKeys = make(map[UpstreamKey]struct{})

View File

@ -826,6 +826,16 @@ func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) {
"",
},
{
"connect-proxy: invalid opaque config",
func(x *NodeService) {
x.Proxy.Config = map[string]interface{}{
"envoy_hcp_metrics_bind_socket_dir": "/Consul/is/a/networking/platform/that/enables/securing/your/networking/",
}
},
"Proxy.Config: envoy_hcp_metrics_bind_socket_dir length 71 exceeds max",
},
{
"connect-proxy: no Proxy.DestinationServiceName",
func(x *NodeService) { x.Proxy.DestinationServiceName = "" },

View File

@ -28,10 +28,10 @@
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"name": "hcp-metrics-collector:/tmp/consul/hcp-metrics/default_web-sidecar-proxy.sock",
"name": "hcp-metrics-collector:/tmp/consul/hcp-metrics/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock",
"address": {
"pipe": {
"path": "/tmp/consul/hcp-metrics/default_web-sidecar-proxy.sock"
"path": "/tmp/consul/hcp-metrics/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock"
}
},
"filterChains": [

View File

@ -5,6 +5,8 @@ package envoy
import (
"bytes"
"crypto/sha1"
"encoding/base64"
"encoding/json"
"fmt"
"net"
@ -14,7 +16,6 @@ import (
"strings"
"text/template"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/api"
)
@ -811,13 +812,28 @@ func (c *BootstrapConfig) generateListenerConfig(args *BootstrapTplArgs, bindAdd
return nil
}
// appendHCPMetricsConfig generates config to enable a socket at path: <hcpMetricsBindSocketDir>/<namespace>_<proxy_id>.sock
// or <hcpMetricsBindSocketDir>/<proxy_id>.sock, if namespace is empty.
// appendHCPMetricsConfig generates config to enable a socket at path: <hcpMetricsBindSocketDir>/<hash of compound proxy ID>.sock
// We take the hash of the compound proxy ID for a few reasons:
//
// - The proxy ID is included because this socket path must be unique per proxy. Each Envoy proxy will ship
// its metrics to HCP using its own loopback listener at this path.
//
// - The hash is needed because UNIX domain socket paths must be less than 104 characters. By using a b64 encoded
// SHA1 hash we end up with 27 chars for the name, 5 chars for the extension, and the remainder is saved for
// the configurable socket dir. The length of the directory's path is validated on writes to avoid going over.
func appendHCPMetricsConfig(args *BootstrapTplArgs, hcpMetricsBindSocketDir string) {
// Normalize namespace to "default". This ensures we match the namespace behaviour in proxycfg package,
// where a dynamic listener will be created at the same socket path via xDS.
sock := fmt.Sprintf("%s_%s.sock", acl.NamespaceOrDefault(args.Namespace), args.ProxyID)
path := path.Join(hcpMetricsBindSocketDir, sock)
ns := args.Namespace
if ns == "" {
ns = "default"
}
id := ns + "_" + args.ProxyID
h := sha1.New()
h.Write([]byte(id))
hash := base64.RawURLEncoding.EncodeToString(h.Sum(nil))
path := path.Join(hcpMetricsBindSocketDir, hash+".sock")
if args.StatsSinksJSON != "" {
args.StatsSinksJSON += ",\n"

View File

@ -556,7 +556,7 @@ const (
"endpoint": {
"address": {
"pipe": {
"path": "/tmp/consul/hcp-metrics/default_web-sidecar-proxy.sock"
"path": "/tmp/consul/hcp-metrics/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock"
}
}
}
@ -654,7 +654,7 @@ func TestBootstrapConfig_ConfigureArgs(t *testing.T) {
"endpoint": {
"address": {
"pipe": {
"path": "/tmp/consul/hcp-metrics/default_web-sidecar-proxy.sock"
"path": "/tmp/consul/hcp-metrics/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock"
}
}
}

View File

@ -67,7 +67,7 @@
"endpoint": {
"address": {
"pipe": {
"path": "/tmp/consul/hcp-metrics/default_test-proxy.sock"
"path": "/tmp/consul/hcp-metrics/k3bWnyJyKvjUYXrBdOX2nXzSSCQ.sock"
}
}
}