Skip to content

Commit 142c792

Browse files
authored
fix(server): harden bundle vendor path against SSRF and resource exhaustion (NVIDIA#2170)
Signed-off-by: framsouza <fram.souza14@gmail.com>
1 parent 1e0ea13 commit 142c792

11 files changed

Lines changed: 2721 additions & 31 deletions

File tree

docs/integrator/kubernetes-deployment.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ duplicated here.
236236
| Variable | Default | Description |
237237
|----------|---------|-------------|
238238
| `PORT` | 8080 | HTTP server port |
239+
| `AICR_SERVER_ADDRESS` | (unset = all interfaces) | Listen address. Unset binds every interface (required for the in-tree Kubernetes Deployment: kubelet livenessProbe/readinessProbe and kube-proxy both dial the pod IP directly, not loopback). Set to `127.0.0.1` for a loopback-only bind on a sidecar or bare-host deployment fronted by a same-pod reverse proxy. Set to a specific interface to constrain listener binding. |
240+
| `AICR_ALLOW_VENDOR_CHARTS` | `false` | Opt-in for `POST /v1/bundle?vendor-charts=true`. When off (default) the vendor path is rejected with 400 — this endpoint drives server-side `helm pull` against a caller-supplied URL and must not be exposed on an unauthenticated network. Parsed by Go's `strconv.ParseBool`: accepts `1`/`t`/`T`/`TRUE`/`true`/`True` to enable (or the matching false values to disable); any other value (including `yes`, `on`, or a typo) is treated as disabled and logged as a warning. |
241+
| `AICR_HELM_REPOSITORY_HOST` | (unset = no credentials attached) | The single repository host the vendor-charts index pre-check may send `HELM_REPOSITORY_USERNAME`/`HELM_REPOSITORY_PASSWORD` to. Attaches credentials ONLY when: this env is set, the request scheme is `https`, and the request host case-insensitively matches this value. Any mismatch suppresses credentials silently so a caller-supplied `Repository` URL cannot exfiltrate the operator's helm credentials. Leave unset unless you need the index pre-check to authenticate against a specific private HTTP repo. |
242+
| `HELM_REPOSITORY_USERNAME` / `HELM_REPOSITORY_PASSWORD` | (unset) | Basic-auth credentials for the vendor-charts index pre-check. Gated by `AICR_HELM_REPOSITORY_HOST` above — no credentials fly unless that host allowlist is set. The upstream `helm pull --repo` subprocess does NOT itself consume these vars; private HTTP repos require an out-of-band `helm repo add --username --password` in the aicrd image. |
239243
| `SHUTDOWN_TIMEOUT_SECONDS` | 30 | Graceful-shutdown drain timeout (seconds) |
240244
| `AICR_LOG_LEVEL` | info | Logging level: debug, info, warn, error |
241245
| `AICR_ALLOWED_ACCELERATORS` | (unset = all) | Comma-separated allowlist of accelerator types (e.g. `h100,l40`) |
@@ -245,6 +249,57 @@ duplicated here.
245249

246250
**Note:** These are the only environment variables the API server reads for criteria filtering and transport; server-side bundle signing (`POST /v1/bundle?attest=true`) reads an additional set documented in [API Reference › Server-Side Signing](../user/api-reference.md#server-side-signing). The four `AICR_ALLOWED_*` allowlists are parsed once at startup to restrict which criteria values the server will accept. Rate-limit, request-timeout, and body-size settings are compiled-in constants from `pkg/defaults`, not environment-tunable. The server uses structured JSON logging to stderr. The CLI supports three logging modes (CLI/Text/JSON), but the API server always uses JSON for consistent log aggregation.
247251

252+
### Network Egress from the Vendor-Charts Path
253+
254+
`POST /v1/bundle?vendor-charts=true` performs server-side `helm pull` against
255+
the repository URL declared by each component in the submitted recipe. Four
256+
controls keep this endpoint safe by default:
257+
258+
1. **Opt-in gate.** Off unless the operator sets
259+
`AICR_ALLOW_VENDOR_CHARTS=true`. The bundle handler rejects
260+
`vendor-charts=true` with `400` when the server is not opted in, so an
261+
accidentally-exposed instance never performs egress on behalf of a request.
262+
2. **Repository egress policy.** Even with opt-in, the vendor layer rejects
263+
repository hosts that resolve to loopback, link-local, RFC1918 / CGNAT /
264+
ULA private ranges, multicast, unspecified, or the well-known cloud-
265+
metadata IPs (169.254.169.254, 100.100.100.200, fd00:ec2::254,
266+
fe80::a9fe:a9fe).
267+
3. **Index-yaml pre-check (HTTP(S) only).** Before invoking `helm pull`, the
268+
server fetches `<repo>/index.yaml` through a hardened HTTP client
269+
(bounded body, redirect-hops validated against the same egress policy),
270+
parses the entries for the requested chart+version, resolves relative
271+
URLs, and rejects the request if ANY declared tarball URL points at a
272+
disallowed host. This closes the classic "public index.yaml points at a
273+
private-network tarball" SSRF vector at pre-check time.
274+
4. **Artifact size cap.** The pulled `.tgz` is capped at 64 MiB — well
275+
above real charts, low enough to bound server memory.
276+
277+
**Residual risks that require operator-side controls:**
278+
279+
- **DNS rebinding** between the pre-check and helm's own re-resolution when
280+
it actually fetches (helm re-resolves without exposing the resolved IP to
281+
us).
282+
- **HTTP redirects during helm's tarball fetch** — helm is a subprocess and
283+
its redirect hops are not visible to the pre-check.
284+
- **OCI protocol** — the OCI distribution redirect chain (manifest → blob
285+
GETs, which registries commonly redirect to a CDN URL) is not intercepted.
286+
- **Resolver divergence** — the pre-check re-implements Helm's semver
287+
constraint resolution to select which chart-version entry from the
288+
fetched `index.yaml` to egress-check. Helm itself re-fetches the index
289+
and re-resolves independently when it actually pulls, so the URL the
290+
pre-check egress-validated can differ from the URL Helm pulls if the
291+
index changes between calls or if the two resolvers pick differently
292+
under ambiguous inputs — a defense-in-depth check that can bit-rot as
293+
Helm evolves.
294+
295+
For all four, the operational control is a Kubernetes `NetworkPolicy` on
296+
the aicrd pod or an equivalent egress firewall that allow-lists only the
297+
public chart registries the deployment needs. If you cannot enforce that
298+
network boundary, keep `AICR_ALLOW_VENDOR_CHARTS` off and front the server
299+
with authenticated ingress. A follow-up will move the tarball fetch
300+
in-process to close these residuals without needing a network-layer
301+
control.
302+
248303
### ConfigMap for Custom Recipe Data (Advanced)
249304

250305
> **Note:** The `aicrd` HTTP server resolves recipes from the binary's

docs/user/api-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ Generate deployment bundles from a recipe.
551551
| `accelerated-node-selector` | string[] | | Node selectors for GPU nodes (format: `key=value`). Repeat for multiple. |
552552
| `accelerated-node-toleration` | string[] | | Tolerations for GPU nodes (format: `key=value:effect`). Repeat for multiple. |
553553
| `nodes` | int | 0 | Estimated number of GPU nodes (0 = unset). Written to Helm value paths declared in the registry under `nodeScheduling.nodeCountPaths`. |
554-
| `vendor-charts` | bool | false | Pull upstream Helm chart bytes into the bundle at bundle time so the artifact is fully self-contained and air-gap deployable. Each vendored chart is recorded in `provenance.yaml` with name, version, source URL, and SHA256. Trades the upstream CVE-yank fail-loud signal for offline deployability — see the CLI reference's "Vendoring Charts for Air-Gap" section for the full tradeoff. Requires the `helm` binary on the API server's `$PATH` and registry credentials configured for any private upstream repos (`HELM_REPOSITORY_USERNAME`/`HELM_REPOSITORY_PASSWORD` for HTTP(S); docker config for OCI). If prerequisites are missing the request fails with a structured error code (`SERVICE_UNAVAILABLE` / HTTP 503 for missing helm, `UNAUTHORIZED` / HTTP 401 for credentials). |
554+
| `vendor-charts` | bool | false | Pull upstream Helm chart bytes into the bundle at bundle time so the artifact is fully self-contained and air-gap deployable. Each vendored chart is recorded in `provenance.yaml` with name, version, source URL, and SHA256. Trades the upstream CVE-yank fail-loud signal for offline deployability — see the CLI reference's "Vendoring Charts for Air-Gap" section for the full tradeoff. Requires the `helm` binary on the API server's `$PATH`. **The server-side vendor path is opt-in and off by default** — the operator must set `AICR_ALLOW_VENDOR_CHARTS=true`, otherwise `vendor-charts=true` returns `400 vendor-charts is not enabled on this server`. Even when enabled, repository hosts that resolve to loopback, link-local, private, or cloud-metadata IPs are rejected with `400 INVALID_REQUEST`, and vendored artifacts are capped at 64 MiB. **Private HTTP(S) repository credentials:** the aicrd pre-check sends `HELM_REPOSITORY_USERNAME`/`HELM_REPOSITORY_PASSWORD` (as HTTP Basic auth) ONLY when `AICR_HELM_REPOSITORY_HOST` is set to that repository's exact host, the request scheme is `https`, and the request host matches (case-insensitive). All three conditions must hold — an operator setting only the username/password env vars will get no credentials attached, preventing a caller-supplied `Repository` URL from harvesting the operator's helm credentials. (Note: the upstream `helm pull --repo` subprocess does not itself read these env vars — private HTTP repos require a prior `helm repo add --username --password` in the aicrd image or an SDK-based puller.) OCI credentials flow through the standard docker config (`~/.docker/config.json` or `$DOCKER_CONFIG`), exactly like `helm pull oci://...`. If prerequisites are missing the request fails with a structured error code (`SERVICE_UNAVAILABLE` / HTTP 503 for missing helm). The index pre-check surfaces upstream HTTP status by class: `404` → `NOT_FOUND` / HTTP 404, `401`/`403` → `UNAUTHORIZED` / HTTP 401, `408`/`429` → `SERVICE_UNAVAILABLE` / HTTP 503 (retryable), other `4xx` → `INVALID_REQUEST` / HTTP 400, `5xx` → `SERVICE_UNAVAILABLE` / HTTP 503. |
555555
| `serial` | bool | false | Sequence components strictly one at a time in deployment order, disabling the parallel rollout of independent components. Affects `deployer=argocd`, `argocd-helm`, `flux`, and `helmfile` (`helm` is already serial): argocd falls back to a linear sync-wave per folder, flux chains each `HelmRelease` `dependsOn` to the previous component, and helmfile chains every release via `needs:` into one linear apply order. An escape hatch for reproducing the pre-parallelism ordering or bisecting a rollout. |
556556
| `deployer` | string | helm | Deployment method: `helm`, `argocd`, `argocd-helm`, `flux`, or `helmfile` |
557557
| `repo` | string | | Git repository URL for GitOps deployments (used with `deployer=argocd` and `deployer=flux`; ignored by `deployer=argocd-helm`) |

0 commit comments

Comments
 (0)