From 13e8efff2556f16d7e40067271682cdb42e34f6a Mon Sep 17 00:00:00 2001 From: Lior Lieberman Date: Wed, 30 Sep 2026 16:57:00 +0000 Subject: [PATCH] envoy dyn module followup (#1995) do not merge, not a draft cause i do want ci running implements: * tie brekaing * wildcard support * added cargo test to ci * some fixes to hostname patterns and port matching tls_passhtrough is a followup. > It's a good idea to open an issue first for discussion. - [x] Tests pass - [ ] Appropriate changes to documentation are included in the PR --- .github/workflows/pr-workflow.yaml | 15 + Makefile | 5 + cmd/atenet/internal/router/egress/egress.go | 49 +- .../internal/router/egress/egress_test.go | 65 +- .../router/egresspolicymanifest_test.go | 149 +++- .../internal/router/extproc/attributes.go | 30 +- .../dynamic-modules/egress-policy/README.md | 67 +- .../dynamic-modules/egress-policy/src/lib.rs | 707 ++++++++---------- demos/egress/main.go | 14 +- demos/egress/main_test.go | 4 +- hack/test-dynamic-modules.sh | 44 ++ .../e2e/suites/egressmitm/egressmitm_test.go | 14 +- .../e2e/suites/networking/networking_test.go | 10 +- internal/egresspolicy/egresspolicy.go | 55 +- internal/egresspolicy/egresspolicy_test.go | 96 ++- .../atenet-egress-with-sdsmint.yaml | 81 +- 16 files changed, 860 insertions(+), 545 deletions(-) create mode 100755 hack/test-dynamic-modules.sh diff --git a/.github/workflows/pr-workflow.yaml b/.github/workflows/pr-workflow.yaml index 5df591fa4..1a164b3be 100644 --- a/.github/workflows/pr-workflow.yaml +++ b/.github/workflows/pr-workflow.yaml @@ -67,6 +67,21 @@ jobs: run: ../../hack/run-tool.sh gotestsum --junitfile "${ARTIFACTS}/apitool.xml" --jsonfile "${ARTIFACTS}/apitool.json" --format standard-verbose -- -race -v ./... + - name: Cache cargo + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: | + ~/.cargo/registry/index + ~/.cargo/registry/cache + ~/.cargo/git/db + cmd/dataplane/envoy/dynamic-modules/*/target + key: cargo-${{ runner.os }}-${{ hashFiles('cmd/dataplane/envoy/dynamic-modules/*/Cargo.lock') }} + restore-keys: cargo-${{ runner.os }}- + - name: dynamic module tests + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq --no-install-recommends clang libclang-dev + make test-dynamic-modules # Root-gated tests (overlay mounts, whiteout mknod, trusted.* xattrs, ...) # skip for the unprivileged runner user above; rerun the packages that # contain them (any test importing internal/roottest) under sudo. diff --git a/Makefile b/Makefile index 71869f37d..d353d9307 100644 --- a/Makefile +++ b/Makefile @@ -144,6 +144,11 @@ build-release-images: build-images build-demos build-envoy-dataplane test: $(GO) test -race ./... +# The Envoy dynamic modules are Rust. CI runs this target. +.PHONY: test-dynamic-modules +test-dynamic-modules: + hack/test-dynamic-modules.sh + .PHONY: e2e e2e: build build-demos hack/run-e2e.sh diff --git a/cmd/atenet/internal/router/egress/egress.go b/cmd/atenet/internal/router/egress/egress.go index 5402742d0..01914045a 100644 --- a/cmd/atenet/internal/router/egress/egress.go +++ b/cmd/atenet/internal/router/egress/egress.go @@ -14,10 +14,8 @@ // Package egress implements the ext_proc handler for outbound actor traffic. // It authenticates the actor behind an egress CONNECT and authorizes what goes -// through the tunnel against the actor's EgressPolicy. A request the gateway -// can read is decided the way the API says: the rules in order, over the Host -// it named and the address the actor dialed, first match wins. What the -// gateway cannot read is decided at the CONNECT, by the address alone. +// through the tunnel against the actor's EgressPolicy. The dataplane decides +// TLS at the ClientHello using the SNI rules returned on CONNECT. // // Identity comes from the actor certificate presented in the mTLS handshake, // never from a request header. On the inner legs it arrives as filter state @@ -132,12 +130,8 @@ func (h *Handler) HandleRequestHeaders(ctx context.Context, md *extproc.RequestM // certificate atunnel presented. Nothing the actor can write contributes to // the identity. // -// The tunnel opens for an actor with a policy that has rules, with nothing to -// dial: every connection is decided inside, request by request. Nothing is -// decided at the CONNECT yet, so a tls_passthrough rule cannot allow a -// connection here; until it can, the passthrough chain closes what it gets. An -// actor with no policy, or none with rules, is refused here, where there is -// still a response. +// It returns the SNI rules for the dialed port. Actors without policy rules +// are refused here. func (h *Handler) handleConnect(ctx context.Context, md *extproc.RequestMetadata, leg string) (extproc.Result, error) { // Sanity check that we were called on the Egress listener filter chain with // a CONNECT. @@ -184,30 +178,29 @@ func (h *Handler) handleConnect(ctx context.Context, md *extproc.RequestMetadata if err != nil { return extproc.Result{}, err } + rules := policy.SNIRules(dest.Port) slog.InfoContext(ctx, "egress tunnel opened: requests inside it are decided one by one", - slog.Any("actor", ref), slog.String("leg", leg), slog.String("destination", md.Host)) + slog.Any("actor", ref), slog.String("leg", leg), slog.String("destination", md.Host), slog.Int("sniRules", len(rules))) res := allow() - res.DynamicMetadata = connectMetadata(policy.HostnamePatterns()) + res.DynamicMetadata = connectMetadata(rules) return res, nil } -// connectMetadata builds the dynamic metadata returned on an allowed CONNECT: -// the policy's allowed SNI patterns under dev.ate.policy.egress. -func connectMetadata(allowedSNIs []string) *structpb.Struct { - sniValues := make([]*structpb.Value, len(allowedSNIs)) - for i, sni := range allowedSNIs { - sniValues[i] = structpb.NewStringValue(sni) +// connectMetadata encodes the SNI rules for EgressPolicyMetadataNamespace. +// An empty list denies all TLS. +func connectMetadata(rules []egresspolicy.SNIRule) *structpb.Struct { + values := make([]*structpb.Value, len(rules)) + for i, rule := range rules { + values[i] = structpb.NewStructValue(&structpb.Struct{Fields: map[string]*structpb.Value{ + extproc.EgressSNIRulePatternKey: structpb.NewStringValue(rule.Pattern), + extproc.EgressSNIRuleModeKey: structpb.NewStringValue(string(rule.Mode)), + }}) } - fields := map[string]*structpb.Value{ - extproc.EgressPolicyMetadataNamespace: structpb.NewStructValue(&structpb.Struct{ - Fields: map[string]*structpb.Value{ - extproc.EgressAllowedSNIsKey: structpb.NewListValue(&structpb.ListValue{ - Values: sniValues, - }), - }, - }), - } - return &structpb.Struct{Fields: fields} + return &structpb.Struct{Fields: map[string]*structpb.Value{ + extproc.EgressPolicyMetadataNamespace: structpb.NewStructValue(&structpb.Struct{Fields: map[string]*structpb.Value{ + extproc.EgressSNIRulesKey: structpb.NewListValue(&structpb.ListValue{Values: values}), + }}), + }} } // metadataAnswer is a one-entry answer in the egress metadata namespace. diff --git a/cmd/atenet/internal/router/egress/egress_test.go b/cmd/atenet/internal/router/egress/egress_test.go index b567d3571..6fee8c2bf 100644 --- a/cmd/atenet/internal/router/egress/egress_test.go +++ b/cmd/atenet/internal/router/egress/egress_test.go @@ -41,6 +41,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "github.com/agent-substrate/substrate/cmd/atenet/internal/router/extproc" + "github.com/agent-substrate/substrate/internal/egresspolicy" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -231,6 +232,12 @@ func httpsPolicy(patterns ...string) *ateapipb.EgressPolicy { }}} } +func httpsPolicyOnPorts(ports *ateapipb.Ports, patterns ...string) *ateapipb.EgressPolicy { + return &ateapipb.EgressPolicy{Rules: []*ateapipb.EgressRule{{ + Https: &ateapipb.HTTPSRule{Hostnames: patterns, Ports: ports}, + }}} +} + func passthroughPolicy(ports *ateapipb.Ports, patterns ...string) *ateapipb.EgressPolicy { return &ateapipb.EgressPolicy{Rules: []*ateapipb.EgressRule{{ TlsPassthrough: &ateapipb.TLSPassthroughRule{Hostnames: patterns, Ports: ports}, @@ -330,37 +337,50 @@ func passthroughDestinationOf(res extproc.Result) string { return res.DynamicMetadata.GetFields()[extproc.EgressMetadataNamespace].GetStructValue().GetFields()[extproc.EgressPassthroughDestinationKey].GetStringValue() } -// The CONNECT opens for any policy with rules, with nothing to dial: every -// connection is decided on the request legs behind it. Nothing is decided -// here yet, a tls_passthrough rule included. +// The CONNECT opens for any policy with rules and returns the https SNI rules +// for the dialed port, most specific first. func TestConnectLegOpensForAnyRules(t *testing.T) { ca := newTestCA(t, "actor-identity-ca") leaf := ca.issueActorCert(t, "spiffe://substrate-actor.local/ateom-for-actor/foo/bar", actorCertOptions{}) + mitm := func(patterns ...string) []egresspolicy.SNIRule { + rules := make([]egresspolicy.SNIRule, len(patterns)) + for i, p := range patterns { + rules[i] = egresspolicy.SNIRule{Pattern: p, Mode: egresspolicy.SNIModeMITM} + } + return rules + } tests := []struct { - name string - policy *ateapipb.EgressPolicy - wantSNIs []string + name string + policy *ateapipb.EgressPolicy + // dialed is the CONNECT authority; port 443 unless set. + dialed string + want []egresspolicy.SNIRule }{ {name: "http", policy: httpPolicy("api.example.com")}, - {name: "https", policy: httpsPolicy("api.example.com"), wantSNIs: []string{"api.example.com"}}, - {name: "tls passthrough", policy: passthroughPolicy(ports(443), "*"), wantSNIs: []string{"*"}}, - {name: "allow all", policy: allowAllPolicy(), wantSNIs: []string{"*"}}, + {name: "https", policy: httpsPolicy("api.example.com"), want: mitm("api.example.com")}, + {name: "tls passthrough", policy: passthroughPolicy(ports(443), "*")}, + {name: "allow all", policy: allowAllPolicy(), want: mitm("*")}, { - name: "multiple https and tls passthrough rules", + name: "https rules only, most specific first", policy: combined( - httpsPolicy("api.example.com", "*.example.org"), + httpsPolicy("*.example.org", "api.example.com"), httpPolicy("plain.example.com"), passthroughPolicy(ports(443), "foo.bar.com"), ), - wantSNIs: []string{"api.example.com", "*.example.org", "foo.bar.com"}, + want: mitm("api.example.com", "*.example.org"), }, + {name: "https rule on another port", policy: httpsPolicy("api.example.com"), dialed: "93.184.216.34:8443"}, + {name: "https rule on the dialed port", policy: httpsPolicyOnPorts(ports(8443), "api.example.com"), dialed: "93.184.216.34:8443", want: mitm("api.example.com")}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { h := New(&egressMockClient{actor: runningActor(), policy: tc.policy}, ca.roots(), 0, nil, "") md := egressMetadata(xfccHeader(leaf)) md.Host = "93.184.216.34:443" + if tc.dialed != "" { + md.Host = tc.dialed + } md.Headers[":authority"] = md.Host res, err := h.HandleRequestHeaders(context.Background(), md) if err != nil { @@ -369,28 +389,31 @@ func TestConnectLegOpensForAnyRules(t *testing.T) { if got := passthroughDestinationOf(res); got != "" { t.Errorf("passthrough destination = %q, want none", got) } - if got := allowedSNIsOf(t, res); !slices.Equal(got, tc.wantSNIs) { - t.Errorf("allowed SNIs = %v, want %v", got, tc.wantSNIs) + if got := sniRulesOf(t, res); !slices.Equal(got, tc.want) { + t.Errorf("SNI rules = %v, want %v", got, tc.want) } }) } } -// allowedSNIsOf reads the allowed SNI patterns a CONNECT decision handed back -// under dev.ate.policy.egress. -func allowedSNIsOf(t *testing.T, res extproc.Result) []string { +// sniRulesOf reads the SNI rules from a CONNECT result. +func sniRulesOf(t *testing.T, res extproc.Result) []egresspolicy.SNIRule { t.Helper() policyStruct := res.DynamicMetadata.GetFields()[extproc.EgressPolicyMetadataNamespace].GetStructValue() if policyStruct == nil { t.Fatalf("missing %q struct in DynamicMetadata", extproc.EgressPolicyMetadataNamespace) } - listVal := policyStruct.GetFields()[extproc.EgressAllowedSNIsKey].GetListValue() + listVal := policyStruct.GetFields()[extproc.EgressSNIRulesKey].GetListValue() if listVal == nil { - t.Fatalf("missing %q list in %q DynamicMetadata", extproc.EgressAllowedSNIsKey, extproc.EgressPolicyMetadataNamespace) + t.Fatalf("missing %q list in %q DynamicMetadata", extproc.EgressSNIRulesKey, extproc.EgressPolicyMetadataNamespace) } - out := make([]string, len(listVal.GetValues())) + out := make([]egresspolicy.SNIRule, len(listVal.GetValues())) for i, v := range listVal.GetValues() { - out[i] = v.GetStringValue() + fields := v.GetStructValue().GetFields() + out[i] = egresspolicy.SNIRule{ + Pattern: fields[extproc.EgressSNIRulePatternKey].GetStringValue(), + Mode: egresspolicy.SNIMode(fields[extproc.EgressSNIRuleModeKey].GetStringValue()), + } } return out } diff --git a/cmd/atenet/internal/router/egresspolicymanifest_test.go b/cmd/atenet/internal/router/egresspolicymanifest_test.go index 6cc16ecb7..1e6deb360 100644 --- a/cmd/atenet/internal/router/egresspolicymanifest_test.go +++ b/cmd/atenet/internal/router/egresspolicymanifest_test.go @@ -332,8 +332,7 @@ func TestEgressManifestsConnectLegDecidesThePassthroughDestination(t *testing.T) // Every inner chain without an HCM is a passthrough chain: a plain tcp_proxy // to the ORIGINAL_DST cluster, dialing the filter state the CONNECT leg's // answer produced and nothing else. The plain gateway needs one per transport -// protocol; on sdsmint egress_tls_mitm claims tls, so only raw_buffer is left. -// See TestEgressManifestsClaimEveryTransportProtocol. +// protocol; sdsmint needs one, selected by the egress-policy module. var wantPassthroughChains = map[string][]string{ egressManifests[0]: {"egress_passthrough", "egress_tls_passthrough"}, egressManifests[1]: {"egress_passthrough"}, @@ -628,3 +627,149 @@ func mustJSON(t *testing.T, n node) string { } return string(j) } + +// sdsmintManifest is the gateway that runs the egress-policy module. +var sdsmintManifest = egressManifests[1] + +// mitmListener returns the sdsmint manifest's inner listener. +func mitmListener(t *testing.T, tree node) node { + t.Helper() + l := byName(listeners(tree), "mitm_listener") + if l == nil { + t.Fatal("no mitm_listener in the sdsmint manifest") + } + return l +} + +// The matcher must select on the module's verdict and map every verdict, with +// "denied" mapping to no chain. The module runs after both inspectors, and no +// chain keeps a filter_chain_match, which Envoy ignores once a matcher is set. +func TestEgressManifestsInnerListenerSelectsOnTheModuleVerdict(t *testing.T) { + tree := bootstrapTree(t, sdsmintManifest) + l := mitmListener(t, tree) + + filters := list(l, "listener_filters") + module := filterIndex(filters, "envoy.filters.listener.dynamic_modules") + if module < 0 { + t.Fatal("mitm_listener has no dynamic_modules listener filter; nothing would write the verdict") + } + for _, inspector := range []string{"envoy.filters.listener.tls_inspector", "envoy.filters.listener.http_inspector"} { + if i := filterIndex(filters, inspector); i < 0 || i > module { + t.Errorf("%s is at listener_filters[%d], the egress-policy module at [%d]; the module would see no transport protocol or SNI", inspector, i, module) + } + } + + matcherTree := child(child(l, "filter_chain_matcher"), "matcher_tree") + if got := str(child(child(matcherTree, "input"), "typed_config"), "key"); got != extproc.EgressFilterChainFilterStateKey { + t.Errorf("filter_chain_matcher keys on filter state %q, want %q", got, extproc.EgressFilterChainFilterStateKey) + } + actions := child(child(matcherTree, "exact_match_map"), "map") + + chains := map[string]bool{} + for _, c := range list(l, "filter_chains") { + chains[str(c, "name")] = true + if child(c, "filter_chain_match") != nil { + t.Errorf("chain %q has a filter_chain_match, which Envoy ignores when the listener has a filter_chain_matcher", str(c, "name")) + } + } + for verdict, want := range map[string]string{ + extproc.EgressFilterChainMITM: extproc.EgressTLSMITMFilterChainName, + extproc.EgressFilterChainCleartext: extproc.EgressCleartextFilterChainName, + } { + got := str(child(child(child(actions, verdict), "action"), "typed_config"), "value") + if got != want { + t.Errorf("verdict %q selects chain %q, want %q", verdict, got, want) + } + if !chains[got] { + t.Errorf("verdict %q selects chain %q, which the listener does not have; the connection would be closed", verdict, got) + } + } + if action := child(child(actions, extproc.EgressFilterChainDenied), "action"); action != nil { + if got := str(child(action, "typed_config"), "value"); chains[got] { + t.Errorf("verdict %q selects chain %q; a denied connection must match no chain", extproc.EgressFilterChainDenied, got) + } + } +} + +// The outer ext_proc must accept dev.ate.policy.egress, and the outer chain +// must copy it after ext_proc into shared filter state for the module. +func TestEgressManifestsConnectLegHandsTheSNIRulesToTheInnerListener(t *testing.T) { + tree := bootstrapTree(t, sdsmintManifest) + outer := outerChain(t, tree) + cfg, extProcAt, filters := extProcOf(outer) + if cfg == nil { + t.Fatalf("chain %q has no ext_proc filter", extproc.EgressFilterChainName) + } + admitted := strs(child(child(cfg, "metadata_options"), "receiving_namespaces"), "untyped") + if !slices.Contains(admitted, extproc.EgressPolicyMetadataNamespace) { + t.Errorf("the CONNECT leg's ext_proc does not admit dynamic metadata in %q; the SNI rules would be dropped and every ClientHello denied", extproc.EgressPolicyMetadataNamespace) + } + + writers := filterStateWriters(tree, extproc.EgressPolicyMetadataNamespace) + if len(writers) != 1 { + t.Fatalf("%s is set by %d filters, want exactly the CONNECT leg's copy of its answer", extproc.EgressPolicyMetadataNamespace, len(writers)) + } + entry := writers[0] + at := -1 + for i, f := range filters { + if str(f, "name") != setFilterStateFilter { + continue + } + for _, v := range list(child(f, "typed_config"), "on_request_headers") { + if str(v, "object_key") == extproc.EgressPolicyMetadataNamespace { + at = i + } + } + } + if at < 0 { + t.Fatalf("the writer of %s is not on the outer chain, where the answer is", extproc.EgressPolicyMetadataNamespace) + } + if at < extProcAt { + t.Errorf("%s is set at http_filters[%d], before ext_proc at [%d]; the metadata it reads does not exist yet", extproc.EgressPolicyMetadataNamespace, at, extProcAt) + } + format := child(entry, "format_string") + if got := str(child(format, "text_format_source"), "inline_string"); got != extproc.EgressPolicyMetadataFormat { + t.Errorf("%s is set from %q, want %q", extproc.EgressPolicyMetadataNamespace, got, extproc.EgressPolicyMetadataFormat) + } + if got := str(entry, "factory_key"); got != "envoy.string" { + t.Errorf("%s uses factory %q, want envoy.string, the string accessor the module reads", extproc.EgressPolicyMetadataNamespace, got) + } + if skip, _ := entry["skip_if_empty"].(bool); !skip { + t.Errorf("%s is not skip_if_empty; an absent answer would be written as unparseable JSON", extproc.EgressPolicyMetadataNamespace) + } + if str(entry, "shared_with_upstream") == "" { + t.Errorf("%s is not shared with upstream; the inner listener would never see it", extproc.EgressPolicyMetadataNamespace) + } +} + +// Denied connections match no chain, so the listener access log is their only +// record. It must log only NR connections and include the SNI, actor, and +// verdict. +func TestEgressManifestsInnerListenerLogsDeniedConnections(t *testing.T) { + tree := bootstrapTree(t, sdsmintManifest) + logs := list(mitmListener(t, tree), "access_log") + if len(logs) == 0 { + t.Fatal("mitm_listener has no access_log; a ClientHello the module denies would leave no record") + } + for i, al := range logs { + if flags := strs(child(child(al, "filter"), "response_flag_filter"), "flags"); !slices.Contains(flags, "NR") { + t.Errorf("access_log[%d] is not confined to response flag NR (got %v); it would log every connection the chains already log", i, flags) + } + format := child(child(child(al, "typed_config"), "log_format"), "json_format") + for _, want := range []string{ + "%REQUESTED_SERVER_NAME%", + "%FILTER_STATE(" + extproc.ActorIdentityFilterStateKey + ":PLAIN)%", + "%FILTER_STATE(" + extproc.EgressFilterChainFilterStateKey + ":PLAIN)%", + } { + found := false + for _, v := range format { + if s, ok := v.(string); ok && strings.Contains(s, want) { + found = true + } + } + if !found { + t.Errorf("access_log[%d] does not log %s", i, want) + } + } + } +} diff --git a/cmd/atenet/internal/router/extproc/attributes.go b/cmd/atenet/internal/router/extproc/attributes.go index 7870611c0..1a69ff129 100644 --- a/cmd/atenet/internal/router/extproc/attributes.go +++ b/cmd/atenet/internal/router/extproc/attributes.go @@ -59,12 +59,29 @@ const ( // allowed it. The outer chain copies it into the ORIGINAL_DST filter state; // absent, a TLS or opaque connection has no upstream and is closed. EgressPassthroughDestinationKey = "passthrough_destination" - // EgressPolicyMetadataNamespace is the dynamic-metadata namespace carrying - // the actor's egress policy on the CONNECT leg. + // EgressPolicyMetadataNamespace holds the SNI rules returned on CONNECT. + // The outer chain copies it as JSON into filter state of the same name for + // the egress-policy module: {"rules": [{"pattern": ..., "mode": ...}]}, + // most specific first. EgressPolicyMetadataNamespace = "dev.ate.policy.egress" - // EgressAllowedSNIsKey, under EgressPolicyMetadataNamespace, is the list - // of allowed SNI patterns from the actor's egress policy. - EgressAllowedSNIsKey = "allowed_snis" + // EgressSNIRulesKey, under EgressPolicyMetadataNamespace, is the ordered + // list of rules; EgressSNIRulePatternKey and EgressSNIRuleModeKey are the + // fields of each. + EgressSNIRulesKey = "rules" + EgressSNIRulePatternKey = "pattern" + EgressSNIRuleModeKey = "mode" + + // EgressFilterChainFilterStateKey holds the egress-policy module's verdict: + // the filter chain name the sdsmint manifest's matcher selects on. + EgressFilterChainFilterStateKey = "dev.ate.egress.filter_chain" + // EgressFilterChainMITM: TLS terminated on EgressTLSMITMFilterChainName. + EgressFilterChainMITM = "mitm" + // EgressFilterChainPassthrough: forwarded unread. Unused for now. + EgressFilterChainPassthrough = "passthrough" + // EgressFilterChainCleartext: not TLS, EgressCleartextFilterChainName. + EgressFilterChainCleartext = "cleartext" + // EgressFilterChainDenied matches no chain; the connection is closed. + EgressFilterChainDenied = "denied" // EgressDialKey, under EgressMetadataNamespace, is a request leg's answer // for an allowed request: where it goes. The manifests' routes match on // it, one route per value and none without, so a request with no answer @@ -103,6 +120,9 @@ const FilterChainNameAttribute = "xds.filter_chain_name" // format string that reads EgressPassthroughDestinationKey back out. const EgressPassthroughDestinationFormat = "%DYNAMIC_METADATA(" + EgressMetadataNamespace + ":" + EgressPassthroughDestinationKey + ")%" +// EgressPolicyMetadataFormat renders EgressPolicyMetadataNamespace as JSON. +const EgressPolicyMetadataFormat = "%DYNAMIC_METADATA(" + EgressPolicyMetadataNamespace + ")%" + // OriginalDstFilterStateKey is Envoy's filter-state key for the address an // ORIGINAL_DST cluster dials. The outer CONNECT chain sets it from // EgressPassthroughDestinationKey; the request legs read it as the address the diff --git a/cmd/dataplane/envoy/dynamic-modules/egress-policy/README.md b/cmd/dataplane/envoy/dynamic-modules/egress-policy/README.md index 42b908773..a137c5b3f 100644 --- a/cmd/dataplane/envoy/dynamic-modules/egress-policy/README.md +++ b/cmd/dataplane/envoy/dynamic-modules/egress-policy/README.md @@ -1,23 +1,57 @@ # Envoy Substrate Egress Policy Implementation - Rust Dynamic Module -This directory contains an Envoy Dynamic Module written in Rust implementing a custom listener filter. +An Envoy dynamic module, written in Rust, that runs as a listener filter on +the sdsmint egress gateway's inner listener and names the filter chain each +tunneled connection belongs on. -## Overview +## What it decides -- **Extension Point:** Listener filter (`envoy.filters.listener.dynamic_modules`) -- **Filter Configuration:** Empty (`EmptyFilterConfig`) -- **Callbacks:** Implements `on_accept` evaluating EgressPolicy and returning `Continue` -- **Output:** Shared library `libenvoy_substrate_egress_policy.so` (`cdylib`) +The gateway's ext_proc sidecar answers every allowed CONNECT with the rules +that decide the tunnel's TLS: the policy's `https` rules for the port the +actor dialed, most specific first (a pattern without a wildcard before one +with, then a rule naming its ports before one naming all of them). The outer +chain copies that answer into the `dev.ate.policy.egress` filter state as +JSON, shared with the inner listener: + +```json +{"rules": [{"pattern": "api.example.com", "mode": "mitm"}, + {"pattern": "*.example.com", "mode": "mitm"}]} +``` + +After `tls_inspector` and `http_inspector` have looked at the first bytes, +this filter writes one of these verdicts to the `dev.ate.egress.filter_chain` +filter state, and the listener's `filter_chain_matcher` selects the chain by +it: + +| First bytes | Verdict | Chain | +|---|---|---| +| A ClientHello whose SNI matches a rule (first match wins; `*` matches every name, `*.suffix` exactly one label, anything else the whole name, ASCII case folded) | `mitm` | `egress_tls_mitm`: terminated with a minted leaf, decided per request | +| Any other ClientHello: no SNI, no match, no rules, unparseable rules | `denied` | none: the connection is closed | +| Not TLS | `cleartext` | `egress_cleartext`: decided per request | +| A transport protocol other than `tls` or `raw_buffer` | `denied` | none | + +`tls_passthrough` rules are not in the answer yet, so their names are closed +rather than forwarded. A connection that sends nothing before the listener +filter timeout never reaches this filter, sets no verdict, and is closed. + +The Go side of the contract is `cmd/atenet/internal/router/extproc` +(`EgressPolicyMetadataNamespace`, `EgressFilterChainFilterStateKey`) and +`internal/egresspolicy` (`SNIRules`, whose pattern grammar this filter +mirrors). The manifest tests in `cmd/atenet/internal/router` hold the +listener configuration to it. ## Building -Prerequisites: Rust toolchain (Cargo, rustc 1.75+). +Prerequisites: Rust toolchain (Cargo, rustc 1.75+), plus `clang` and +`libclang-dev` for the SDK's bindgen step. ```bash cargo build --release ``` -The compiled shared object will be located at `target/release/libenvoy_substrate_egress_policy.so`. +The compiled shared object will be located at +`target/release/libenvoy_substrate_egress_policy.so`. `cmd/dataplane/envoy/Dockerfile` +builds it the same way and packages it into the Envoy image. ## Testing @@ -25,19 +59,28 @@ The compiled shared object will be located at `target/release/libenvoy_substrate cargo test ``` -## Envoy Configuration +## Envoy configuration -To load this dynamic module as a listener filter in Envoy, add the dynamic modules filter to the listener's `listener_filters`: +Add the filter after the inspectors in the listener's `listener_filters`: ```yaml listener_filters: +- name: envoy.filters.listener.tls_inspector + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector +- name: envoy.filters.listener.http_inspector + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.listener.http_inspector.v3.HttpInspector - name: envoy.filters.listener.dynamic_modules typed_config: "@type": type.googleapis.com/envoy.extensions.filters.listener.dynamic_modules.v3.DynamicModuleListenerFilter dynamic_module_config: name: envoy_substrate_egress_policy filter_name: envoy_substrate_egress_policy - filter_config: {} ``` -Set the environment variable `ENVOY_DYNAMIC_MODULES_SEARCH_PATH` to the directory containing `libenvoy_substrate_egress_policy.so` (e.g. `export ENVOY_DYNAMIC_MODULES_SEARCH_PATH=/path/to/target/release`). +Set the environment variable `ENVOY_DYNAMIC_MODULES_SEARCH_PATH` to the +directory containing `libenvoy_substrate_egress_policy.so` (e.g. +`export ENVOY_DYNAMIC_MODULES_SEARCH_PATH=/path/to/target/release`). +`manifests/ate-install/atenet-egress-with-sdsmint.yaml` is the complete +configuration, matcher and chains included. diff --git a/cmd/dataplane/envoy/dynamic-modules/egress-policy/src/lib.rs b/cmd/dataplane/envoy/dynamic-modules/egress-policy/src/lib.rs index 3973ea63a..833db5975 100644 --- a/cmd/dataplane/envoy/dynamic-modules/egress-policy/src/lib.rs +++ b/cmd/dataplane/envoy/dynamic-modules/egress-policy/src/lib.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! The egress-policy listener filter picks the filter chain for a tunneled +//! connection. See the README in this directory. + use envoy_proxy_dynamic_modules_rust_sdk::{ abi::envoy_dynamic_module_type_on_listener_filter_status, declare_listener_filter_init_functions, envoy_log_trace, EnvoyListenerFilter, @@ -19,103 +22,144 @@ use envoy_proxy_dynamic_modules_rust_sdk::{ }; use serde::Deserialize; -/// Key of the filter state object holding the Substrate egress policy. +/// Filter state holding the SNI rules as JSON. See +/// EgressPolicyMetadataNamespace in cmd/atenet/internal/router/extproc. pub const ATE_POLICY_EGRESS: &[u8] = b"dev.ate.policy.egress"; -/// Key of the filter state object holding the SNI passthrough match result. +/// Filter state this filter writes the chosen chain name to. pub const ATE_EGRESS_FILTER_CHAIN: &[u8] = b"dev.ate.egress.filter_chain"; -/// Filter chain name for MITM traffic. +/// Verdict for TLS an https rule allows. pub const ATE_EGRESS_FILTER_CHAIN_MITM: &str = "mitm"; -/// Filter chain name for cleartext traffic. +/// Verdict for anything that is not TLS. pub const ATE_EGRESS_FILTER_CHAIN_CLEARTEXT: &str = "cleartext"; -/// Filter chain name when request is denied by policy. -/// Since there is no filter chain with this name, TCP connection will be reset. -pub const ATE_EGRESS_FILTER_CHAIN_NONE: &str = "denied"; +/// Verdict for a denied connection. No chain has this name. +pub const ATE_EGRESS_FILTER_CHAIN_DENIED: &str = "denied"; +/// Mode of a rule whose match terminates the connection. +pub const SNI_MODE_MITM: &str = "mitm"; -/// Parsed Substrate egress policy from the `ATE_POLICY_EGRESS` filter state JSON. -#[derive(Debug, Deserialize)] +/// Transport protocol tls_inspector sets for TLS. +const TRANSPORT_TLS: &str = "tls"; + +/// Transport protocol Envoy assumes when no inspector detected one. +const TRANSPORT_RAW_BUFFER: &str = "raw_buffer"; + +/// The SNI rules for a connection. +#[derive(Debug, Deserialize, PartialEq)] pub struct EgressPolicy { - pub allowed_snis: Vec, + /// Most specific first; the first match wins. + pub rules: Vec, } -/// Empty filter configuration for the listener filter. -pub struct EmptyFilterConfig; +/// A pattern and the mode applied when it matches. +#[derive(Debug, Deserialize, PartialEq)] +pub struct SniRule { + pub pattern: String, + pub mode: String, +} -impl ListenerFilterConfig for EmptyFilterConfig { - fn new_listener_filter(&self, _envoy: &mut ELF) -> Box> { - Box::new(EmptyListenerFilter) +/// Reports whether a normalized `hostname` matches `pattern`. Must agree with +/// HostnamePattern.Matches in internal/egresspolicy. +pub fn pattern_matches(pattern: &str, hostname: &str) -> bool { + if hostname.is_empty() { + return false; + } + if pattern == "*" { + return true; + } + if let Some(suffix) = pattern.strip_prefix("*.") { + return match hostname + .strip_suffix(suffix) + .and_then(|rest| rest.strip_suffix('.')) + { + Some(label) => !label.is_empty() && !label.contains('.'), + None => false, + }; + } + pattern == hostname +} + +/// ASCII-lowercases an SNI and strips one trailing dot, as the gateway does. +fn normalize_sni(sni: &str) -> String { + let lower = sni.to_ascii_lowercase(); + match lower.strip_suffix('.') { + Some(stripped) => stripped.to_owned(), + None => lower, } } -/// A listener filter that matches requested server name (SNI) against the SNI passthrough policy. -pub struct EmptyListenerFilter; +/// Returns the verdict from the transport alone, or None for TLS, which is +/// decided by SNI. Unknown transports are denied. +pub fn transport_verdict(transport: Option<&str>) -> Option<&'static str> { + match transport { + Some(TRANSPORT_TLS) => None, + None | Some(TRANSPORT_RAW_BUFFER) => Some(ATE_EGRESS_FILTER_CHAIN_CLEARTEXT), + Some(_) => Some(ATE_EGRESS_FILTER_CHAIN_DENIED), + } +} -impl ListenerFilter for EmptyListenerFilter { +/// Returns the mode of the first rule matching the SNI, or denied. +pub fn tls_verdict(policy: Option<&EgressPolicy>, sni: Option<&str>) -> &'static str { + let (Some(policy), Some(sni)) = (policy, sni) else { + return ATE_EGRESS_FILTER_CHAIN_DENIED; + }; + let hostname = normalize_sni(sni); + match policy + .rules + .iter() + .find(|rule| pattern_matches(&rule.pattern, &hostname)) + { + Some(rule) if rule.mode == SNI_MODE_MITM => ATE_EGRESS_FILTER_CHAIN_MITM, + _ => ATE_EGRESS_FILTER_CHAIN_DENIED, + } +} + +/// The filter takes no configuration. +pub struct EgressPolicyFilterConfig; + +impl ListenerFilterConfig for EgressPolicyFilterConfig { + fn new_listener_filter(&self, _envoy: &mut ELF) -> Box> { + Box::new(EgressPolicyFilter) + } +} + +/// Runs after tls_inspector and http_inspector, once per connection. +pub struct EgressPolicyFilter; + +impl ListenerFilter for EgressPolicyFilter { fn on_accept( &mut self, envoy_filter: &mut ELF, ) -> envoy_dynamic_module_type_on_listener_filter_status { - let transport_protocol_str = envoy_filter + let transport = envoy_filter .get_detected_transport_protocol() - .map(|transport_protocol| { - String::from_utf8_lossy(transport_protocol.as_slice()).into_owned() - }); - envoy_log_trace!("transport_protocol: {:#?} : {}", transport_protocol_str, transport_protocol_str.as_deref() != Some("tls")); + .map(|value| String::from_utf8_lossy(value.as_slice()).into_owned()); - if transport_protocol_str.as_deref() != Some("tls") - { - // TODO(yanavlasov): allow plaintext traffic only if there are `http` rules in the policy - envoy_filter.set_filter_state_bytes( - ATE_EGRESS_FILTER_CHAIN, - ATE_EGRESS_FILTER_CHAIN_CLEARTEXT.as_bytes(), - ); - envoy_log_trace!( - "dev.ate.egress.filter_chain: {}", - ATE_EGRESS_FILTER_CHAIN_CLEARTEXT - ); - return envoy_dynamic_module_type_on_listener_filter_status::Continue; - } - - let server_name_str = envoy_filter - .get_requested_server_name() - .map(|server_name| { - String::from_utf8_lossy(server_name.as_slice()).into_owned() - }); - - let sni_passthrough_policy_str = envoy_filter - .get_filter_state_bytes(ATE_POLICY_EGRESS) - .map(|sni_passthrough_policy| { - String::from_utf8_lossy(sni_passthrough_policy.as_slice()).into_owned() - }); - - let egress_policy = sni_passthrough_policy_str - .as_deref() - .and_then(|policy_str| serde_json::from_str::(policy_str).ok()); - - let comparison_result = match (&server_name_str, &egress_policy) { - (Some(server_name), Some(policy)) - if policy - .allowed_snis - .iter() - // TODO(yanavlasov): implement wildcard matching. - .any(|sni| server_name.eq_ignore_ascii_case(sni)) => - { - ATE_EGRESS_FILTER_CHAIN_MITM + let verdict = match transport_verdict(transport.as_deref()) { + Some(verdict) => verdict, + None => { + let sni = envoy_filter + .get_requested_server_name() + .map(|value| String::from_utf8_lossy(value.as_slice()).into_owned()); + // Unparseable rules deny, like absent ones. + let policy = envoy_filter + .get_filter_state_bytes(ATE_POLICY_EGRESS) + .and_then(|value| serde_json::from_slice::(value.as_slice()).ok()); + let verdict = tls_verdict(policy.as_ref(), sni.as_deref()); + envoy_log_trace!("egress policy: sni={:?} chain={}", sni, verdict); + verdict } - // TODO(yanavlasov): implement passthrough TLS policy. - _ => ATE_EGRESS_FILTER_CHAIN_NONE, }; - envoy_filter.set_filter_state_bytes( - ATE_EGRESS_FILTER_CHAIN, - comparison_result.as_bytes(), + envoy_filter.set_filter_state_bytes(ATE_EGRESS_FILTER_CHAIN, verdict.as_bytes()); + envoy_log_trace!( + "egress policy: transport={:?} chain={}", + transport, + verdict ); - envoy_log_trace!("dev.ate.egress.filter_chain: {}", comparison_result); - envoy_dynamic_module_type_on_listener_filter_status::Continue } } @@ -136,7 +180,7 @@ fn new_listener_filter_config_fn< _name: &str, _config: &[u8], ) -> Option>> { - Some(Box::new(EmptyFilterConfig)) + Some(Box::new(EgressPolicyFilterConfig)) } #[cfg(test)] @@ -146,353 +190,252 @@ mod tests { EnvoyBuffer, MockEnvoyListenerFilter, MockEnvoyListenerFilterConfig, }; + fn policy(rules: &[(&str, &str)]) -> EgressPolicy { + EgressPolicy { + rules: rules + .iter() + .map(|(pattern, mode)| SniRule { + pattern: pattern.to_string(), + mode: mode.to_string(), + }) + .collect(), + } + } + + #[test] + fn test_pattern_matches() { + let cases: &[(&str, &str, bool)] = &[ + // "*" matches every name and only names. + ("*", "anything.example.com", true), + ("*", "localhost", true), + ("*", "", false), + // "*.suffix" matches exactly one non-empty leftmost label. + ("*.example.com", "api.example.com", true), + ("*.example.com", "example.com", false), + ("*.example.com", "a.b.example.com", false), + ("*.example.com", ".example.com", false), + ("*.example.com", "xexample.com", false), + ("*.example.com", "api.example.co", false), + ("*.example.com", "api.example.com.evil", false), + // Anything else matches the whole name. + ("api.example.com", "api.example.com", true), + ("api.example.com", "www.api.example.com", false), + ("api.example.com", "api.example.co", false), + ("example.com", "api.example.com", false), + // Normalization is the caller's job. + ("api.example.com", "API.example.com", false), + ("api.example.com", "api.example.com.", false), + ]; + for (pattern, hostname, want) in cases { + assert_eq!( + pattern_matches(pattern, hostname), + *want, + "pattern_matches({pattern:?}, {hostname:?})" + ); + } + } + + #[test] + fn test_normalize_sni() { + assert_eq!(normalize_sni("API.Example.COM."), "api.example.com"); + assert_eq!(normalize_sni("api.example.com"), "api.example.com"); + // One trailing dot, not every one. + assert_eq!(normalize_sni("api.example.com.."), "api.example.com."); + // Only ASCII folds: U+212A KELVIN SIGN stays itself and cannot become "k". + assert_eq!(normalize_sni("\u{212A}.example.com"), "\u{212A}.example.com"); + } + + #[test] + fn test_transport_verdict() { + assert_eq!(transport_verdict(Some("tls")), None); + assert_eq!(transport_verdict(None), Some(ATE_EGRESS_FILTER_CHAIN_CLEARTEXT)); + assert_eq!( + transport_verdict(Some("raw_buffer")), + Some(ATE_EGRESS_FILTER_CHAIN_CLEARTEXT) + ); + assert_eq!(transport_verdict(Some("quic")), Some(ATE_EGRESS_FILTER_CHAIN_DENIED)); + assert_eq!(transport_verdict(Some("")), Some(ATE_EGRESS_FILTER_CHAIN_DENIED)); + } + + #[test] + fn test_tls_verdict_denies_without_inputs() { + let p = policy(&[("*", SNI_MODE_MITM)]); + assert_eq!(tls_verdict(None, Some("api.example.com")), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_eq!(tls_verdict(Some(&p), None), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_eq!(tls_verdict(Some(&p), Some("")), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_eq!( + tls_verdict(Some(&policy(&[])), Some("api.example.com")), + ATE_EGRESS_FILTER_CHAIN_DENIED + ); + } + + #[test] + fn test_tls_verdict_matches_wildcards() { + let p = policy(&[("api.example.com", SNI_MODE_MITM), ("*.example.org", SNI_MODE_MITM)]); + assert_eq!(tls_verdict(Some(&p), Some("api.example.com")), ATE_EGRESS_FILTER_CHAIN_MITM); + assert_eq!(tls_verdict(Some(&p), Some("API.EXAMPLE.COM.")), ATE_EGRESS_FILTER_CHAIN_MITM); + assert_eq!(tls_verdict(Some(&p), Some("www.example.org")), ATE_EGRESS_FILTER_CHAIN_MITM); + assert_eq!(tls_verdict(Some(&p), Some("example.org")), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_eq!(tls_verdict(Some(&p), Some("a.b.example.org")), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_eq!(tls_verdict(Some(&p), Some("www.example.com")), ATE_EGRESS_FILTER_CHAIN_DENIED); + + let any = policy(&[("*", SNI_MODE_MITM)]); + assert_eq!(tls_verdict(Some(&any), Some("whatever.test")), ATE_EGRESS_FILTER_CHAIN_MITM); + } + + #[test] + fn test_tls_verdict_first_match_decides() { + // The first match is final, even with an unknown mode. + let unknown_first = policy(&[("*.example.com", "not-a-mode"), ("api.example.com", SNI_MODE_MITM)]); + assert_eq!( + tls_verdict(Some(&unknown_first), Some("api.example.com")), + ATE_EGRESS_FILTER_CHAIN_DENIED + ); + let known_first = policy(&[("api.example.com", SNI_MODE_MITM), ("*.example.com", "not-a-mode")]); + assert_eq!( + tls_verdict(Some(&known_first), Some("api.example.com")), + ATE_EGRESS_FILTER_CHAIN_MITM + ); + assert_eq!( + tls_verdict(Some(&known_first), Some("www.example.com")), + ATE_EGRESS_FILTER_CHAIN_DENIED + ); + } + + #[test] + fn test_policy_json_shape() { + let parsed: EgressPolicy = serde_json::from_str( + r#"{"rules":[{"pattern":"api.example.com","mode":"mitm"},{"pattern":"*","mode":"mitm"}]}"#, + ) + .unwrap(); + assert_eq!(parsed, policy(&[("api.example.com", "mitm"), ("*", "mitm")])); + assert!(serde_json::from_str::(r#"{"allowed_snis":["api.example.com"]}"#).is_err()); + } + + fn new_filter_config() -> Box> { + let mut mock_config = MockEnvoyListenerFilterConfig::new(); + new_listener_filter_config_fn::( + &mut mock_config, + "envoy_substrate_egress_policy", + b"", + ) + .expect("a filter config") + } + + /// Runs on_accept for a TLS connection with the given SNI and policy filter + /// state, asserting the verdict written. + fn assert_tls_verdict(sni: Option<&'static [u8]>, policy: Option<&'static [u8]>, want: &'static str) { + let config = new_filter_config(); + let mut mock_filter = MockEnvoyListenerFilter::new(); + mock_filter + .expect_get_detected_transport_protocol() + .returning(|| Some(EnvoyBuffer::new(b"tls"))); + mock_filter + .expect_get_requested_server_name() + .returning(move || sni.map(EnvoyBuffer::new)); + mock_filter + .expect_get_filter_state_bytes() + .withf(|key| key == ATE_POLICY_EGRESS) + .returning(move |_| policy.map(EnvoyBuffer::new)); + mock_filter + .expect_set_filter_state_bytes() + .withf(move |key, value| key == ATE_EGRESS_FILTER_CHAIN && value == want.as_bytes()) + .times(1) + .returning(|_, _| true); + + let mut filter = config.new_listener_filter(&mut mock_filter); + assert_eq!( + filter.on_accept(&mut mock_filter), + envoy_dynamic_module_type_on_listener_filter_status::Continue + ); + } + + /// Runs on_accept for a connection with the given detected transport, + /// asserting the verdict written. + fn assert_transport_verdict(transport: Option<&'static [u8]>, want: &'static str) { + let config = new_filter_config(); + let mut mock_filter = MockEnvoyListenerFilter::new(); + mock_filter + .expect_get_detected_transport_protocol() + .returning(move || transport.map(EnvoyBuffer::new)); + mock_filter + .expect_set_filter_state_bytes() + .withf(move |key, value| key == ATE_EGRESS_FILTER_CHAIN && value == want.as_bytes()) + .times(1) + .returning(|_, _| true); + + let mut filter = config.new_listener_filter(&mut mock_filter); + assert_eq!( + filter.on_accept(&mut mock_filter), + envoy_dynamic_module_type_on_listener_filter_status::Continue + ); + assert_eq!( + filter.on_data(&mut mock_filter, 0), + envoy_dynamic_module_type_on_listener_filter_status::Continue + ); + } + #[test] fn test_init() { assert!(init()); } #[test] - fn test_empty_listener_filter_lifecycle() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b""); - assert!(config.is_some()); - let config = config.unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| None); - mock_filter - .expect_get_requested_server_name() - .returning(|| None); - mock_filter - .expect_get_filter_state_bytes() - .returning(|_| None); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"cleartext" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); - - let status = filter.on_data(&mut mock_filter, 0); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + fn test_on_accept_no_transport_is_cleartext() { + assert_transport_verdict(None, ATE_EGRESS_FILTER_CHAIN_CLEARTEXT); } #[test] - fn test_on_accept_raw_buffer_transport_protocol() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"raw_buffer"))); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"cleartext" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + fn test_on_accept_raw_buffer_is_cleartext() { + assert_transport_verdict(Some(b"raw_buffer"), ATE_EGRESS_FILTER_CHAIN_CLEARTEXT); } #[test] - fn test_on_accept_matching_sni_and_policy() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); + fn test_on_accept_unknown_transport_is_denied() { + assert_transport_verdict(Some(b"quic"), ATE_EGRESS_FILTER_CHAIN_DENIED); + } - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"www.google.com"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| { - Some(EnvoyBuffer::new( - br#"{"allowed_snis":["api.google.com","www.google.com"]}"#, - )) - }); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"mitm" - }) - .times(1) - .returning(|_, _| true); + const RULES: &[u8] = + br#"{"rules":[{"pattern":"api.google.com","mode":"mitm"},{"pattern":"*.google.com","mode":"mitm"}]}"#; - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + #[test] + fn test_on_accept_matching_sni() { + assert_tls_verdict(Some(b"api.google.com"), Some(RULES), ATE_EGRESS_FILTER_CHAIN_MITM); } #[test] - fn test_on_accept_case_insensitive_matching_sni_and_policy() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"WWW.Google.COM"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| { - Some(EnvoyBuffer::new(br#"{"allowed_snis":["www.google.com"]}"#)) - }); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"mitm" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + fn test_on_accept_wildcard_sni() { + assert_tls_verdict(Some(b"mail.google.com"), Some(RULES), ATE_EGRESS_FILTER_CHAIN_MITM); } #[test] - fn test_on_accept_mismatched_sni_and_policy() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"www.google.com"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| { - Some(EnvoyBuffer::new( - br#"{"allowed_snis":["api.google.com","mail.google.com"]}"#, - )) - }); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"denied" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + fn test_on_accept_case_insensitive_sni() { + assert_tls_verdict(Some(b"API.Google.COM"), Some(RULES), ATE_EGRESS_FILTER_CHAIN_MITM); } #[test] - fn test_on_accept_empty_allowed_snis() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); + fn test_on_accept_mismatched_sni() { + assert_tls_verdict(Some(b"google.com"), Some(RULES), ATE_EGRESS_FILTER_CHAIN_DENIED); + assert_tls_verdict(Some(b"www.example.com"), Some(RULES), ATE_EGRESS_FILTER_CHAIN_DENIED); + } - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"www.google.com"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| Some(EnvoyBuffer::new(br#"{"allowed_snis":[]}"#))); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"denied" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + #[test] + fn test_on_accept_no_rules() { + assert_tls_verdict(Some(b"api.google.com"), Some(br#"{"rules":[]}"#), ATE_EGRESS_FILTER_CHAIN_DENIED); } #[test] fn test_on_accept_invalid_policy_json() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"www.google.com"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| Some(EnvoyBuffer::new(b"not-json"))); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"denied" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + assert_tls_verdict(Some(b"api.google.com"), Some(b"not-json"), ATE_EGRESS_FILTER_CHAIN_DENIED); } #[test] fn test_on_accept_missing_policy() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| Some(EnvoyBuffer::new(b"www.google.com"))); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| None); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"denied" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + assert_tls_verdict(Some(b"api.google.com"), None, ATE_EGRESS_FILTER_CHAIN_DENIED); } #[test] fn test_on_accept_missing_sni() { - let mut mock_config = MockEnvoyListenerFilterConfig::new(); - let config = new_listener_filter_config_fn::< - MockEnvoyListenerFilterConfig, - MockEnvoyListenerFilter, - >(&mut mock_config, "envoy_substrate_egress_policy", b"") - .unwrap(); - - let mut mock_filter = MockEnvoyListenerFilter::new(); - mock_filter - .expect_get_detected_transport_protocol() - .returning(|| Some(EnvoyBuffer::new(b"tls"))); - mock_filter - .expect_get_requested_server_name() - .returning(|| None); - mock_filter - .expect_get_filter_state_bytes() - .withf(|key| key == ATE_POLICY_EGRESS) - .returning(|_| { - Some(EnvoyBuffer::new(br#"{"allowed_snis":["www.google.com"]}"#)) - }); - mock_filter - .expect_set_filter_state_bytes() - .withf(|key, value| { - key == ATE_EGRESS_FILTER_CHAIN && value == b"denied" - }) - .times(1) - .returning(|_, _| true); - - let mut filter = config.new_listener_filter(&mut mock_filter); - - let status = filter.on_accept(&mut mock_filter); - assert_eq!( - status, - envoy_dynamic_module_type_on_listener_filter_status::Continue - ); + assert_tls_verdict(None, Some(RULES), ATE_EGRESS_FILTER_CHAIN_DENIED); } } - diff --git a/demos/egress/main.go b/demos/egress/main.go index d42f90b7d..09c7b2677 100644 --- a/demos/egress/main.go +++ b/demos/egress/main.go @@ -19,7 +19,6 @@ package main import ( "context" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -105,13 +104,6 @@ func main() { } func newHandler(client *http.Client) http.Handler { - httpsTransport := http.DefaultTransport.(*http.Transport).Clone() - httpsTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - httpsClient := *client - if httpsClient.Transport == nil { - httpsClient.Transport = httpsTransport - } - mux := http.NewServeMux() mux.HandleFunc("/readyz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) @@ -143,11 +135,7 @@ func newHandler(client *http.Client) http.Handler { if traceparent := r.Header.Get("traceparent"); traceparent != "" { outbound.Header.Set("traceparent", traceparent) } - doClient := client - if outbound.URL.Scheme == "https" { - doClient = &httpsClient - } - response, err := doClient.Do(outbound) + response, err := client.Do(outbound) if err != nil { writeJSON(w, http.StatusBadGateway, fetchResponse{Error: fmt.Sprintf("request failed: %v", err)}) return diff --git a/demos/egress/main_test.go b/demos/egress/main_test.go index 2360ef732..095252982 100644 --- a/demos/egress/main_test.go +++ b/demos/egress/main_test.go @@ -80,7 +80,9 @@ func TestFetchHTTPAndHTTPS(t *testing.T) { })) defer httpsSrv.Close() - handler := newHandler(&http.Client{Timeout: requestTimeout}) + // The demo verifies origins with the roots its client is given: in the + // cluster the projected trust bundle, here the test server's own. + handler := newHandler(httpsSrv.Client()) for _, tc := range []struct { name string diff --git a/hack/test-dynamic-modules.sh b/hack/test-dynamic-modules.sh new file mode 100755 index 000000000..5359c1b27 --- /dev/null +++ b/hack/test-dynamic-modules.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash + +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Runs the Rust unit tests of every Envoy dynamic module. Needs cargo, clang, +# and libclang-dev. Fails if a module runs no tests. + +set -o errexit -o nounset -o pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "${ROOT}" + +if ! command -v cargo >/dev/null 2>&1; then + echo "cargo not found: install a Rust toolchain (https://rustup.rs) with clang and libclang-dev" >&2 + exit 1 +fi + +status=0 +for manifest in cmd/dataplane/envoy/dynamic-modules/*/Cargo.toml; do + module="$(dirname "${manifest}")" + echo "==> cargo test in ${module}" + log="$(mktemp)" + if ! cargo test --locked --manifest-path "${manifest}" 2>&1 | tee "${log}"; then + status=1 + fi + if ! grep -Eq 'test result: ok\. [1-9][0-9]* passed' "${log}"; then + echo "${module}: no tests ran" >&2 + status=1 + fi + rm -f "${log}" +done +exit "${status}" diff --git a/internal/e2e/suites/egressmitm/egressmitm_test.go b/internal/e2e/suites/egressmitm/egressmitm_test.go index 0b7069c76..576f95734 100644 --- a/internal/e2e/suites/egressmitm/egressmitm_test.go +++ b/internal/e2e/suites/egressmitm/egressmitm_test.go @@ -129,11 +129,17 @@ func TestActorEgressMITMTrust(t *testing.T) { t.Errorf("fetch with system roots failed, but not with a certificate-verification error: %s", neg.Error) } - // The policy names example.com only, so another host is refused. Encapsulated - // TLS connection is closed since SNI does not match the policy. + // A host outside the policy is closed at the ClientHello: expect a + // transport error, not a certificate error or an HTTP status. The error + // text varies, so only its presence is checked. denied := probeFetch(t, ctx, rc, id, "https://example.org/", "bundle") - if denied.Error != "Get \"https://example.org/\": EOF" { - t.Errorf("fetch of a host outside the policy did not fail at the transport. Error: %s. Status: %s", denied.Error, denied.Status) + switch { + case denied.Error == "": + t.Errorf("fetch of a host outside the policy succeeded with status %s, want the connection closed at the ClientHello", denied.Status) + case strings.Contains(denied.Error, "certificate") || strings.Contains(denied.Error, "x509"): + t.Errorf("fetch of a host outside the policy was intercepted (certificate error %q), want the connection closed at the ClientHello", denied.Error) + case denied.Status != "": + t.Errorf("fetch of a host outside the policy got status %s with error %q, want no HTTP exchange at all", denied.Status, denied.Error) } } diff --git a/internal/e2e/suites/networking/networking_test.go b/internal/e2e/suites/networking/networking_test.go index b4f1c97f4..339665917 100644 --- a/internal/e2e/suites/networking/networking_test.go +++ b/internal/e2e/suites/networking/networking_test.go @@ -144,12 +144,12 @@ func TestActorEgress(t *testing.T) { } // TestActorEgressHTTPS covers the same path as TestActorEgress with a TLS -// origin, where the gateway cannot see inside the request. atenet-egress -// authorizes the CONNECT against the Actor's actor-identity certificate and -// then relays raw TCP: it never decrypts, so the TLS session runs end to end -// between the Actor and the origin. +// origin, through the sdsmint gateway's MITM. The plain gateway closes all TLS, +// so this runs only against sdsmint. func TestActorEgressHTTPS(t *testing.T) { - t.Skip("TODO: the gateway does not forward TLS unread yet; it intercepts every connection, so end-to-end TLS with the origin cannot hold") + if !egressMITM() { + t.Skip("covers the sdsmint gateway; set E2E_EGRESS_MITM") + } ctx := context.Background() fixture := egressFixture() actorAtespace, actorName, _ := createAndResumeActorWithEgress(t, ctx, "egress-https", fixture, e2e.EgressAllowAll()...) diff --git a/internal/egresspolicy/egresspolicy.go b/internal/egresspolicy/egresspolicy.go index 7ca169ed1..337d82a4d 100644 --- a/internal/egresspolicy/egresspolicy.go +++ b/internal/egresspolicy/egresspolicy.go @@ -16,10 +16,8 @@ // destination. ateapi validates patterns with the same parser the gateway // matches with, so the two cannot drift. // -// The gateway does not decide at the ClientHello yet: every TLS connection is -// intercepted, and a tls_passthrough rule matches nothing until it does. The -// rules are decided per request, on the authority, plus the SNI of the -// connection for https. +// Requests are decided here; TLS connections are decided by the dataplane +// against SNIRules. tls_passthrough rules match nothing for now. // // The package is pure: no I/O, no logging. package egresspolicy @@ -165,19 +163,54 @@ func (r compiledRule) matchesPort(port uint16) bool { return r.anyPort || slices.Contains(r.ports, port) } -// HostnamePatterns returns all compiled SNI patterns from the policy's https -// and tls_passthrough rules, in rule order. -func (p *Policy) HostnamePatterns() []string { - var patterns []string +// SNIMode is how a TLS connection is handled when an SNIRule matches. Values +// must match cmd/dataplane/envoy/dynamic-modules/egress-policy. +type SNIMode string + +// SNIModeMITM terminates TLS and decides each request inside. +const SNIModeMITM SNIMode = "mitm" + +// SNIRule is an SNI pattern and the mode applied when it matches first. +type SNIRule struct { + Pattern string + Mode SNIMode +} + +// SNIRules returns the https rules for port, most specific first: exact names +// before wildcards, then named ports before all ports. Ties keep policy order. +// tls_passthrough rules are left out so their names are denied, not +// intercepted. +func (p *Policy) SNIRules(port uint16) []SNIRule { + type ranked struct { + rule SNIRule + rank matchRank + } + var entries []ranked for _, rule := range p.rules { - if rule.protocol != protocolHTTPS && rule.protocol != protocolTLSPassthrough { + if rule.protocol != protocolHTTPS || !rule.matchesPort(port) { continue } for _, pattern := range rule.patterns { - patterns = append(patterns, pattern.String()) + entries = append(entries, ranked{ + rule: SNIRule{Pattern: pattern.String(), Mode: SNIModeMITM}, + rank: matchRank{name: pattern.rank(), port: rule.portRank()}, + }) } } - return patterns + slices.SortStableFunc(entries, func(a, b ranked) int { + switch { + case a.rank.beats(b.rank): + return -1 + case b.rank.beats(a.rank): + return 1 + } + return 0 + }) + rules := make([]SNIRule, len(entries)) + for i, e := range entries { + rules[i] = e.rule + } + return rules } // EvaluateRequest decides one request the gateway can read, on the name or diff --git a/internal/egresspolicy/egresspolicy_test.go b/internal/egresspolicy/egresspolicy_test.go index 8e8650c69..0a40d40b9 100644 --- a/internal/egresspolicy/egresspolicy_test.go +++ b/internal/egresspolicy/egresspolicy_test.go @@ -16,6 +16,7 @@ package egresspolicy import ( "net/netip" + "slices" "testing" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" @@ -369,6 +370,19 @@ func TestEvaluateRequest(t *testing.T) { dest: host("api.example.com"), want: Decision{Allowed: true, RuleIndex: 0}, }, + { + // A rule ranks by its best matching pattern. + name: "a rule holding star and an exact name wins the exact name", + policy: policy(httpRule("*", "api.google.com"), httpRule("*.google.com")), + dest: host("api.google.com"), + want: Decision{Allowed: true, RuleIndex: 0}, + }, + { + name: "a rule holding star and an exact name loses other names to a labeled wildcard", + policy: policy(httpRule("*", "api.google.com"), httpRule("*.google.com")), + dest: host("admin.google.com"), + want: Decision{Allowed: true, RuleIndex: 1}, + }, { name: "empty rule matches nothing", policy: policy(&ateapipb.EgressRule{}, httpRule("example.com")), @@ -385,34 +399,76 @@ func TestEvaluateRequest(t *testing.T) { } } -func TestHostnamePatterns(t *testing.T) { +func httpsRuleOnPorts(ports *ateapipb.Ports, patterns ...string) *ateapipb.EgressRule { + return &ateapipb.EgressRule{Https: &ateapipb.HTTPSRule{Hostnames: patterns, Ports: ports}} +} + +func TestSNIRules(t *testing.T) { + mitm := func(patterns ...string) []SNIRule { + rules := make([]SNIRule, len(patterns)) + for i, p := range patterns { + rules[i] = SNIRule{Pattern: p, Mode: SNIModeMITM} + } + return rules + } tests := []struct { name string policy *ateapipb.EgressPolicy - want []string + port uint16 + want []SNIRule }{ - {name: "no rules", policy: &ateapipb.EgressPolicy{}}, - {name: "http only", policy: policy(httpRule("api.example.com"))}, - {name: "https rule", policy: policy(httpsRule("api.example.com", "*.example.org")), want: []string{"api.example.com", "*.example.org"}}, - {name: "tls_passthrough rule", policy: policy(passthroughRule(ports(443), "tls.example.com", "*")), want: []string{"tls.example.com", "*"}}, - {name: "https and tls_passthrough mixed with http", policy: policy( - httpsRule("api.example.com"), - httpRule("plain.example.com"), - passthroughRule(ports(443), "*.example.org", "foo.bar.com"), - ), want: []string{"api.example.com", "*.example.org", "foo.bar.com"}}, - {name: "invalid patterns dropped", policy: policy(httpsRule("good.example.com", "not a hostname")), want: []string{"good.example.com"}}, + {name: "no rules", policy: &ateapipb.EgressPolicy{}, port: 443}, + {name: "http rules decide requests, not connections", policy: policy(httpRule("api.example.com")), port: 80}, + {name: "tls_passthrough is not decided yet", policy: policy(passthroughRule(ports(443), "tls.example.com", "*")), port: 443}, + {name: "https on its default port", policy: policy(httpsRule("api.example.com", "*.example.org")), port: 443, want: mitm("api.example.com", "*.example.org")}, + {name: "https default port covers no other", policy: policy(httpsRule("api.example.com")), port: 8443}, + {name: "https on a named port", policy: policy(httpsRuleOnPorts(ports(8443, 9443), "api.example.com")), port: 9443, want: mitm("api.example.com")}, + {name: "https on all ports", policy: policy(httpsRuleOnPorts(allPorts(), "api.example.com")), port: 12345, want: mitm("api.example.com")}, + { + name: "only https rules, only those for the dialed port", + policy: policy( + httpsRule("api.example.com"), + httpsRuleOnPorts(ports(8443), "alt.example.com"), + httpRule("plain.example.com"), + passthroughRule(ports(443), "pinned.example.com"), + ), + port: 443, + want: mitm("api.example.com"), + }, + { + // Name specificity outranks port specificity. + name: "most specific first", + policy: policy( + httpsRule("*"), + httpsRuleOnPorts(allPorts(), "a.example.com"), + httpsRule("*.example.com"), + httpsRule("b.example.com"), + httpsRuleOnPorts(allPorts(), "*.example.org"), + ), + port: 443, + want: mitm("b.example.com", "a.example.com", "*.example.com", "*.example.org", "*"), + }, + { + // Patterns rank individually, not per rule, matching + // EvaluateRequest. + name: "patterns of one rule are split by rank", + policy: policy(httpsRule("*", "api.google.com"), httpsRule("*.google.com")), + port: 443, + want: mitm("api.google.com", "*.google.com", "*"), + }, + { + name: "ties keep policy order", + policy: policy(httpsRule("b.example.com", "a.example.com"), httpsRule("c.example.com")), + port: 443, + want: mitm("b.example.com", "a.example.com", "c.example.com"), + }, + {name: "invalid patterns dropped", policy: policy(httpsRule("good.example.com", "not a hostname")), port: 443, want: mitm("good.example.com")}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { compiled, _ := Compile(tc.policy) - got := compiled.HostnamePatterns() - if len(got) != len(tc.want) { - t.Fatalf("HostnamePatterns() = %v, want %v", got, tc.want) - } - for i := range got { - if got[i] != tc.want[i] { - t.Errorf("HostnamePatterns()[%d] = %q, want %q", i, got[i], tc.want[i]) - } + if got := compiled.SNIRules(tc.port); !slices.Equal(got, tc.want) { + t.Errorf("SNIRules(%d) = %v, want %v", tc.port, got, tc.want) } }) } diff --git a/manifests/ate-install/atenet-egress-with-sdsmint.yaml b/manifests/ate-install/atenet-egress-with-sdsmint.yaml index bc44d2022..3d122ee4d 100644 --- a/manifests/ate-install/atenet-egress-with-sdsmint.yaml +++ b/manifests/ate-install/atenet-egress-with-sdsmint.yaml @@ -183,10 +183,9 @@ data: inline_string: "%DOWNSTREAM_PEER_URI_SAN%" omit_empty_values: true # The CONNECT leg. The sidecar authenticates the actor from its - # certificate and decides the tls_passthrough rules against the - # CONNECT authority (see cmd/atenet/internal/router/egress). It - # answers with dev.ate.egress:passthrough_destination when one allowed - # the connection. Fails closed if the router is down. + # certificate (see cmd/atenet/internal/router/egress) and returns + # the SNI rules in dev.ate.policy.egress. Fails closed if the + # router is down. - name: envoy.filters.http.ext_proc typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.ext_proc.v3.ExternalProcessor @@ -244,6 +243,8 @@ data: text_format_source: inline_string: "%DYNAMIC_METADATA(dev.ate.egress:passthrough_destination)%" omit_empty_values: true + # SNI rules for the egress-policy module. If absent, all TLS + # is denied. - object_key: dev.ate.policy.egress factory_key: envoy.string skip_if_empty: true @@ -267,13 +268,10 @@ data: # tunnelled TLS with a leaf sdsmint mints for the SNI, reads the real # Host, and re-originates. # - # Three chains, selected by what the tunnel actually carries. - # tls_inspector tags a ClientHello "tls" and anything else "raw_buffer". - # http_inspector then splits "raw_buffer" again, because "not TLS" is not - # the same claim as "HTTP": without it, SSH and every other non-HTTP - # protocol lands on an HTTP connection manager that parses the first bytes - # of the stream as a request line, finds no request, and drops the - # connection with nothing in the access log to say why. + # The egress-policy module (cmd/dataplane/envoy/dynamic-modules/egress-policy) + # writes the chain name to dev.ate.egress.filter_chain: TLS matching an + # SNI rule goes to MITM, other TLS is denied, and non-TLS goes to + # cleartext. "denied" matches no chain, so the connection is closed. # # The two HTTP chains are named so ext_proc can tell them apart # (xds.filter_chain_name; the names must match @@ -291,8 +289,7 @@ data: # The passthrough chain is a tcp_proxy, so it has no HTTP filters of its # own, and it decides nothing: it dials the address the CONNECT leg # allowed, carried here as filter state, and closes when there is none. - # Nothing allows one yet: the CONNECT leg does not decide - # tls_passthrough rules until the gateway decides at the ClientHello. + # Nothing selects it yet. # --------------------------------------------------------------------- - name: mitm_listener stat_prefix: mitm @@ -300,16 +297,34 @@ data: # Both inspectors work by peeking at bytes the client sends first, so on # a server-speaks-first protocol -- SSH, SMTP, MySQL -- they wait for a # client that is itself waiting for the origin, and the tunnel deadlocks - # until the actor's own timeout fires. Giving up quickly and continuing - # is the only way out: continue_on_listener_filters_timeout hands the - # socket to the chains with no transport protocol detected, and Envoy - # defaults that to raw_buffer, which is exactly the passthrough chain - # such a connection belongs on. The cost is paid only by connections + # until the actor's own timeout fires. Giving up quickly is the only way + # out: on timeout the module is skipped, no chain matches, and the + # connection is closed. The cost is paid only by connections # that send nothing; a client that speaks is classified immediately. One # second is far longer than a local actor needs to emit a ClientHello, # and short enough to stay well inside the probe timeouts. listener_filters_timeout: 1s continue_on_listener_filters_timeout: true + # Logs connections that matched no chain (NR). A "denied" chain is a + # policy denial; an empty one is a timeout. + access_log: + - name: envoy.access_loggers.file + filter: + response_flag_filter: + flags: ["NR"] + typed_config: + "@type": type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + log_format: + json_format: + leg: clienthello + time: "%START_TIME%" + actor: "%FILTER_STATE(dev.ate.actor.identity:PLAIN)%" + sni: "%REQUESTED_SERVER_NAME%" + chain: "%FILTER_STATE(dev.ate.egress.filter_chain:PLAIN)%" + destination: "%FILTER_STATE(dev.ate.connect.authority:PLAIN)%" + flags: "%RESPONSE_FLAGS%" + details: "%RESPONSE_CODE_DETAILS%" listener_filters: - name: envoy.filters.listener.tls_inspector typed_config: @@ -571,13 +586,8 @@ data: # The cleartext chain. Nothing to terminate and nothing to mint: the # Host header is already in the clear, so this leg reads the # destination directly rather than from a certificate it issued. - # - # application_protocols is what confines it to traffic http_inspector - # actually recognised as HTTP. + # Non-HTTP streams fail in the codec. - name: egress_cleartext - filter_chain_match: - transport_protocol: raw_buffer - application_protocols: ["http/1.0", "http/1.1", "h2c"] filters: - name: envoy.filters.network.http_connection_manager typed_config: @@ -694,18 +704,9 @@ data: typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router - # The passthrough chain: everything the inspectors could not claim. It - # matches raw_buffer rather than nothing, because Envoy buckets filter - # chains by transport protocol and never falls back out of a populated - # bucket: egress_cleartext owns the raw_buffer bucket with its HTTP - # application protocols, so a chain matching nothing would be - # unreachable. Nothing here decides: tcp_proxy dials the - # original_dst_address filter state the CONNECT leg produced, and with - # none present the connection is closed before a byte is relayed - # (flags UH in the log). + # The passthrough chain, unused for now. tcp_proxy dials + # original_dst_address and closes the connection if it is unset (UH). - name: egress_passthrough - filter_chain_match: - transport_protocol: raw_buffer filters: - name: envoy.filters.network.tcp_proxy typed_config: @@ -750,13 +751,11 @@ data: explicit_http_config: http_protocol_options: {} # Copies every filter state object shared with the upstream onto the - # internal listener's downstream connection. Two objects ride this hop, - # both set on the CONNECT leg above: dev.ate.actor.identity, which the - # policy ext_proc and the MITM access logs read, and - # original_dst_address, present only when an address rule allowed the - # original destination, which the passthrough chain dials. + # internal listener's downstream connection: dev.ate.actor.identity, + # dev.ate.connect.authority, dev.ate.policy.egress, and + # original_dst_address, all set on the CONNECT leg above. # - # Without this neither reaches mitm_listener, and nothing reports it: + # Without this none of them reaches mitm_listener, and nothing reports it: # the request proxies and every read resolves to the zero value, which # looks exactly like the filters above not running. The wrapped socket # is required and is raw_buffer because there is no TLS on this hop --