Skip to content

Commit

Permalink
feat: add example ValidatingAdmissionPolicy to block param interpolat…
Browse files Browse the repository at this point in the history
…ion. Fixes #5114

Signed-off-by: Mason Malone <[email protected]>
  • Loading branch information
MasonM committed Jan 2, 2025
1 parent 57ba14a commit 5e634fd
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 3 deletions.
4 changes: 2 additions & 2 deletions .devcontainer/pre-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ set -eux

# install kubernetes
wget -q -O - https://raw.githubusercontent.com/k3d-io/k3d/main/install.sh | bash
k3d cluster get k3s-default || k3d cluster create --image rancher/k3s:v1.27.3-k3s1 --wait
k3d cluster get k3s-default || k3d cluster create --image rancher/k3s:v1.29.10-k3s1 --wait
k3d kubeconfig merge --kubeconfig-merge-default

# install kubectl
curl -LO https://dl.k8s.io/release/v1.27.3/bin/linux/$(go env GOARCH)/kubectl
curl -LO https://dl.k8s.io/release/v1.29.10/bin/linux/$(go env GOARCH)/kubectl
chmod +x ./kubectl
sudo mv ./kubectl /usr/local/bin/kubectl
kubectl cluster-info
Expand Down
41 changes: 41 additions & 0 deletions examples/argo-dangerous-interpolation-vap.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# This admission policy (https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/)
# can be used to block workflows that use string interpolation inside the fields "command",
# "args", or "source".
#
# Interpolating untrusted user input into this fields can lead to command
# injection vulnerabilities (https://owasp.org/www-community/attacks/Command_Injection).
#
# This policy will only work when using the full CRDs (https://argo-workflows.readthedocs.io/en/latest/installation/#full-crds).
# You must create a ValidatingAdmissionPolicyBinding to use it.
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicy
metadata:
name: "argo-dangerous-interpolation-vap"
spec:
failurePolicy: Fail
matchConstraints:
resourceRules:
- apiGroups: ["argoproj.io"]
apiVersions: ["v1alpha1"]
operations: ["CREATE", "UPDATE"]
resources: ["workflows"]
validations:
- expression: >
!object.spec.templates.map(
t, has(t.container)
? (
(has(t.container.command) ? t.container.command : [])
+
(has(t.container.args) ? t.container.args : [])
)
: (
has(t.script)
? (
(has(t.script.command) ? t.script.command : [])
+
(has(t.script.source) ? [t.script.source] : [])
)
: []
)
).exists(values, values.exists(value, value.matches('\\{\\{[^\\}]*\\}\\}')))
message: "Dangerous interpolation detected"
2 changes: 1 addition & 1 deletion examples/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

func TestValidateExamples(t *testing.T) {
failures, err := ValidateArgoYamlRecursively(".", []string{"testvolume.yaml", "simple-parameters-configmap.yaml", "memoize-simple.yaml"})
failures, err := ValidateArgoYamlRecursively(".", []string{"testvolume.yaml", "simple-parameters-configmap.yaml", "memoize-simple.yaml", "argo-dangerous-interpolation-vap.yaml"})
if err != nil {
t.Errorf("There was an error: %s", err)
}
Expand Down
72 changes: 72 additions & 0 deletions test/e2e/argo_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
admissionv1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -668,6 +669,77 @@ func (s *ArgoServerSuite) TestPermission() {
})
}

func (s *ArgoServerSuite) TestWorkflowRejectedByValidatingAdmissionPolicy() {
ctx := context.Background()

s.Run("Create ValidatingAdmissionPolicy", func() {
obj, err := fixtures.LoadObject("@functional/argo-dangerous-interpolation-vap.yaml")
s.Require().NoError(err)
_, err = s.KubeClient.AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(ctx, obj.(*admissionv1.ValidatingAdmissionPolicy), metav1.CreateOptions{})
s.Require().NoError(err)
})
s.T().Cleanup(func() {
_ = s.KubeClient.AdmissionregistrationV1().ValidatingAdmissionPolicies().Delete(ctx, "argo-dangerous-interpolation-vap", metav1.DeleteOptions{})
})

s.Run("Create ValidatingAdmissionPolicyBinding", func() {
obj, err := fixtures.LoadObject("@functional/argo-dangerous-interpolation-vap-binding.yaml")
s.Require().NoError(err)
_, err = s.KubeClient.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Create(ctx, obj.(*admissionv1.ValidatingAdmissionPolicyBinding), metav1.CreateOptions{})
s.Require().NoError(err)
})
s.T().Cleanup(func() {
_ = s.KubeClient.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Delete(ctx, "argo-dangerous-interpolation-vap-binding", metav1.DeleteOptions{})
})

// Sleep to wait for VAP to be created
time.Sleep(2 * time.Second)

workflow := `{
"workflow": {
"metadata": {
"name": "test",
"labels": { "workflows.argoproj.io/test": "dangerous-interpolation" }
},
"spec": {
"arguments": {
"parameters": [{ "name": "message", "value": "foo" }]
},
"entrypoint": "run-workflow",
"templates": [{
"name": "run-workflow",
%s
}]
}
}
}`
s.Run("Rejects workflow with interpolation in script template", func() {
s.e().POST("/api/v1/workflows/argo").
WithText(fmt.Sprintf(workflow, `"script": {
"image": "argoproj/argosay:v2",
"command": ["sh", "-c"],
"source": "echo message: {{workflow.parameters.message}}"
}`)).
Expect().
Status(400).
Body().
Contains("denied request: Dangerous interpolation detected")
})

s.Run("Rejects workflow with interpolation in container template", func() {
s.e().POST("/api/v1/workflows/argo").
WithText(fmt.Sprintf(workflow, `"container": {
"image": "argoproj/argosay:v2",
"command": ["sh", "-c"],
"args": ["echo message: {{workflow.parameters.message}}"]
}`)).
Expect().
Status(400).
Body().
Contains("denied request: Dangerous interpolation detected")
})
}

func (s *ArgoServerSuite) TestLintWorkflow() {
s.e().POST("/api/v1/workflows/argo/lint").
WithBytes([]byte((`{
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/functional/argo-dangerous-interpolation-vap-binding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicyBinding
metadata:
name: "argo-dangerous-interpolation-vap-binding"
namespace: argo
spec:
policyName: "argo-dangerous-interpolation-vap"
validationActions: [Deny]
matchResources:
objectSelector:
matchLabels:
workflows.argoproj.io/test: dangerous-interpolation
1 change: 1 addition & 0 deletions test/e2e/functional/argo-dangerous-interpolation-vap.yaml

0 comments on commit 5e634fd

Please sign in to comment.