fix: dicht systemische template-injectie in OPI-manifesten#77
Conversation
|
Sterke gerichte fix — Scope-volledigheid — overgebleven templatesVan de 28 manifest-templates raken er 6+ dezelfde primitive (
Maar — gemitigeerd door PR #68 augmentatieIk heb op
Plus: deze PR's AppProject-whitelist beperkt cluster-scoped resources fors. De namespace-scoped uitbraak ( AanbevelingMergen als deze PR — de gefixte templates dichten de exploitabele paden, AppProject-whitelist beperkt blast radius, en #68-augmentatie filtert payloads bij ingang. Direct na merge: follow-up PR voor de overgebleven 6+ templates met dezelfde aanpak:
Plus de tweede laag — Klein punt
Test-uitbreiding bij follow-up
Allemaal toe te voegen in de follow-up, met dezelfde test-shape (render → parse → assert geen sibling-keys). |
uittenbroekrobbert
force-pushed
the
fix/template-injection-sweep
branch
from
ba27a37 to
4bb6f6a
Compare
Follow-up: rebased + audit afgerond, LGTM voor merge`ba27a375` (service.yaml injectie-fix) pakte mijn aanbevolen follow-up #1 exact op — `service.yaml.jinja` quote nu `name`, `namespace`, `project.name` via `| tojson` met undefined-tolerant default. Consistent met deployment/ingress. Rebase op main was nodig (merge-conflict in `ingress.yaml.jinja` en `project_file_handler.py`). Rebased + force-pushed (`4bb6f6ab`):
Verificatie deep-audit
Follow-up nog steeds nodig (uit eerdere comment)Service.yaml is nu klaar; nog open:
LGTM voor merge in huidige vorm. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # the safe URL-path charset to prevent nginx configuration-snippet | ||
| # injection (see _sanitize_path_value). | ||
| if isinstance(path_config, str): | ||
| logger.debug(f"Found single path '{path_config}' for component '{component_name}'") |
Follow-up issues aangemaakt
|
uittenbroekrobbert
force-pushed
the
fix/template-injection-sweep
branch
from
4bb6f6a to
18641ab
Compare
Zelfde injectieklasse als de reeds gefixte auth-wall sidecar, maar verspreid over de deployment- en ingress-templates. ManifestGenerator rendert met autoescape=False en tenant-bestuurde scalars werden ongequote (of naief dubbel-gequote) geinterpoleerd. Een newline plus dubbele quote in bijv. een storage mount-path of env-waarde liet een aanvaller siblings in de podSpec injecteren (privileged: true / hostPath), die ArgoCD vervolgens toepast. - deployment.yaml.jinja: alle tenant-bestuurde scalars (env-naam/-waarde, storage naam/mount-path/pvc/size, imageURL) via | tojson; een JSON-string is een geldige YAML-flow-scalar en escapet quotes en newlines. - ingress.yaml.jinja: match/rewrite worden bij de bron gevalideerd tegen een strikte URL-pad-charset (_sanitize_path_value / _normalize_path_config in project_file_handler.py), zodat ze geen nginx-directives in de configuration-snippet kunnen injecteren (auth-bypass / proxy_pass-SSRF). Overige scalars in de spec via | tojson. - argocd-appproject.yaml.jinja: cluster/namespace resource-whitelist teruggebracht van */* naar de expliciete minimale set kinds die OPI daadwerkelijk genereert (defense in depth die de amplifier sluit). - Regressietest test_template_injection_sweep.py: rendert de echte templates met kwaadaardige en normale waarden en controleert de geparste YAML. Bestaande manifest-, template- en multi-path-ingress-tests blijven groen; simple-example.yaml rendert ongewijzigd.
De sweep quote-de tenant-gestuurde scalars in deployment.yaml.jinja en ingress.yaml.jinja, maar service.yaml.jinja werd overgeslagen. Dat template wordt in dezelfde per-component-lus gerenderd met exact dezelfde tenant-gestuurde waarden (name = deployment-component, namespace, project.name). Component- en deployment-namen zijn in het projectschema ($defs) ongevalideerd (type: string), dus een newline plus quote in een component- of deployment-naam brak uit de YAML-scalar en injecteerde broer-sleutels op documentniveau van het Service-manifest dat ArgoCD vervolgens toepast: dezelfde injectieklasse als de auth-wall fix. name, namespace, labels.app, labels.project en selector.app worden nu met | tojson gequote, conform het patroon van de rest van de sweep. Regressietests toegevoegd die het echte template met newline+quote payloads renderen en zowel uitbraak-preventie als waardebehoud controleren; rood tegen het ongepatchte template, groen erna.
destinations.namespace pinning is de grens; binnen die ns moeten helm-charts vrij zijn om hun resources aan te maken (ServiceAccount, StatefulSet, Job, CronJob, etc.). Cluster-scoped blijft dicht via lege whitelist.
uittenbroekrobbert
force-pushed
the
fix/template-injection-sweep
branch
from
6b3bdc0 to
842d617
Compare
3 participants
Probleem
Dezelfde injectieklasse als de reeds gemergede auth-wall sidecar-fix, maar verspreid over de deployment- en ingress-templates.
opi/generation/manifests.pyrendert metautoescape=Falseen tenant-bestuurde waarden uit het project-YAML werden ongequote (of naief dubbel-gequote met"{{ x }}") in YAML-scalars geinterpoleerd.Bevestigde paden:
deployment.yaml.jinja:mountPath: {{ storage['mount-path'] }}(volledig ongequote),value: "{{ env_value }}"en- name: {{ env_name }}(naieve dubbele quote, breekt op"of newline), plus de volumes-blok. Bron: storagemount-pathen env-vars uit project-YAML, ongevalideerd. Een newline-payload levert een willekeurige podSpec op (privileged: true/hostPath) die ArgoCD vervolgens toepast.ingress.yaml.jinja:rewrite "^{{ path }}/?(.*)$" "{{ rewrite_base }}/$1" break;binnen een nginxconfiguration-snippet. Bron: componentmatch/rewriteuit project-YAML, ongesanitiseerd, dus rauwe nginx-config-injectie (auth-bypass /proxy_pass-SSRF).argocd-appproject.yaml.jinjametclusterResourceWhitelist/namespaceResourceWhitelistopgroup: '*' kind: '*'.Wijzigingen
imageURL) via| tojson. Een JSON-string is een geldige YAML-flow-scalar;tojsonescapet quotes en newlines, zodat uitbreken naar sibling-keys onmogelijk wordt.match/rewriteworden bij de bron gevalideerd tegen een strikte URL-pad-charset (_sanitize_path_value/_normalize_path_configinproject_file_handler.py) voordat ze enige template bereiken. Daardoor kunnen ze geen nginx-directives, quotes of newlines in de snippet injecteren. De nginxrewrite-directive blijft een letterlijke nginx-regel (tojson zou de regex breken), maar de inputs zijn nu charset-begrensd. Overige spec-scalars (hostname,path,service_name,tls_secret_name, haproxyrewrite-target) via| tojson, undefined-tolerant gemaakt.*/*vervangen door de expliciete minimale set kinds die OPI daadwerkelijk genereert (afgeleid uit allekind:inmanifests/*.jinja): cluster-scoped enkelNamespace; namespacedConfigMap,Secret,Service,PersistentVolumeClaim,Deployment,Ingress,NetworkPolicy,Issuer, CNPGCluster,VolumeSnapshot. De feitelijk via de user-AppProject gesyncte manifesten (deployment,service,allow-all-network-policy,ingress, configmaps/secrets/pvc/postgresql-cluster/issuer) vallen hier allemaal binnen. Backup/restore-pods worden out-of-band door connectors toegepast, niet via deze AppProject. Defense in depth die de amplifier sluit.Testplan
tests/test_template_injection_sweep.py(21 tests): rendert de echtedeployment.yaml.jinjaeningress.yaml.jinjaviarender_template(zonder de FastAPI-importketen) met een payload"\n privileged: true\n x: "voor mount-path, env-waarde en hostname, en assert dat de geparste YAML geen geinjecteerde sibling-keys bevat en de waarde intact blijft. Normale waarden produceren een geldig manifest. Plus charset-validatie van_sanitize_path_value/_normalize_path_config(legitieme paden/,/api,/v1/users,/kadergeaccepteerd; injectie-payloads afgewezen).uv run pytest tests/test_template_injection_sweep.py tests/test_manifests.py tests/test_templates.py tests/test_multi_path_ingress.py tests/test_actual_template.py tests/test_service_path.py --noconftest: alle groen (138 tests).simple-example.yamlrendert ongewijzigd; derepositories[].pathdaarin is een git-subdir, niet een component-routing-pad, en wordt niet door de sanitizer geraakt.uv run ruff check+uv run ruff format+uv run pyrightop de gewijzigde Python-bestanden: schoon (0 errors).Onafhankelijk / pre-existing
De volledige suite faalt bij collection door een pre-existing omgevings-incompatibiliteit (Python 3.14.0b4 + gepinde pydantic 2.12.5:
import fastapifaalt). Dat staat los van deze wijziging en is hier niet aangeraakt; de nieuwe test draait geisoleerd viarender_templatemet--noconftest. De auth-wall sidecar (sidecar-authorization-wall.yaml.jinja) enargo-repository*.jinjazijn bewust niet aangeraakt (reeds gefixt).