Skip to content

Commit

Permalink
Hash namespace+proxy ID when creating socket path (#17204)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
freddygv committed May 9, 2023
1 parent cb73282 commit 0752fc2
Show file tree
Hide file tree
Showing 10 changed files with 205 additions and 113 deletions.
12 changes: 10 additions & 2 deletions agent/proxycfg/connect_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package proxycfg

import (
"context"
"crypto/sha1"
"encoding/base64"
"fmt"
"path"
"strings"
Expand Down Expand Up @@ -657,8 +659,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,
Expand Down
2 changes: 1 addition & 1 deletion agent/proxycfg/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3710,7 +3710,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",
},
Expand Down
15 changes: 15 additions & 0 deletions agent/structs/config_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,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
}
Expand Down Expand Up @@ -1281,6 +1285,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 {
Expand Down
39 changes: 39 additions & 0 deletions agent/structs/config_entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3195,6 +3195,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 access log type": {
entry: &ProxyConfigEntry{
Name: "global",
Expand Down Expand Up @@ -3356,3 +3365,33 @@ func testConfigEntryNormalizeAndValidate(t *testing.T, cases map[string]configEn
func uintPointer(v uint32) *uint32 {
return &v
}

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)
})
}
}
4 changes: 4 additions & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,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{})
Expand Down
10 changes: 10 additions & 0 deletions agent/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,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 = "" },
Expand Down
Loading

0 comments on commit 0752fc2

Please sign in to comment.