feat: add coding-agent egress proxy sidecar support#306
Conversation
archf
force-pushed
the
feature/add-coding-agent-sidecar-proxy
branch
from
8b7f04e to
7d5bc95
Compare
| corev1.EnvVar{Name: "http_proxy", Value: proxyURL}, | ||
| corev1.EnvVar{Name: "https_proxy", Value: proxyURL}, | ||
| corev1.EnvVar{Name: "no_proxy", Value: "localhost,127.0.0.1,::1"}, | ||
| ) |
There was a problem hiding this comment.
This is good for reducing accidental exposure, but I don't think it's as hard of a restriction as we need. A determined agent could work around this by unsetting the env vars, or using some connection method that doesn't respect the env vars like opening a raw TCP socket e.g. here's a simple bypass that can "fail" in the existing test script:
env -u http_proxy -u https_proxy -u HTTP_PROXY -u HTTPS_PROXY \
curl -sS --max-time 15 -o /dev/null -w "%{http_code}\n" https://xkcd.com/2347/I think the original proposal mentioned having an init container with NET_ADMIN that can set iptables -- that shape sounds like it comes with more guarantees, since our main container won't have NET_ADMIN privileges to get around it. That said, as long as we have some path to squid proxy being a hard limiter, then I'd be in favour of shipping this as-is and plopping the "more hardened" fix on top. This addition of squid proxy is the bulk of the work and looks good, I just want to be sure a little more iteration can lead to something impenetrable
There was a problem hiding this comment.
OK so I did a bit more reading and I think iptables still leaves a small gap allowing DNS tunneling: If a user owns domain evil.com and asks the agent to hit <sensitive-data>.evil.com, the agent would still be able to send a UDP packet to resolve the IP at <sensitive-data>.evil.com, which allows the attacker to successfully record <sensitive-data>.
Getting around this would require we also implement some sort of DNS filtering. I'm not sure if it's worth the effort at this point, but definitely something worth documenting as a gap, and maybe we could go the route of abuse detection instead of implementing yet another sidecar (or mucking with our cluster-wide DNS service which sounds scary). This vector is distinctly malicious, requiring deliberately "evil" + clever prompting, whereas the other stuff we're fixing has the possibility of being a bit more incidental...
There was a problem hiding this comment.
I haven't thought of it that way. If the LLM can figure there's an environment variable to unset, it would indeed be able to escape.
Perhaps we need to engineer the networking differently: when the LLM unsets the env vars, no networking at all should work.
Job pods may only reach:
- DNS
- A shared squid proxy service deployment
And All other direct connection attempts are blocked with a k8s NetworkPolicy
For the customer-specific escape hatch, we would need to augment the shared squid proxy with an ACL helper with this directive in squid v7 and bake the account ID, perhaps, in the http_proxy env var.
This directive is not available in the v8 version of Squid.
But unfortunately there is this warning in the documentation and it might not be so future forward. We could need to re-implement the whole thing or get stuck at v7 forever.
But then the problem of the LLM forging random accountID and looping over http_proxy en vars comes in, but it might be good enough given the sheer amount of effort it would need to go through to figure a) a working/valid account ID that b) grants access to a extra very specific domains it doesn't know about in the first place.
Alternatively, we can dig into a solution with Envoy Proxy.
sequenceDiagram
autonumber
participant User as User / Web UI
participant App as OpsLevel Application
participant DB as RDS MySQL
participant Runner as opslevel-runner
participant K8s as Kubernetes API
participant Pod as Job Pod
participant Envoy as Envoy Egress Proxy
participant AuthZ as Egress AuthZ Service
participant Internet as Destination
User->>App: Configure additional allowed domains
App->>DB: Persist account egress policy
Note over Runner,DB: Later, when scheduling a job
Runner->>Runner: Obtain accountID and jobID
Runner->>Runner: Build Pod spec with<br/>HTTP_PROXY=http://accountID:jobID@envoy-egress:3128
Runner->>K8s: Create Job Pod
K8s->>Pod: Start Pod
Note over Pod,Envoy: NetworkPolicy allows only DNS and Envoy
Pod->>Envoy: CONNECT api.vendor.example:443<br/>Proxy-Authorization contains accountID:jobID
Envoy->>AuthZ: ext_authz CheckRequest<br/>accountID, jobID, host, port, method
AuthZ->>DB: Verify job/account and load account policy
DB-->>AuthZ: Job metadata + allowed domains
AuthZ->>AuthZ: Evaluate:<br/>global allowlist<br/>+ account allowlist<br/>default deny
alt Allowed
AuthZ-->>Envoy: ALLOW
Envoy->>Internet: Resolve and connect
else Denied or unknown
AuthZ-->>Envoy: DENY
Envoy-->>Pod: 403 / proxy denial
end
ext_authz is the adapter between Envoy and the existing application data to retrieve the account specific allowed domains.
Envoy proxy looks to be better fitted for cloud environments at a first glance. The immediate win is that we would not need to have the customer-specific domains circulate from the app to the orchestrator to the job. It will be pulled directly from the datastore where it is needed.
But that extra complexity is required if we do want a unique per customer unique configuration consisting of a specific layer of allowed domains layered on top of the shared one.
It could be acceptable to regenerate a globally shared squid configmap and hot reload squid proxies with the base layer plus all the account specific added domains as I would trust that a given customer would not allow himself access to an evil domain.
There was a problem hiding this comment.
hmm is there a reason just changing iptables in a privileged (NET_ADMIN) initcontainer wouldn't work for restricting egress to just the squid proxy? I think this is what Paul proposal + AI recommended. The idea is our main LLM container wouldn't have permissions to change it afterwards to get around the restriction, unlike env vars
I'm not sure how much of a pain it is to set this up in the init container tho...
There was a problem hiding this comment.
You can block an IP with iptables but you can't put up domain based rules that easily. You need to convert them all to IPs first. Were back to fqdn-controller or fqdn-policy.
There was a problem hiding this comment.
oh yeah that's why I'm suggesting this works with the squid proxy approach -- we keep everything in this PR basically, and then the iptables config is just to restrict our main container's traffic to the sidecar IP. Then squid can still handle the DNS magic, right?
| // so kubelet gates the *next* init container on its TCP startupProbe. | ||
| // working with the queue name; can't use agentMode until agentMode stops | ||
| // implying privileged mode | ||
| if s.podConfig.Queue == "coding-agent" { |
There was a problem hiding this comment.
for "v2" (follow-up), we should just add a new arg that can be specified when starting the runner, e.g. --use-proxy. The coupling to coding-agent queue name is a bit weird in that it points back to our deployed chart's specific implementation detail - not really relevant to the runner code itself
There was a problem hiding this comment.
This is a good point. We can add a --proxy flag immediately and inject the sidecar based on that. But I had a mental model where we had a single opslevel-runner instance running that was 'smart'. IIRC we added a second opslevel-runner instance for the privileged containers as we needed to pass a flag.
Interim solution: add a --proxy-queue-name=<job queue name for which to add sidecar>
And then we can potentially go back to a single opslevel-runner deployment
There was a problem hiding this comment.
Imo the runner's queue handling is very bad - there is no separation of queue + worker, so higher priority queues starve out lower priority queues completely. From that perspective, we'll probably need to keep two runner deployments for now even if they don't need separate flags (e.g. agent-mode). It's useful to just have two workers picking off of different queues.
The proxy potentially being for specific queue within a deployment is a good point though, I hadn't thought about that. --proxy-queue-name does the job I think, even if we don't squash down to one runner
There was a problem hiding this comment.
--proxy-queue-name would allows for any possibilities.
I did not like it much that the queue name was hardcoded tbh. I'm glad that we were finding new possible solutions whatever we decide.
| SCRIPT_DIR="${BASH_SOURCE[0]%/*}/../bin" | ||
| source "$SCRIPT_DIR/kind-env.sh" | ||
|
|
||
| echo "Applying squid-config ConfigMap..." |
There was a problem hiding this comment.
There's one big gap left here: We're requiring the baseline squid configmap be applied before running jobs, otherwise the pod will get stuck trying to mount the squid volume.
In line with opslevel-runner managing all the k8s resources internally, I think this configmap should actually be in the runner code, and it should apply the baseline configmap itself. The consumer shouldn't need to do anything, except specify allowed_domains overrides as desired.
Basically we should be able to remove "step 1" in this test script entirely, and ol-runner should do it all internally I think
2 participants
Description
Add an egress proxy sidecar when job is read from a specific queue (i.e.: coding-agent).
Queuemust be passed in the job spec. We're not relying on the--job-agent-modesince this would cause the pod to run in privileged mode, what we do not want. We can always leverage `--job-agent-mode later on once we retire SWE-Agent.Tophatting
Tested by enqueuing a job in Faktory mode
Opslevel-Runner logs
Test