mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-10-02 03:24:42 +08:00
egress: make verify-egress-demo.sh robust and executable
The gateway access-log assertion read the log once, immediately after curl
returned, and failed if it saw nothing new. That is racy twice over:
* Envoy emits the CONNECT access-log entry asynchronously, so for an
external destination it can land seconds after the actor's response.
* The Actor's HTTP client keeps the tunnel alive. A repeat fetch to a host
it already reached rides the open tunnel and produces no new entry at
all, so a run against a warm actor failed even though egress was working.
Poll for the new entry, and fall back to any tunnel already open for this
actor's SAN before declaring failure. Also read -c envoy explicitly (the pod
also runs the ext-proc sidecar) and mark the script executable, as every other
directly-invoked script in hack/ is.
This commit is contained in:
Lior Lieberman
@@ -202,31 +202,14 @@ func (h *Handler) authenticateActorCertificate(md *extproc.RequestMetadata) (*su
|
||||
// actor certificate issued by the actor-identity CA, and returns the single
|
||||
// ActorIdentity it carries.
|
||||
//
|
||||
// ###########################################################################
|
||||
// SEE(lior): READ THIS IF YOU ARE WONDERING WHY WE VERIFY THE CHAIN TWICE.
|
||||
//
|
||||
// The egress listener already does full mTLS: require_client_certificate with
|
||||
// the actor-identity CA as its trusted_ca, so Envoy refuses the handshake for
|
||||
// anything this function would also reject on chain, expiry, or signature. The
|
||||
// re-verification below is therefore redundant *today*, and it is here on
|
||||
// purpose:
|
||||
//
|
||||
// - Envoy validates the chain but cannot look at the ActorIdentity
|
||||
// extension. This function has to parse the certificate regardless, and
|
||||
// parsing an unverified certificate and then trusting its contents is the
|
||||
// failure mode that keeps producing CVEs. Verifying what we parse keeps the
|
||||
// trust decision in one place instead of split across a YAML file and a Go
|
||||
// file.
|
||||
// - It makes the handler safe under Envoy config drift. Someone relaxing
|
||||
// require_client_certificate, widening trusted_ca, or putting another proxy
|
||||
// in front should not silently turn this into an unauthenticated endpoint.
|
||||
// - It costs one signature verification per CONNECT, not per request: the
|
||||
// tunnel is established once and then carries raw TCP.
|
||||
//
|
||||
// If you decide the Envoy-side check is authoritative and this is dead weight,
|
||||
// this function is the thing to delete — but keep the ActorIdentity extraction
|
||||
// and the IsCA/EKU/purpose checks below it, because Envoy does none of those.
|
||||
// ###########################################################################
|
||||
// The chain is verified here even though Envoy already did it at the handshake
|
||||
// (require_client_certificate with the actor-identity CA as trusted_ca). We have
|
||||
// to parse the certificate anyway to read the ActorIdentity extension, which
|
||||
// Envoy cannot see, and trusting a parsed-but-unverified certificate is a
|
||||
// well-worn source of CVEs. It also keeps the handler safe if the Envoy config
|
||||
// is ever loosened, and costs one signature check per CONNECT rather than per
|
||||
// request. The IsCA, ClientAuth-EKU, and purpose checks below have no Envoy-side
|
||||
// equivalent at all.
|
||||
func (h *Handler) verifyActorCertificate(chain []*x509.Certificate) (*substratex509.ActorIdentity, error) {
|
||||
leaf := chain[0]
|
||||
intermediates := x509.NewCertPool()
|
||||
|
||||
@@ -124,13 +124,6 @@ fi
|
||||
##############################################################################
|
||||
log "NEGATIVE — a pod identity is not an actor identity (expect a refused handshake)"
|
||||
##############################################################################
|
||||
# SEE(lior): this used to be a 403 assertion. On the header-based design the
|
||||
# gateway trusted any podidentity holder's mTLS and let ext_proc adjudicate the
|
||||
# X-Ate-* headers it asserted, so a probe pod could reach the CONNECT and be
|
||||
# denied there. The gateway now trusts only the actor-identity CA, so the same
|
||||
# probe never gets past the TLS handshake — the denial moved a layer down and
|
||||
# there is no CONNECT response to read a status code out of. Assert the
|
||||
# handshake failure instead; it is the stronger property.
|
||||
${K} apply -f - >/dev/null <<'YAML'
|
||||
apiVersion: v1
|
||||
kind: Pod
|
||||
|
||||
+6
-17
@@ -280,24 +280,13 @@ create_actor_id_ca_pool_secret() {
|
||||
--secret-namespace=ate-system
|
||||
}
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SEE(lior): actor-identity CA trust bundle for the egress PEP.
|
||||
#
|
||||
# The egress gateway has to verify actor client certificates, which means it
|
||||
# needs the actor-identity CA *root* — and only the root. The authoritative
|
||||
# copy today is the actor-id-ca-pool Secret, but its pool.json also carries the
|
||||
# CA signing key, so mounting that Secret into the gateway would put the key
|
||||
# that mints every actor identity inside a pod that only ever needs to verify
|
||||
# them. This derives a cert-only Secret instead, following exactly the pattern
|
||||
# create_valkey_ca_certs_secret already uses for the signer roots.
|
||||
# needs the actor-identity CA root. actor-id-ca-pool Secret containts both
|
||||
# root and CA signing key. This derives a cert-only Secret instead, following
|
||||
# exactly the pattern create_valkey_ca_certs_secret already uses for the
|
||||
# signer roots.
|
||||
#
|
||||
# TODO(liorlieberman): revisit. The other CAs reach their consumers as
|
||||
# ClusterTrustBundles published by the podcertificate controller, and the
|
||||
# actor-identity CA arguably should too — then the gateway would project a
|
||||
# trust bundle like it already does for servicedns/podidentity and this
|
||||
# install-time Secret would go away. Doing that needs a signer/controller path
|
||||
# that does not exist yet, so this is the interim shape.
|
||||
# ---------------------------------------------------------------------------
|
||||
# TODO(liorlieberman): should this be published as ClusterTrustBundles?
|
||||
create_actor_id_ca_certs_secret() {
|
||||
log_step "create_actor_id_ca_certs_secret"
|
||||
# Extract into its own variable first: errexit cannot see a substitution fail
|
||||
@@ -459,7 +448,7 @@ ensure_apiserver_prerequisites() {
|
||||
|| create_jwt_authority_pool_secret
|
||||
run_kubectl get secret -n ate-system actor-id-ca-pool >/dev/null 2>&1 \
|
||||
|| create_actor_id_ca_pool_secret
|
||||
# SEE(lior): derived from actor-id-ca-pool above, so it must come after it.
|
||||
# Derived from actor-id-ca-pool above, so it must come after it.
|
||||
run_kubectl get secret -n ate-system actor-id-ca-certs >/dev/null 2>&1 \
|
||||
|| create_actor_id_ca_certs_secret
|
||||
run_kubectl get secret -n podcertificate-controller-system service-dns-ca-pool >/dev/null 2>&1 \
|
||||
|
||||
Regular → Executable
+28
-7
@@ -44,7 +44,9 @@ kubectl-ate --context "${CTX}" create actor "${ACTOR}" \
|
||||
${K} -n ate-system wait --for=condition=Ready "actor/${ACTOR}" 2>/dev/null || sleep 10
|
||||
|
||||
echo "== snapshot gateway log offset =="
|
||||
BEFORE=$(${K} -n ate-system logs deployment/atenet-egress --tail=-1 2>/dev/null | wc -l | tr -d ' ')
|
||||
# -c envoy explicitly: the gateway pod also runs the ext-proc sidecar, and the
|
||||
# [egress] access log belongs to Envoy.
|
||||
BEFORE=$(${K} -n ate-system logs deployment/atenet-egress -c envoy --tail=-1 2>/dev/null | wc -l | tr -d ' ')
|
||||
|
||||
echo "== drive actor egress: GET ${TARGET_URL} via the actor =="
|
||||
${K} -n ate-system port-forward service/atenet-router 18000:80 >/tmp/pf.log 2>&1 &
|
||||
@@ -57,10 +59,29 @@ RESP=$(curl -s -o /dev/null -w "%{http_code}" -X POST http://localhost:18000/ \
|
||||
echo "actor round-trip HTTP ${RESP} (200 = the actor fetched ${TARGET_URL} through egress)"
|
||||
|
||||
echo "== NEW egress gateway access log lines (proof of CONNECT+mTLS+identity) =="
|
||||
${K} -n ate-system logs deployment/atenet-egress --tail=-1 2>/dev/null \
|
||||
| tail -n +"$((BEFORE + 1))" | grep '\[egress\]' || {
|
||||
echo "!! no [egress] lines — dumping recent gateway logs:"
|
||||
${K} -n ate-system logs deployment/atenet-egress --tail=20
|
||||
exit 1
|
||||
}
|
||||
# Envoy emits the CONNECT entry asynchronously (an external dst can land seconds
|
||||
# after the actor's response), so we poll. And the Actor's HTTP client keeps the
|
||||
# tunnel alive: a repeat fetch to a host it already reached rides the open tunnel
|
||||
# and produces no new access-log entry at all, so a run against a warm actor
|
||||
# would fail even though egress is working. Fall back to any tunnel already open
|
||||
# for this actor's SAN before declaring failure.
|
||||
SAN="atespace/${ATESPACE}/actor/${ACTOR}"
|
||||
NEW=""
|
||||
for _ in $(seq 1 15); do
|
||||
NEW=$(${K} -n ate-system logs deployment/atenet-egress -c envoy --tail=-1 2>/dev/null \
|
||||
| tail -n +"$((BEFORE + 1))" | grep '\[egress\]' || true)
|
||||
[ -n "${NEW}" ] && break
|
||||
sleep 2
|
||||
done
|
||||
if [ -n "${NEW}" ]; then
|
||||
echo "${NEW}"
|
||||
elif ${K} -n ate-system logs deployment/atenet-egress -c envoy --tail=-1 2>/dev/null \
|
||||
| grep -q "\[egress\].*${SAN}"; then
|
||||
echo " no new CONNECT — the actor reused an already-open tunnel; its existing entries:"
|
||||
${K} -n ate-system logs deployment/atenet-egress -c envoy --tail=-1 | grep "\[egress\].*${SAN}" | tail -3
|
||||
else
|
||||
echo "!! no [egress] lines for ${SAN} — dumping recent gateway logs:"
|
||||
${K} -n ate-system logs deployment/atenet-egress -c envoy --tail=20
|
||||
exit 1
|
||||
fi
|
||||
echo "== PASS: actor egress traversed the Envoy egress gateway =="
|
||||
|
||||
@@ -24,14 +24,6 @@ import (
|
||||
"github.com/agent-substrate/substrate/internal/resources"
|
||||
)
|
||||
|
||||
// SEE(lior): this file used to also hold TestResolveHTTPTargetPort and
|
||||
// TestIsPodReady. Main moved both into internal/portforward/portforward_test.go
|
||||
// (upstream 4453b5e7, "Prefactoring: Consolidate logic to port-forward to a
|
||||
// service Pod") and deleted this file, so git rename-matched this branch's
|
||||
// version of it against the new portforward test and reported the whole thing as
|
||||
// a conflict. Took main's move as-is and re-added only the net-new PostJSON test
|
||||
// here, rather than resurrecting the port-forward tests in a package that no
|
||||
// longer owns that code.
|
||||
func TestRouterClientPostJSON(t *testing.T) {
|
||||
client := &RouterClient{
|
||||
baseURL: "http://router.test",
|
||||
|
||||
@@ -104,14 +104,11 @@ spec:
|
||||
- --actor-id-ca-pool=/run/actor-id-ca-pool/pool.json
|
||||
- --atelet-client-cred-bundle=/run/podidentity.podcert.ate.dev/credential-bundle.pem
|
||||
- --pod-identity-ca-certs=/run/podidentity.podcert.ate.dev/trust-bundle.pem
|
||||
# ONLY UNTIL THE EGRESS API IS DECIDED, FOR POC: turn on pluggable actor
|
||||
# TODO(lior):Remove when egress API pr is merged. This turns on pluggable actor
|
||||
# egress cluster-wide. ateapi stamps this address onto every atelet
|
||||
# Run/Restore, ateom hands it to atunnel, and actor TCP egress is
|
||||
# transparently redirected (nftables) into atunnel, which wraps it in
|
||||
# mTLS + HTTP CONNECT to the egress gateway.
|
||||
#
|
||||
# SEE(lior): this flag lived on atelet on the pre-rebase branch; PR 708
|
||||
# moved egress-gateway configuration to ateapi, so it is set here now.
|
||||
- --egress-gateway-address=atenet-egress.ate-system.svc:443
|
||||
# Graceful shutdown knobs. The sum must fit within terminationGracePeriodSeconds.
|
||||
- --drain-delay=13s
|
||||
|
||||
@@ -17,13 +17,6 @@
|
||||
# actor-identity client cert),
|
||||
# * terminate the actor's HTTP CONNECT and tunnel raw TCP to the CONNECT
|
||||
# authority (the actor's original destination, always sent as IP:port).
|
||||
#
|
||||
# SEE(lior): the client cert is the actor's own identity — minted per actor by
|
||||
# ateapi's actoridentity service off the actor-identity CA and carrying the
|
||||
# ActorIdentity X.509 extension — not the pod's podidentity cert. The pod cert
|
||||
# says "some substrate pod"; only the actor cert says *which actor*, which is
|
||||
# the thing the gateway has to authorize. The CONNECT carries no identity
|
||||
# headers at all; everything the PEP decides on comes out of the certificate.
|
||||
apiVersion: v1
|
||||
kind: ServiceAccount
|
||||
metadata:
|
||||
@@ -89,9 +82,7 @@ data:
|
||||
# ActorIdentity in Go is to have Envoy hand over the raw chain.
|
||||
# SANITIZE_SET drops whatever x-forwarded-client-cert the client
|
||||
# sent and writes Envoy's own view of the verified peer, and
|
||||
# chain: true puts the full URL-encoded PEM chain in it. Removing
|
||||
# either of these breaks egress closed: the handler sees no XFCC
|
||||
# and denies every CONNECT.
|
||||
# chain: true puts the full URL-encoded PEM chain in it.
|
||||
forward_client_cert_details: SANITIZE_SET
|
||||
set_current_client_cert_details:
|
||||
chain: true
|
||||
@@ -106,16 +97,6 @@ data:
|
||||
"@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
|
||||
log_format:
|
||||
text_format_source:
|
||||
# SEE(lior): the peer fields come from the verified client
|
||||
# certificate, not from request headers. The actor sends no
|
||||
# identity headers any more, and logging attacker-controlled
|
||||
# ones would have made this log lie about who egressed.
|
||||
# peer_san= is the actor's SPIFFE URI SAN,
|
||||
# spiffe://substrate-actor.local/atespace/<atespace>/actor/<name>
|
||||
# — the only actor-identifying field Envoy can format
|
||||
# (ateapi mints these with an empty Subject, and the UID
|
||||
# lives in the ActorIdentity extension Envoy cannot read).
|
||||
# The ext_proc sidecar logs atespace/actor/UID separately.
|
||||
inline_string: "[egress] authority=%REQ(:AUTHORITY)% peer_san=%DOWNSTREAM_PEER_URI_SAN% peer_serial=%DOWNSTREAM_PEER_SERIAL% code=%RESPONSE_CODE% flags=%RESPONSE_FLAGS% up_bytes=%BYTES_RECEIVED% down_bytes=%BYTES_SENT%\n"
|
||||
route_config:
|
||||
name: connect_route
|
||||
@@ -360,19 +341,6 @@ spec:
|
||||
matchLabels:
|
||||
podcert.ate.dev/canarying: live
|
||||
path: trust-bundle.pem
|
||||
# SEE(lior): the actor-identity CA root, as a cert-only Secret that
|
||||
# install-ate.sh derives from the actor-id-ca-pool Secret. It is a plain
|
||||
# Secret rather than a projected clusterTrustBundle — unlike the two above
|
||||
# — because the actor-identity CA has no signer/controller publishing a
|
||||
# trust bundle for it yet; its only in-cluster home is actor-id-ca-pool,
|
||||
# whose pool.json also holds the CA *signing key*. Mounting that here would
|
||||
# put the key that mints every actor identity into the pod that merely
|
||||
# verifies them, so install-ate.sh extracts the root and nothing else.
|
||||
#
|
||||
# TODO(liorlieberman): revisit once the actor-identity CA is published as a
|
||||
# ClusterTrustBundle; this volume then becomes a third projected source and
|
||||
# the install-time Secret goes away. See create_actor_id_ca_certs_secret in
|
||||
# hack/install-ate.sh.
|
||||
- name: actor-id-ca-certs
|
||||
secret:
|
||||
secretName: actor-id-ca-certs
|
||||
|
||||
Reference in New Issue
Block a user