Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/pr-workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,32 @@ jobs:
# TLS is intercepted, their passthrough assumptions
# (TestActorEgressHTTPS's end-to-end TLS with the origin) no longer hold.
run: hack/install-ate-kind.sh --deploy-atenet --atenet-dataplane=${{ matrix.dataplane }} --experimental-use-sdsmint
- name: Run E2E tests (egress certificate identity, sdsmint)
run: hack/run-e2e-kind.sh ./internal/e2e/suites/egressauthz -count=1 -v -args --no-color
- name: Verify egress certificate chain producer mutation (Envoy)
if: matrix.dataplane == 'envoy'
run: |
set -euo pipefail
kube=(kubectl --context kind-kind)
original_data="$(mktemp)"
leaf_data="$(mktemp)"
"${kube[@]}" -n ate-system get configmap atenet-egress -o json | jq '{data:.data}' > "$original_data"
jq -e '.data["envoy.yaml"] | contains("urlEncodedPemEncodedPeerCertificateChain()")' "$original_data" >/dev/null
jq '.data["envoy.yaml"] |= sub("urlEncodedPemEncodedPeerCertificateChain"; "urlEncodedPemEncodedPeerCertificate") | {data:.data}' "$original_data" > "$leaf_data"
restore() {
"${kube[@]}" -n ate-system patch configmap atenet-egress --type merge --patch-file "$original_data" >/dev/null
"${kube[@]}" -n ate-system rollout restart deployment/atenet-egress >/dev/null
"${kube[@]}" -n ate-system rollout status deployment/atenet-egress --timeout=180s
rm -f "$original_data" "$leaf_data"
}
trap restore EXIT
"${kube[@]}" -n ate-system patch configmap atenet-egress --type merge --patch-file "$leaf_data" >/dev/null
"${kube[@]}" -n ate-system rollout restart deployment/atenet-egress >/dev/null
"${kube[@]}" -n ate-system rollout status deployment/atenet-egress --timeout=180s
E2E_EGRESS_CERTIFICATE_LEAF_ONLY=1 hack/run-e2e-kind.sh ./internal/e2e/suites/egressauthz -run '^TestGatewayCertificateMetadataTransport$' -count=1 -v -args --no-color
restore
trap - EXIT
hack/run-e2e-kind.sh ./internal/e2e/suites/egressauthz -run '^TestGatewayCertificateMetadataTransport$' -count=1 -v -args --no-color
- name: Run E2E tests (egress MITM trust)
# The consumption half of the trust-bundle chain: an actor does TLS with
# the MITM gateway's minted leaf using ONLY the projected bundle, plus a
Expand Down
142 changes: 16 additions & 126 deletions cmd/atenet/internal/router/egress/egress.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,9 @@ import (
const (
// agentgatewayClientCertificateAttribute is the PEM peer certificate agentgateway
// computes from the downstream TLS connection for ext_proc.
agentgatewayClientCertificateAttribute = "source.certificate"
// forwardedClientCertHeader is the header Envoy fills in with details of
// the mTLS peer, including the PEM chain it validated. The egress filter
// chain sets forward_client_cert_details: SANITIZE_SET, so whatever a
// client sends under this name is discarded and replaced by Envoy's own
// value.
//
// This is the only channel that can carry a whole certificate to ext_proc:
// the CEL request attributes Envoy exposes (subject, SANs, SHA-256 digest)
// cannot express the custom ActorIdentity X.509 extension this gateway
// authorizes on.
forwardedClientCertHeader = "x-forwarded-client-cert"
// xfccChainKey is the x-forwarded-client-cert key holding the URL-encoded
// PEM of the full presented chain, leaf first.
xfccChainKey = "chain"
agentgatewayClientCertificateAttribute = "source.certificate"
envoyClientCertificateMetadataNamespace = "dev.ate.egress.peer_certificate"
envoyClientCertificateChainKey = "chain"
)

// deniedBody is the body of every policy denial. The reason goes to the log,
Expand Down Expand Up @@ -321,22 +309,26 @@ func (h *Handler) validateActor(ctx context.Context, identity *substratex509.Act
// on the request into a verified ActorIdentity, or an error describing why it
// cannot be trusted.
func (h *Handler) authenticateActorCertificate(md *extproc.RequestMetadata) (*substratex509.ActorIdentity, error) {
if peer, present := md.DynamicMetadata[envoyClientCertificateMetadataNamespace]; present {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review this trust boundary together with the metadata propagation changes in this commit: when Envoy supplies this namespace, its chain should remain authoritative, and malformed or incomplete metadata must fail closed rather than fall back to the AgentGateway leaf-only attribute. In particular, verify the full chain survives the Envoy FilterMetadata → ext_proc → authenticateActorCertificate path.

encoded := peer.GetFields()[envoyClientCertificateChainKey].GetStringValue()
chainPEM, err := url.PathUnescape(encoded)
if err != nil {
return nil, fmt.Errorf("decoding the client certificate chain: %w", err)
}
chain, err := parseCertificateChainPEM([]byte(chainPEM))
if err != nil {
return nil, err
}
return h.verifyActorCertificate(chain)
}
if certificate := md.Attribute(agentgatewayClientCertificateAttribute); certificate != "" {
chain, err := parseCertificateChainPEM([]byte(certificate))
if err != nil {
return nil, err
}
return h.verifyActorCertificate(chain)
}
header := md.Header(forwardedClientCertHeader)
if header == "" {
return nil, fmt.Errorf("request carries no %s header", forwardedClientCertHeader)
}
chain, err := parseXFCCChain(header)
if err != nil {
return nil, err
}
return h.verifyActorCertificate(chain)
return nil, fmt.Errorf("request carries no trusted peer certificate")
}

// verifyActorCertificate checks that chain[0] is a live, non-CA, client-auth
Expand Down Expand Up @@ -415,32 +407,6 @@ func (h *Handler) verifyActorCertificate(chain []*x509.Certificate) (*substratex
return identity, nil
}

// parseXFCCChain extracts the presented certificate chain, leaf first, from an
// x-forwarded-client-cert header value.
func parseXFCCChain(header string) ([]*x509.Certificate, error) {
// One element per proxy hop. SANITIZE_SET makes Envoy the only writer, so
// anything but exactly one element means either an unexpected proxy in front
// of the gateway or a listener that lost SANITIZE_SET — in both cases we no
// longer know which element describes our actual peer, so refuse to guess.
elements := splitXFCCUnquoted(header, ',')
if len(elements) != 1 {
return nil, fmt.Errorf("expected exactly one %s element, got %d", forwardedClientCertHeader, len(elements))
}
encoded, ok := xfccValue(elements[0], xfccChainKey)
if !ok {
return nil, fmt.Errorf("%s carries no %q value", forwardedClientCertHeader, xfccChainKey)
}
// Envoy percent-encodes the PEM. PathUnescape, not QueryUnescape: base64
// bodies contain '+', and query unescaping would decode it to a space and
// silently corrupt the DER.
chainPEM, err := url.PathUnescape(encoded)
if err != nil {
return nil, fmt.Errorf("decoding the client certificate chain: %w", err)
}

return parseCertificateChainPEM([]byte(chainPEM))
}

func parseCertificateChainPEM(chainPEM []byte) ([]*x509.Certificate, error) {
var chain []*x509.Certificate
rest := chainPEM
Expand All @@ -465,82 +431,6 @@ func parseCertificateChainPEM(chainPEM []byte) ([]*x509.Certificate, error) {
return chain, nil
}

// xfccValue returns the value of key in one x-forwarded-client-cert element.
// Keys are matched case-insensitively; Envoy emits "Chain", but the header is
// consumed by enough different proxies that assuming its casing is not worth
// the failure mode.
func xfccValue(element, key string) (string, bool) {
for _, pair := range splitXFCCUnquoted(element, ';') {
k, v, found := strings.Cut(pair, "=")
if !found || !strings.EqualFold(strings.TrimSpace(k), key) {
continue
}
return unquoteXFCC(strings.TrimSpace(v)), true
}
return "", false
}

// splitXFCCUnquoted splits on sep, ignoring separators inside a quoted value.
// x-forwarded-client-cert quotes any value containing its own delimiters, which
// the PEM ones always do.
func splitXFCCUnquoted(s string, sep rune) []string {
var parts []string
var current strings.Builder
quoted := false
escaped := false
for _, r := range s {
switch {
case escaped:
current.WriteRune(r)
escaped = false
case quoted && r == '\\':
current.WriteRune(r)
escaped = true
case r == '"':
quoted = !quoted
current.WriteRune(r)
case r == sep && !quoted:
parts = append(parts, current.String())
current.Reset()
default:
current.WriteRune(r)
}
}
parts = append(parts, current.String())

trimmed := make([]string, 0, len(parts))
for _, part := range parts {
if part = strings.TrimSpace(part); part != "" {
trimmed = append(trimmed, part)
}
}
return trimmed
}

// unquoteXFCC strips the surrounding quotes from an x-forwarded-client-cert
// value and undoes the backslash escaping inside them.
func unquoteXFCC(value string) string {
if len(value) < 2 || !strings.HasPrefix(value, `"`) || !strings.HasSuffix(value, `"`) {
return value
}
inner := value[1 : len(value)-1]
var out strings.Builder
escaped := false
for _, r := range inner {
if escaped {
out.WriteRune(r)
escaped = false
continue
}
if r == '\\' {
escaped = true
continue
}
out.WriteRune(r)
}
return out.String()
}

// mapEgressIdentityError converts a GetActor failure into a client-facing
// ext_proc denial. An unknown actor is treated as forbidden (the actor was
// deleted out from under a still-valid certificate); transient control-plane
Expand Down
Loading
Loading