From 4bb44946eec6d8cb355908b2a36ebfd7e39aa1c5 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 29 May 2025 14:47:27 +0100 Subject: [PATCH 01/30] now checks for privileged system users instead of groups --- chart/templates/deployment.yaml | 1 - chart/values.yaml | 1 - src/main.go | 80 +++++++++++++++++++++++---------- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/chart/templates/deployment.yaml b/chart/templates/deployment.yaml index 4df9bd4..5930751 100644 --- a/chart/templates/deployment.yaml +++ b/chart/templates/deployment.yaml @@ -25,5 +25,4 @@ spec: args: - --log-level={{ .Values.logLevel }} - --protected-namespaces={{ join "," .Values.protectedNamespaces }} - - --unpriveleged-group={{ .Values.unprivilegedGroup }} - --allow-opinion-mode={{ .Values.allowOpinionMode }} diff --git a/chart/values.yaml b/chart/values.yaml index f6c873d..3d423fb 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -5,7 +5,6 @@ logLevel: 1 protectedNamespaces: - kube-system - openstack-system -unprivilegedGroup: "oidc:/platform-users" allowOpinionMode: false ingress: diff --git a/src/main.go b/src/main.go index 4caadfb..73f0530 100644 --- a/src/main.go +++ b/src/main.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httputil" "os" + "regexp" "slices" "strings" ) @@ -25,27 +26,44 @@ type SubjectAccessReviewAPI struct { Status authorizationv1.SubjectAccessReviewStatus } type SubjectAccessReviewSpecAPI struct { - ResourceAttributes *authorizationv1.ResourceAttributes + ResourceAttributes *authorizationv1.ResourceAttributes NonResourceAttributes *authorizationv1.NonResourceAttributes - User string - Group []string - Groups []string - Extra map[string]authorizationv1.ExtraValue - UID string + User string + Group []string + Groups []string + Extra map[string]authorizationv1.ExtraValue + UID string } type SubjectAccessReviewHTTPResponse struct { - ApiVersion string `json:"apiVersion"` - Kind string `json:"kind"` - Status authorizationv1.SubjectAccessReviewStatus `json:"status"` + ApiVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Status authorizationv1.SubjectAccessReviewStatus `json:"status"` } var readonlyVerbs = []string{"get", "list", "watch", "proxy"} -func createAuthorizer(protectedNamespaces []string, unprivilegedGroup string,opinionMode bool,logLevel int) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { +func isInternalAccount(user string, namespace string) bool { + + systemAccountRegex, _ := regexp.Compile("system:.+") + namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") + if err != nil { + fmt.Printf("Error compiling regex " + "\"system:serviceaccount:" + namespace + ":.+\": " + err.Error()) + } - if(logLevel >= 2) { fmt.Printf("Request received from %s\n:", r.RemoteAddr) } + if user == "system:anonymous" { + return false + } else if namespaceServiceAccountRegex.MatchString(user) { + return true + } else if systemAccountRegex.MatchString(user) { + return true + } + + return false +} + +func createAuthorizer(protectedNamespaces []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { // Print dump for logging dump, dumperr := httputil.DumpRequest(r, true) @@ -54,35 +72,35 @@ func createAuthorizer(protectedNamespaces []string, unprivilegedGroup string,opi return } - if(logLevel >= 2) { fmt.Println(string(dump)) } - var sar SubjectAccessReviewAPI err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { - fmt.Println("[ERROR] ",err.Error()) + fmt.Println("[ERROR] ", err.Error()) http.Error(w, "Invalid JSON", http.StatusBadRequest) return } defer r.Body.Close() - isUnprivilegedUser := slices.Contains(sar.Spec.Groups, unprivilegedGroup) || slices.Contains(sar.Spec.Group, unprivilegedGroup) + isPrivilegedUser := sar.Spec.ResourceAttributes != nil && isInternalAccount(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace) isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) // TODO: test if you can bypass with empty or all namespaces isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" //TODO: test if you can bypass with * or singular nouns isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) status := new(authorizationv1.SubjectAccessReviewStatus) status.Allowed = false - if isUnprivilegedUser && isProtectedNamespace && isSecret { + if !isPrivilegedUser && isProtectedNamespace && isSecret { status.Denied = true status.Reason = "Cannot access secrets in protected namespace" - } else if isUnprivilegedUser && isProtectedNamespace && !isReadonlyVerb { + } else if !isPrivilegedUser && isProtectedNamespace && !isReadonlyVerb { status.Denied = true status.Reason = "Cannot write to protected namespace" } else { status.Denied = false status.Allowed = opinionMode - if(!opinionMode){ status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" } + if !opinionMode { + status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" + } } //todo: add status.EvaluationError handling @@ -91,8 +109,23 @@ func createAuthorizer(protectedNamespaces []string, unprivilegedGroup string,opi responseReview.Kind = "SubjectAccessReview" responseReview.Status = *status - if(status.Denied && logLevel == 1) { fmt.Println(string(dump)) } - if(status.Denied && logLevel >= 1){ fmt.Printf("[DENIED] Reason: %s\n",status.Reason) } + var deniedLogOutput string + if status.Denied { + deniedLogOutput = "Denied" + } else { + deniedLogOutput = "Allowed" + } + if logLevel >= 1 && sar.Spec.NonResourceAttributes != nil { + fmt.Printf("%s non-resource request from \"%s\". Reason: %s\n", deniedLogOutput, sar.Spec.User, status.Reason) + } + if logLevel >= 1 && sar.Spec.ResourceAttributes != nil { + fmt.Printf("%s request from \"%s\" to \"%s\" \"%s\" in namespace \"%s\". Reason: %s \n", + deniedLogOutput, sar.Spec.User, sar.Spec.ResourceAttributes.Verb, sar.Spec.ResourceAttributes.Resource, sar.Spec.ResourceAttributes.Namespace, status.Reason) + } + if logLevel >= 2 { + fmt.Println("HTTP Dump:") + fmt.Println(string(dump)) + } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(responseReview) @@ -100,15 +133,14 @@ func createAuthorizer(protectedNamespaces []string, unprivilegedGroup string,opi } func main() { - var unprivelegedGroup = flag.String("unpriveleged-group", "oidc:/platform-users", "Name of group which should have their permissions restricted for protected namespaces") var protectedNamespacesCSL = flag.String("protected-namespaces", "kube-system,openstack-system", "Comma separated list of namespaces which unprivileged users will have limited permissions for") var logLevel = flag.Int("log-level", 1, "Verbosity of logs. Values: [0-2]") - var opinionMode = flag.Bool("allow-opinion-mode",false,"Specifies if this webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to true in SubjectAccessReview.") + var opinionMode = flag.Bool("allow-opinion-mode", false, "Specifies if this webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to true in SubjectAccessReview.") flag.Parse() protectedNamespaces := strings.Split(*protectedNamespacesCSL, ",") - http.HandleFunc("/authorize", createAuthorizer(protectedNamespaces, *unprivelegedGroup, *opinionMode, *logLevel)) + http.HandleFunc("/authorize", createAuthorizer(protectedNamespaces, *opinionMode, *logLevel)) fmt.Printf("Server started\n") err := http.ListenAndServe(":8080", nil) if err != nil { From bd73d5324391b2038179b6bf1d5caae3d541055b Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 29 May 2025 14:57:20 +0100 Subject: [PATCH 02/30] image bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 3d423fb..7587226 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: 2f72ea1 +version: '5178677' logLevel: 1 protectedNamespaces: From e1bece14a3d96f89eabc92963f0c9616d9b660fa Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 29 May 2025 15:34:21 +0100 Subject: [PATCH 03/30] can now specify additional privileged users --- chart/templates/deployment.yaml | 1 + chart/values.yaml | 2 ++ src/main.go | 25 ++++++++++++++++--------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/chart/templates/deployment.yaml b/chart/templates/deployment.yaml index 5930751..dd363dd 100644 --- a/chart/templates/deployment.yaml +++ b/chart/templates/deployment.yaml @@ -23,6 +23,7 @@ spec: containerPort: 8080 protocol: TCP args: + - --additional-privileged-users={{ join "," .Values.additionalPrivilegedUsers }} - --log-level={{ .Values.logLevel }} - --protected-namespaces={{ join "," .Values.protectedNamespaces }} - --allow-opinion-mode={{ .Values.allowOpinionMode }} diff --git a/chart/values.yaml b/chart/values.yaml index 7587226..81f0d79 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -5,6 +5,8 @@ logLevel: 1 protectedNamespaces: - kube-system - openstack-system +additionalPrivilegedUsers: + - kubernetes-admin allowOpinionMode: false ingress: diff --git a/src/main.go b/src/main.go index 73f0530..7cee6a4 100644 --- a/src/main.go +++ b/src/main.go @@ -43,7 +43,7 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} -func isInternalAccount(user string, namespace string) bool { +func isPrivilegedForNamespace(user string, namespace string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") @@ -62,7 +62,7 @@ func isInternalAccount(user string, namespace string) bool { return false } -func createAuthorizer(protectedNamespaces []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { +func createAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { // Print dump for logging @@ -82,28 +82,33 @@ func createAuthorizer(protectedNamespaces []string, opinionMode bool, logLevel i defer r.Body.Close() - isPrivilegedUser := sar.Spec.ResourceAttributes != nil && isInternalAccount(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace) + isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) + userPrivilegedForNamespace := sar.Spec.ResourceAttributes != nil && isPrivilegedForNamespace(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace) isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) // TODO: test if you can bypass with empty or all namespaces isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" //TODO: test if you can bypass with * or singular nouns isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) status := new(authorizationv1.SubjectAccessReviewStatus) status.Allowed = false - if !isPrivilegedUser && isProtectedNamespace && isSecret { + if isPrivilegedUser { + status.Denied = false + status.Allowed = opinionMode + } else if !userPrivilegedForNamespace && isProtectedNamespace && isSecret { status.Denied = true status.Reason = "Cannot access secrets in protected namespace" - } else if !isPrivilegedUser && isProtectedNamespace && !isReadonlyVerb { + } else if !userPrivilegedForNamespace && isProtectedNamespace && !isReadonlyVerb { status.Denied = true status.Reason = "Cannot write to protected namespace" } else { status.Denied = false status.Allowed = opinionMode - if !opinionMode { - status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" - } } //todo: add status.EvaluationError handling + if !opinionMode && !status.Denied { + status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" + } + responseReview := new(SubjectAccessReviewHTTPResponse) responseReview.ApiVersion = "authorization.k8s.io/v1" responseReview.Kind = "SubjectAccessReview" @@ -133,14 +138,16 @@ func createAuthorizer(protectedNamespaces []string, opinionMode bool, logLevel i } func main() { + var additionalPrivilegedUsersCSL = flag.String("additional-privileged-users", "kubernetes-admin", "Comma separated list of users that should be allowed to write to protected namespaces, excluding 'system:*' users") var protectedNamespacesCSL = flag.String("protected-namespaces", "kube-system,openstack-system", "Comma separated list of namespaces which unprivileged users will have limited permissions for") var logLevel = flag.Int("log-level", 1, "Verbosity of logs. Values: [0-2]") var opinionMode = flag.Bool("allow-opinion-mode", false, "Specifies if this webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to true in SubjectAccessReview.") flag.Parse() protectedNamespaces := strings.Split(*protectedNamespacesCSL, ",") + additionalPrivilegedUsers := strings.Split(*additionalPrivilegedUsersCSL, ",") - http.HandleFunc("/authorize", createAuthorizer(protectedNamespaces, *opinionMode, *logLevel)) + http.HandleFunc("/authorize", createAuthorizer(protectedNamespaces, additionalPrivilegedUsers, *opinionMode, *logLevel)) fmt.Printf("Server started\n") err := http.ListenAndServe(":8080", nil) if err != nil { From cae45277a68b95a98f5d64b9c2f829511a83e2ad Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 29 May 2025 15:41:00 +0100 Subject: [PATCH 04/30] bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 81f0d79..a7dc75c 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: '5178677' +version: 5fd0285 logLevel: 1 protectedNamespaces: From b1cb8107e11def91b953d96850551be04b970965 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 10:52:32 +0100 Subject: [PATCH 05/30] added unit tests --- .github/workflows/unit-test.yml | 25 +++ src/main.go | 9 +- src/main_test.go | 291 ++++++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/unit-test.yml create mode 100644 src/main_test.go diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml new file mode 100644 index 0000000..e0e52ec --- /dev/null +++ b/.github/workflows/unit-test.yml @@ -0,0 +1,25 @@ +name: Go unit tests + +on: + pull_request: + push: + branches: + - main + +jobs: + build-and-test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: '1.24.x' + - name: Install dependencies + run: | + cd src + go get . + - name: Test with Go + run: | + cd src + go test -v diff --git a/src/main.go b/src/main.go index 7cee6a4..d8952db 100644 --- a/src/main.go +++ b/src/main.go @@ -48,7 +48,8 @@ func isPrivilegedForNamespace(user string, namespace string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") if err != nil { - fmt.Printf("Error compiling regex " + "\"system:serviceaccount:" + namespace + ":.+\": " + err.Error()) + fmt.Printf("Error compiling regex \"system:serviceaccount:%s:.+\": %s\n",namespace,err.Error()) + return false } if user == "system:anonymous" { @@ -62,7 +63,7 @@ func isPrivilegedForNamespace(user string, namespace string) bool { return false } -func createAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { +func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { // Print dump for logging @@ -75,7 +76,7 @@ func createAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers [] var sar SubjectAccessReviewAPI err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { - fmt.Println("[ERROR] ", err.Error()) + fmt.Println("JSON decoding error: ", err.Error()) http.Error(w, "Invalid JSON", http.StatusBadRequest) return } @@ -147,7 +148,7 @@ func main() { protectedNamespaces := strings.Split(*protectedNamespacesCSL, ",") additionalPrivilegedUsers := strings.Split(*additionalPrivilegedUsersCSL, ",") - http.HandleFunc("/authorize", createAuthorizer(protectedNamespaces, additionalPrivilegedUsers, *opinionMode, *logLevel)) + http.HandleFunc("/authorize", CreateWebhookAuthorizer(protectedNamespaces, additionalPrivilegedUsers, *opinionMode, *logLevel)) fmt.Printf("Server started\n") err := http.ListenAndServe(":8080", nil) if err != nil { diff --git a/src/main_test.go b/src/main_test.go new file mode 100644 index 0000000..02fd006 --- /dev/null +++ b/src/main_test.go @@ -0,0 +1,291 @@ +package main + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +var defaultProtectedNamespaces = []string{"kube-system", "openstack-system"} +var defaultAdditionalPrivilegedUsers = []string{} + +var defaultAuthorizer func(w http.ResponseWriter, r *http.Request) = CreateWebhookAuthorizer(defaultProtectedNamespaces, defaultAdditionalPrivilegedUsers, false, 0) + +func TestSystemUserAllowed(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:kube-controller-manager", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestNamespaceServiceAccountAllowed(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:serviceaccount:kube-system:good-service-account", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestWrongNamespaceServiceAccountDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,true, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:serviceaccount:othernamespace:bad-service-account", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestSystemAnonymousDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,true, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:anonymous", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestInvalidJSONDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,true,[]byte("{ bad json }")) +} + +func TestInvalidRegexDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"[badregexnamespace", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:serviceaccount:[badregexnamespace:bad-user", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestPrivilegedUserAllowed(t *testing.T){ + authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"kubernetes-admin"}, false, 0) + accessTest(t,authorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"kubernetes-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestUnprivilegedUserDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,true, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"kubernetes-not-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestReadUnprotectedSecretsAllowed(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"safe-namespace", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"kubernetes-not-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestReadProtectedNonSecretsAllowed(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"pods", + "name":"system-pod" + }, + "user":"kubernetes-not-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestWriteProtectedNonSecretsDenied(t *testing.T){ + accessTest(t,defaultAuthorizer,true, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"delete", + "version":"v1", + "resource":"pods", + "name":"system-pod" + }, + "user":"kubernetes-not-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestWriteUnprotectedResourcesAllowed(t *testing.T){ + accessTest(t,defaultAuthorizer,false, + []byte ( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"safe-namespace", + "verb":"delete", + "version":"v1", + "resource":"pods", + "name":"generic-pod" + }, + "user":"kubernetes-not-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func accessTest(t *testing.T,authorizer func(w http.ResponseWriter, r *http.Request), expectDenied bool, jsonData []byte) { + data := bytes.NewBuffer(jsonData) + req := httptest.NewRequest(http.MethodPost, "/authorize", data) + req.Header.Set("Content-Type", "application/json") + resp := httptest.NewRecorder() + authorizer(resp, req) + var sarResponse SubjectAccessReviewHTTPResponse + _ = json.NewDecoder(resp.Body).Decode(&sarResponse) + if(sarResponse.Status.Denied != expectDenied){ + var expectedResp string + if(expectDenied){ + expectedResp = "denied" + }else{ + expectedResp = "allowed" + } + t.Errorf("Expected request to be %s\n", expectedResp) + } +} From 1276b8f9330ca8d9c2e3d1de51380bc2ce61893f Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 11:00:39 +0100 Subject: [PATCH 06/30] fixed unprivileged service account being allowed --- src/main.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main.go b/src/main.go index d8952db..a119ea6 100644 --- a/src/main.go +++ b/src/main.go @@ -46,6 +46,7 @@ var readonlyVerbs = []string{"get", "list", "watch", "proxy"} func isPrivilegedForNamespace(user string, namespace string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") + serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") if err != nil { fmt.Printf("Error compiling regex \"system:serviceaccount:%s:.+\": %s\n",namespace,err.Error()) @@ -54,8 +55,8 @@ func isPrivilegedForNamespace(user string, namespace string) bool { if user == "system:anonymous" { return false - } else if namespaceServiceAccountRegex.MatchString(user) { - return true + } else if serviceAccountRegex.MatchString(user) { + return namespaceServiceAccountRegex.MatchString(user) } else if systemAccountRegex.MatchString(user) { return true } From e0c02d50c1272ff83cb1673796b8214e7275108f Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 11:20:10 +0100 Subject: [PATCH 07/30] fixed JSON test --- src/main.go | 2 +- src/main_test.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main.go b/src/main.go index a119ea6..805ae20 100644 --- a/src/main.go +++ b/src/main.go @@ -78,7 +78,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { fmt.Println("JSON decoding error: ", err.Error()) - http.Error(w, "Invalid JSON", http.StatusBadRequest) + http.Error(w, "JSON decoding error: "+err.Error(), http.StatusBadRequest) return } diff --git a/src/main_test.go b/src/main_test.go index 02fd006..4491c82 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -106,7 +106,14 @@ func TestSystemAnonymousDenied(t *testing.T){ } func TestInvalidJSONDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,true,[]byte("{ bad json }")) + data := bytes.NewBuffer([]byte("{ bad json }")) + req := httptest.NewRequest(http.MethodPost, "/authorize", data) + req.Header.Set("Content-Type", "application/json") + resp := httptest.NewRecorder() + defaultAuthorizer(resp, req) + if(resp.Code != http.StatusBadRequest){ + t.Error("Expected 400 error for invalid JSON") + } } func TestInvalidRegexDenied(t *testing.T){ From 20cfb575e2bfa1a8b5c8f556fc563d6b6d8c842f Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 11:27:10 +0100 Subject: [PATCH 08/30] formatting --- src/go.mod | 6 ++-- src/main.go | 2 +- src/main_test.go | 78 ++++++++++++++++++++++++------------------------ 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/go.mod b/src/go.mod index fa5f8c7..830f5fe 100644 --- a/src/go.mod +++ b/src/go.mod @@ -2,7 +2,10 @@ module azimuth-cloud/azimuth-authorizaton-webhook go 1.24.3 -require k8s.io/api v0.33.1 +require ( + k8s.io/api v0.33.1 + k8s.io/apimachinery v0.33.1 +) require ( github.com/fxamacker/cbor/v2 v2.7.0 // indirect @@ -16,7 +19,6 @@ require ( golang.org/x/net v0.38.0 // indirect golang.org/x/text v0.23.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect - k8s.io/apimachinery v0.33.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect diff --git a/src/main.go b/src/main.go index 805ae20..5d78403 100644 --- a/src/main.go +++ b/src/main.go @@ -49,7 +49,7 @@ func isPrivilegedForNamespace(user string, namespace string) bool { serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") if err != nil { - fmt.Printf("Error compiling regex \"system:serviceaccount:%s:.+\": %s\n",namespace,err.Error()) + fmt.Printf("Error compiling regex \"system:serviceaccount:%s:.+\": %s\n", namespace, err.Error()) return false } diff --git a/src/main_test.go b/src/main_test.go index 4491c82..d2352d6 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -13,9 +13,9 @@ var defaultAdditionalPrivilegedUsers = []string{} var defaultAuthorizer func(w http.ResponseWriter, r *http.Request) = CreateWebhookAuthorizer(defaultProtectedNamespaces, defaultAdditionalPrivilegedUsers, false, 0) -func TestSystemUserAllowed(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestSystemUserAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -36,9 +36,9 @@ func TestSystemUserAllowed(t *testing.T){ }`)) } -func TestNamespaceServiceAccountAllowed(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestNamespaceServiceAccountAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -59,9 +59,9 @@ func TestNamespaceServiceAccountAllowed(t *testing.T){ }`)) } -func TestWrongNamespaceServiceAccountDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,true, - []byte ( +func TestWrongNamespaceServiceAccountDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -82,9 +82,9 @@ func TestWrongNamespaceServiceAccountDenied(t *testing.T){ }`)) } -func TestSystemAnonymousDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,true, - []byte ( +func TestSystemAnonymousDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -105,20 +105,20 @@ func TestSystemAnonymousDenied(t *testing.T){ }`)) } -func TestInvalidJSONDenied(t *testing.T){ +func TestInvalidJSONDenied(t *testing.T) { data := bytes.NewBuffer([]byte("{ bad json }")) req := httptest.NewRequest(http.MethodPost, "/authorize", data) req.Header.Set("Content-Type", "application/json") resp := httptest.NewRecorder() defaultAuthorizer(resp, req) - if(resp.Code != http.StatusBadRequest){ + if resp.Code != http.StatusBadRequest { t.Error("Expected 400 error for invalid JSON") } } -func TestInvalidRegexDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestInvalidRegexDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -139,10 +139,10 @@ func TestInvalidRegexDenied(t *testing.T){ }`)) } -func TestPrivilegedUserAllowed(t *testing.T){ +func TestPrivilegedUserAllowed(t *testing.T) { authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"kubernetes-admin"}, false, 0) - accessTest(t,authorizer,false, - []byte ( + accessTest(t, authorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -163,9 +163,9 @@ func TestPrivilegedUserAllowed(t *testing.T){ }`)) } -func TestUnprivilegedUserDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,true, - []byte ( +func TestUnprivilegedUserDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -186,9 +186,9 @@ func TestUnprivilegedUserDenied(t *testing.T){ }`)) } -func TestReadUnprotectedSecretsAllowed(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestReadUnprotectedSecretsAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -209,9 +209,9 @@ func TestReadUnprotectedSecretsAllowed(t *testing.T){ }`)) } -func TestReadProtectedNonSecretsAllowed(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestReadProtectedNonSecretsAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -232,9 +232,9 @@ func TestReadProtectedNonSecretsAllowed(t *testing.T){ }`)) } -func TestWriteProtectedNonSecretsDenied(t *testing.T){ - accessTest(t,defaultAuthorizer,true, - []byte ( +func TestWriteProtectedNonSecretsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -255,9 +255,9 @@ func TestWriteProtectedNonSecretsDenied(t *testing.T){ }`)) } -func TestWriteUnprotectedResourcesAllowed(t *testing.T){ - accessTest(t,defaultAuthorizer,false, - []byte ( +func TestWriteUnprotectedResourcesAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( `{ "kind":"SubjectAccessReview", "apiVersion":"authorization.k8s.io/v1", @@ -278,7 +278,7 @@ func TestWriteUnprotectedResourcesAllowed(t *testing.T){ }`)) } -func accessTest(t *testing.T,authorizer func(w http.ResponseWriter, r *http.Request), expectDenied bool, jsonData []byte) { +func accessTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Request), expectDenied bool, jsonData []byte) { data := bytes.NewBuffer(jsonData) req := httptest.NewRequest(http.MethodPost, "/authorize", data) req.Header.Set("Content-Type", "application/json") @@ -286,11 +286,11 @@ func accessTest(t *testing.T,authorizer func(w http.ResponseWriter, r *http.Requ authorizer(resp, req) var sarResponse SubjectAccessReviewHTTPResponse _ = json.NewDecoder(resp.Body).Decode(&sarResponse) - if(sarResponse.Status.Denied != expectDenied){ + if sarResponse.Status.Denied != expectDenied { var expectedResp string - if(expectDenied){ + if expectDenied { expectedResp = "denied" - }else{ + } else { expectedResp = "allowed" } t.Errorf("Expected request to be %s\n", expectedResp) From 593563339f937d9d55d58084adc8d50b17f5b6f8 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 11:34:47 +0100 Subject: [PATCH 09/30] image bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index a7dc75c..19497cc 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: 5fd0285 +version: df39e6e logLevel: 1 protectedNamespaces: From 68399ec97ef7c485722bf9318b8f2498d2f7db73 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 12:16:04 +0100 Subject: [PATCH 10/30] now allows protected service accounts to write to each other's namespaces --- src/main.go | 13 +++++-------- src/main_test.go | 29 ++++++----------------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/src/main.go b/src/main.go index 5d78403..e693deb 100644 --- a/src/main.go +++ b/src/main.go @@ -43,20 +43,17 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} -func isPrivilegedForNamespace(user string, namespace string) bool { +func isPrivilegedForNamespace(user string, namespace string, protectedNamespaces []string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") - namespaceServiceAccountRegex, err := regexp.Compile("system:serviceaccount:" + namespace + ":.+") - if err != nil { - fmt.Printf("Error compiling regex \"system:serviceaccount:%s:.+\": %s\n", namespace, err.Error()) - return false - } if user == "system:anonymous" { return false } else if serviceAccountRegex.MatchString(user) { - return namespaceServiceAccountRegex.MatchString(user) + // Considers service accounts from protected namespaces as privileged + serviceAccountNamespace := strings.Split(user, ":")[2] + return slices.Contains(protectedNamespaces, serviceAccountNamespace) } else if systemAccountRegex.MatchString(user) { return true } @@ -85,7 +82,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU defer r.Body.Close() isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) - userPrivilegedForNamespace := sar.Spec.ResourceAttributes != nil && isPrivilegedForNamespace(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace) + userPrivilegedForNamespace := sar.Spec.ResourceAttributes != nil && isPrivilegedForNamespace(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace, protectedNamespaces) isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) // TODO: test if you can bypass with empty or all namespaces isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" //TODO: test if you can bypass with * or singular nouns isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) diff --git a/src/main_test.go b/src/main_test.go index d2352d6..544bf27 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -59,6 +59,12 @@ func TestNamespaceServiceAccountAllowed(t *testing.T) { }`)) } +func TestCrossProtectedNamespaceServiceAccountAccessAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( + `{"kind":"SubjectAccessReview","apiVersion":"authorization.k8s.io/v1","metadata":{"creationTimestamp":null},"spec":{"resourceAttributes":{"namespace":"openstack-system","verb":"create","group":"apps","version":"v1","resource":"controllerrevisions"},"user":"system:serviceaccount:kube-system:daemon-set-controller","groups":["system:serviceaccounts","system:serviceaccounts:kube-system","system:authenticated"],"extra":{"authentication.kubernetes.io/credential-id":["JTI=3cf7d9de-5324-4df7-9447-47adb900f846"]},"uid":"cb35e0b5-1cfb-432f-acdf-5b5a0f924211"},"status":{"allowed":false}}`)) +} + func TestWrongNamespaceServiceAccountDenied(t *testing.T) { accessTest(t, defaultAuthorizer, true, []byte( @@ -116,29 +122,6 @@ func TestInvalidJSONDenied(t *testing.T) { } } -func TestInvalidRegexDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, false, - []byte( - `{ - "kind":"SubjectAccessReview", - "apiVersion":"authorization.k8s.io/v1", - "spec":{ - "resourceAttributes":{ - "namespace":"[badregexnamespace", - "verb":"get", - "version":"v1", - "resource":"secrets", - "name":"important-creds" - }, - "user":"system:serviceaccount:[badregexnamespace:bad-user", - "groups":["system:authenticated"] - }, - "status":{ - "allowed":false - } - }`)) -} - func TestPrivilegedUserAllowed(t *testing.T) { authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"kubernetes-admin"}, false, 0) accessTest(t, authorizer, false, From f67fe91d7832cbfb5550ceb1ee9ea5d7b692f3b1 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Fri, 30 May 2025 14:28:32 +0100 Subject: [PATCH 11/30] bumped image --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 19497cc..8251562 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: df39e6e +version: 866cb1a logLevel: 1 protectedNamespaces: From e00a1fddafbc7c1c240f1608e1199af79ad21c3d Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 10:17:37 +0100 Subject: [PATCH 12/30] added protections for wildcard requests --- src/main.go | 21 +++++--- src/main_test.go | 138 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 7 deletions(-) diff --git a/src/main.go b/src/main.go index e693deb..1fc72f6 100644 --- a/src/main.go +++ b/src/main.go @@ -43,7 +43,7 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} -func isPrivilegedForNamespace(user string, namespace string, protectedNamespaces []string) bool { +func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") @@ -82,27 +82,34 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU defer r.Body.Close() isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) - userPrivilegedForNamespace := sar.Spec.ResourceAttributes != nil && isPrivilegedForNamespace(sar.Spec.User, sar.Spec.ResourceAttributes.Namespace, protectedNamespaces) - isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) // TODO: test if you can bypass with empty or all namespaces - isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" //TODO: test if you can bypass with * or singular nouns + isPrivilegedSystemUser := sar.Spec.ResourceAttributes != nil && isPrivilegedSystemUser(sar.Spec.User, protectedNamespaces) + isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) + isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) + isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && (sar.Spec.ResourceAttributes.Namespace == "" || sar.Spec.ResourceAttributes.Namespace == "all") + isAllResourceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "*" status := new(authorizationv1.SubjectAccessReviewStatus) status.Allowed = false if isPrivilegedUser { status.Denied = false status.Allowed = opinionMode - } else if !userPrivilegedForNamespace && isProtectedNamespace && isSecret { + } else if !isPrivilegedSystemUser && isAllNamespaceRequest { + status.Denied = true + status.Reason = "Cannot make all namespace requests" + } else if isProtectedNamespace && !isPrivilegedSystemUser && isAllResourceRequest { + status.Denied = true + status.Reason = "Cannot make all resource requests in protected namespace" + } else if isProtectedNamespace && !isPrivilegedSystemUser && isSecret { status.Denied = true status.Reason = "Cannot access secrets in protected namespace" - } else if !userPrivilegedForNamespace && isProtectedNamespace && !isReadonlyVerb { + } else if isProtectedNamespace && !isPrivilegedSystemUser && !isReadonlyVerb { status.Denied = true status.Reason = "Cannot write to protected namespace" } else { status.Denied = false status.Allowed = opinionMode } - //todo: add status.EvaluationError handling if !opinionMode && !status.Denied { status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" diff --git a/src/main_test.go b/src/main_test.go index 544bf27..7de7040 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -261,6 +261,144 @@ func TestWriteUnprotectedResourcesAllowed(t *testing.T) { }`)) } +func TestEmptyNamespacesRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestAllNamespacesRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"all", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestProtectedReadAllRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"*", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestProtectedWriteAllRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"delete", + "version":"v1", + "resource":"*", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestProtectedAllVerbRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"*", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestProtectedAllVerbNonSecretRequestsDenied(t *testing.T) { + accessTest(t, defaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"*", + "version":"v1", + "resource":"pods", + "name":"important-creds" + }, + "user":"not-admin", + "groups":["group1"] + }, + "status":{ + "allowed":false + } + }`)) +} + func accessTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Request), expectDenied bool, jsonData []byte) { data := bytes.NewBuffer(jsonData) req := httptest.NewRequest(http.MethodPost, "/authorize", data) From 69a4d1602350373f6ea7437af5eb3a9fa88a4e7d Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 11:07:26 +0100 Subject: [PATCH 13/30] refactor + docs --- README.md | 19 +++++++++++++ src/main.go | 82 ++++++++++++++++++++++++++++++++--------------------- 2 files changed, 69 insertions(+), 32 deletions(-) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..88f68b9 --- /dev/null +++ b/README.md @@ -0,0 +1,19 @@ +# Azimuth Authorization Webhook + +A Kubernetes authorization webhook to protect sensitive namespaces when users require +read-write access to all other cluster resources (e.g when they wish to install arbitrary CRDS). + +Policy: +- Users cannot read secrets in protected namespaces by default +- Users cannot write any other resource in protected namespaces by default +- Internal K8s `system:` users may read/write to protected namespaces, excluding service accounts and `system:anonymous` +- Service accounts in protected namespaces may read/write to all protected namespaces +- Users specified as privileged may read/write to protected namespaces + +## Flags +| Flag | Arguments | +| --- | --- | +| `--allow-opinion-mode` | Specifies if the webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to `true` in SubjectAccessReview response. Default: `false` | +| `--additional-privileged-users` | Comma separate listed of users to be given read/write access to protected namespaces. Default: `kubernetes-admin` | +| `--log-level` | Verbosity of logs
`0`: Internal errors only.
`1`: Logs high level requests info.
`2`: Logs HTTP dumps of requests.
Default: `1` | +| `--protected-namespaces` | Comma separated list of protected namespaces. Default: `kube-system,openstack-system` | diff --git a/src/main.go b/src/main.go index 1fc72f6..3153b82 100644 --- a/src/main.go +++ b/src/main.go @@ -17,6 +17,7 @@ import ( // Creating mirror of authorizationv1.SubjectAccessReview struct but with modified Spec // to account for disparity in name of group key between Go ('Groups') and HTTP ('Group') API // causing issues with JSON unmarshalling +// Should not be written as HTTP response type SubjectAccessReviewAPI struct { metav1.TypeMeta metav1.ObjectMeta @@ -35,6 +36,8 @@ type SubjectAccessReviewSpecAPI struct { UID string } + +// Minimal SubjectAccessReview HTTP response type SubjectAccessReviewHTTPResponse struct { ApiVersion string `json:"apiVersion"` Kind string `json:"kind"` @@ -43,6 +46,7 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} +// Returns true if user is a service account with correct privileges or a privileged internal K8s system user func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") @@ -51,20 +55,54 @@ func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { if user == "system:anonymous" { return false } else if serviceAccountRegex.MatchString(user) { - // Considers service accounts from protected namespaces as privileged + // Allows service accounts if they originate from protected namespaces serviceAccountNamespace := strings.Split(user, ":")[2] return slices.Contains(protectedNamespaces, serviceAccountNamespace) } else if systemAccountRegex.MatchString(user) { + // All other system accounts allowed return true } return false } +// Returns true if request passes webhook's resource access checks. If false, string with reason for rejection will also be returned, otherwise nil string +func isRequestAuthorized(sar SubjectAccessReviewAPI,protectedNamespaces []string,additionalPrivilegedUsers []string) (bool, string) { + isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) + isPrivilegedSystemUser := sar.Spec.ResourceAttributes != nil && isPrivilegedSystemUser(sar.Spec.User, protectedNamespaces) + isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) + isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" + isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) + isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && (sar.Spec.ResourceAttributes.Namespace == "" || sar.Spec.ResourceAttributes.Namespace == "all") + isAllResourceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "*" + + var denyReason string + authorized := false + if isPrivilegedUser { + authorized = true + } else if !isPrivilegedSystemUser && isAllNamespaceRequest { + authorized = false + denyReason = "Cannot make all namespace requests" + } else if isProtectedNamespace && !isPrivilegedSystemUser && isAllResourceRequest { + authorized = false + denyReason = "Cannot make all resource requests in protected namespace" + } else if isProtectedNamespace && !isPrivilegedSystemUser && isSecret { + authorized = false + denyReason = "Cannot access secrets in protected namespace" + } else if isProtectedNamespace && !isPrivilegedSystemUser && !isReadonlyVerb { + authorized = false + denyReason = "Cannot write to protected namespace" + } else { + authorized = true + } + return authorized, denyReason +} + + +// Returns HTTP request handler to handle SubjectAccessReview API requests func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - // Print dump for logging dump, dumperr := httputil.DumpRequest(r, true) if dumperr != nil { fmt.Println("Error dumping request:", dumperr) @@ -81,38 +119,18 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU defer r.Body.Close() - isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) - isPrivilegedSystemUser := sar.Spec.ResourceAttributes != nil && isPrivilegedSystemUser(sar.Spec.User, protectedNamespaces) - isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) - isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" - isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) - isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && (sar.Spec.ResourceAttributes.Namespace == "" || sar.Spec.ResourceAttributes.Namespace == "all") - isAllResourceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "*" - + authorized, denyReason := isRequestAuthorized(sar,protectedNamespaces,additionalPrivilegedUsers) + status := new(authorizationv1.SubjectAccessReviewStatus) - status.Allowed = false - if isPrivilegedUser { - status.Denied = false - status.Allowed = opinionMode - } else if !isPrivilegedSystemUser && isAllNamespaceRequest { - status.Denied = true - status.Reason = "Cannot make all namespace requests" - } else if isProtectedNamespace && !isPrivilegedSystemUser && isAllResourceRequest { - status.Denied = true - status.Reason = "Cannot make all resource requests in protected namespace" - } else if isProtectedNamespace && !isPrivilegedSystemUser && isSecret { - status.Denied = true - status.Reason = "Cannot access secrets in protected namespace" - } else if isProtectedNamespace && !isPrivilegedSystemUser && !isReadonlyVerb { - status.Denied = true - status.Reason = "Cannot write to protected namespace" - } else { - status.Denied = false - status.Allowed = opinionMode - } - - if !opinionMode && !status.Denied { + status.Denied = !authorized + + if(status.Denied){ + status.Reason = denyReason + }else if(!opinionMode){ + status.Allowed = false status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" + }else{ + status.Allowed = true } responseReview := new(SubjectAccessReviewHTTPResponse) From b83f9780cf6909605e043cf44717c6cdaababd14 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 11:14:26 +0100 Subject: [PATCH 14/30] bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 8251562..95cfc78 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: 866cb1a +version: eb9da50 logLevel: 1 protectedNamespaces: From d9eaf9942ff008be182dc2e68c19a90bca756ccc Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 13:29:51 +0100 Subject: [PATCH 15/30] ensured allowed is always set --- src/main.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main.go b/src/main.go index 3153b82..40a2c56 100644 --- a/src/main.go +++ b/src/main.go @@ -123,14 +123,12 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU status := new(authorizationv1.SubjectAccessReviewStatus) status.Denied = !authorized + status.Allowed = opinionMode && authorized if(status.Denied){ status.Reason = denyReason }else if(!opinionMode){ - status.Allowed = false status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" - }else{ - status.Allowed = true } responseReview := new(SubjectAccessReviewHTTPResponse) From 92c6200360b67e2d94170931552966879c8b3a14 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 13:36:06 +0100 Subject: [PATCH 16/30] bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 95cfc78..2ca75fa 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: eb9da50 +version: 6adae90 logLevel: 1 protectedNamespaces: From a3fa051af9f00a886efecd62b9a8697add673570 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 14:51:39 +0100 Subject: [PATCH 17/30] loosened all-namespace request rules --- README.md | 2 +- chart/values.yaml | 1 + src/main.go | 12 +++++------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 88f68b9..a52d77c 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,6 @@ Policy: | Flag | Arguments | | --- | --- | | `--allow-opinion-mode` | Specifies if the webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to `true` in SubjectAccessReview response. Default: `false` | -| `--additional-privileged-users` | Comma separate listed of users to be given read/write access to protected namespaces. Default: `kubernetes-admin` | +| `--additional-privileged-users` | Comma separate listed of users to be given read/write access to protected namespaces. Default: `kubernetes-admin,kube-apiserver-kubelet-client` | | `--log-level` | Verbosity of logs
`0`: Internal errors only.
`1`: Logs high level requests info.
`2`: Logs HTTP dumps of requests.
Default: `1` | | `--protected-namespaces` | Comma separated list of protected namespaces. Default: `kube-system,openstack-system` | diff --git a/chart/values.yaml b/chart/values.yaml index 2ca75fa..1203337 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -7,6 +7,7 @@ protectedNamespaces: - openstack-system additionalPrivilegedUsers: - kubernetes-admin + - kube-apiserver-kubelet-client allowOpinionMode: false ingress: diff --git a/src/main.go b/src/main.go index 40a2c56..284410c 100644 --- a/src/main.go +++ b/src/main.go @@ -75,21 +75,19 @@ func isRequestAuthorized(sar SubjectAccessReviewAPI,protectedNamespaces []string isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && (sar.Spec.ResourceAttributes.Namespace == "" || sar.Spec.ResourceAttributes.Namespace == "all") isAllResourceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "*" + isSensitiveNamespaceRequest := isAllNamespaceRequest || isProtectedNamespace var denyReason string authorized := false if isPrivilegedUser { authorized = true - } else if !isPrivilegedSystemUser && isAllNamespaceRequest { - authorized = false - denyReason = "Cannot make all namespace requests" - } else if isProtectedNamespace && !isPrivilegedSystemUser && isAllResourceRequest { + } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && isAllResourceRequest { authorized = false denyReason = "Cannot make all resource requests in protected namespace" - } else if isProtectedNamespace && !isPrivilegedSystemUser && isSecret { + } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && isSecret { authorized = false denyReason = "Cannot access secrets in protected namespace" - } else if isProtectedNamespace && !isPrivilegedSystemUser && !isReadonlyVerb { + } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && !isReadonlyVerb { authorized = false denyReason = "Cannot write to protected namespace" } else { @@ -160,7 +158,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU } func main() { - var additionalPrivilegedUsersCSL = flag.String("additional-privileged-users", "kubernetes-admin", "Comma separated list of users that should be allowed to write to protected namespaces, excluding 'system:*' users") + var additionalPrivilegedUsersCSL = flag.String("additional-privileged-users", "kubernetes-admin,kube-apiserver-kubelet-client", "Comma separated list of users that should be allowed to write to protected namespaces, excluding 'system:*' users") var protectedNamespacesCSL = flag.String("protected-namespaces", "kube-system,openstack-system", "Comma separated list of namespaces which unprivileged users will have limited permissions for") var logLevel = flag.Int("log-level", 1, "Verbosity of logs. Values: [0-2]") var opinionMode = flag.Bool("allow-opinion-mode", false, "Specifies if this webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to true in SubjectAccessReview.") From 59811f44af1da02d81edaff5ee916f67fb078a2e Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 15:01:40 +0100 Subject: [PATCH 18/30] bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 1203337..2ec14c5 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: 6adae90 +version: f31ac8a logLevel: 1 protectedNamespaces: From 7ee025d7119b9b084ed920728ac7009e5b57276c Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 15:24:48 +0100 Subject: [PATCH 19/30] all namespace bans now only apply to secrets --- src/main.go | 25 +++++++++++-------------- src/main_test.go | 23 ----------------------- 2 files changed, 11 insertions(+), 37 deletions(-) diff --git a/src/main.go b/src/main.go index 284410c..95376e2 100644 --- a/src/main.go +++ b/src/main.go @@ -36,7 +36,6 @@ type SubjectAccessReviewSpecAPI struct { UID string } - // Minimal SubjectAccessReview HTTP response type SubjectAccessReviewHTTPResponse struct { ApiVersion string `json:"apiVersion"` @@ -67,27 +66,26 @@ func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { } // Returns true if request passes webhook's resource access checks. If false, string with reason for rejection will also be returned, otherwise nil string -func isRequestAuthorized(sar SubjectAccessReviewAPI,protectedNamespaces []string,additionalPrivilegedUsers []string) (bool, string) { +func isRequestAuthorized(sar SubjectAccessReviewAPI, protectedNamespaces []string, additionalPrivilegedUsers []string) (bool, string) { isPrivilegedUser := slices.Contains(additionalPrivilegedUsers, sar.Spec.User) isPrivilegedSystemUser := sar.Spec.ResourceAttributes != nil && isPrivilegedSystemUser(sar.Spec.User, protectedNamespaces) isProtectedNamespace := sar.Spec.ResourceAttributes != nil && slices.Contains(protectedNamespaces, sar.Spec.ResourceAttributes.Namespace) isSecret := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "secrets" isReadonlyVerb := sar.Spec.ResourceAttributes != nil && slices.Contains(readonlyVerbs, sar.Spec.ResourceAttributes.Verb) - isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && (sar.Spec.ResourceAttributes.Namespace == "" || sar.Spec.ResourceAttributes.Namespace == "all") + isAllNamespaceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Namespace == "" isAllResourceRequest := sar.Spec.ResourceAttributes != nil && sar.Spec.ResourceAttributes.Resource == "*" - isSensitiveNamespaceRequest := isAllNamespaceRequest || isProtectedNamespace var denyReason string authorized := false if isPrivilegedUser { authorized = true - } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && isAllResourceRequest { + } else if isProtectedNamespace && !isPrivilegedSystemUser && isAllResourceRequest { authorized = false - denyReason = "Cannot make all resource requests in protected namespace" - } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && isSecret { + denyReason = "Cannot make * resource requests in protected namespace" + } else if (isAllNamespaceRequest || isProtectedNamespace) && !isPrivilegedSystemUser && isSecret { authorized = false denyReason = "Cannot access secrets in protected namespace" - } else if isSensitiveNamespaceRequest && !isPrivilegedSystemUser && !isReadonlyVerb { + } else if isProtectedNamespace && !isPrivilegedSystemUser && !isReadonlyVerb { authorized = false denyReason = "Cannot write to protected namespace" } else { @@ -96,7 +94,6 @@ func isRequestAuthorized(sar SubjectAccessReviewAPI,protectedNamespaces []string return authorized, denyReason } - // Returns HTTP request handler to handle SubjectAccessReview API requests func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -117,15 +114,15 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU defer r.Body.Close() - authorized, denyReason := isRequestAuthorized(sar,protectedNamespaces,additionalPrivilegedUsers) - + authorized, denyReason := isRequestAuthorized(sar, protectedNamespaces, additionalPrivilegedUsers) + status := new(authorizationv1.SubjectAccessReviewStatus) status.Denied = !authorized status.Allowed = opinionMode && authorized - - if(status.Denied){ + + if status.Denied { status.Reason = denyReason - }else if(!opinionMode){ + } else if !opinionMode { status.Reason = "Webhook doesn't give opinion, delegated to other authorizers" } diff --git a/src/main_test.go b/src/main_test.go index 7de7040..a72fd0b 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -284,29 +284,6 @@ func TestEmptyNamespacesRequestsDenied(t *testing.T) { }`)) } -func TestAllNamespacesRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, - []byte( - `{ - "kind":"SubjectAccessReview", - "apiVersion":"authorization.k8s.io/v1", - "spec":{ - "resourceAttributes":{ - "namespace":"all", - "verb":"get", - "version":"v1", - "resource":"secrets", - "name":"important-creds" - }, - "user":"not-admin", - "groups":["group1"] - }, - "status":{ - "allowed":false - } - }`)) -} - func TestProtectedReadAllRequestsDenied(t *testing.T) { accessTest(t, defaultAuthorizer, true, []byte( From 830fc0e8e8668d558eb9bfaf7b7d3835c4b58646 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Mon, 2 Jun 2025 15:33:13 +0100 Subject: [PATCH 20/30] bump --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 2ec14c5..3b3b6ce 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: f31ac8a +version: 13a5067 logLevel: 1 protectedNamespaces: From 217ce15a1e7dbbfd26754a59c16ca28dd68dc20e Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 10:07:07 +0100 Subject: [PATCH 21/30] nits --- src/main.go | 5 +++-- src/main_test.go | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main.go b/src/main.go index 95376e2..a2d27bd 100644 --- a/src/main.go +++ b/src/main.go @@ -107,8 +107,9 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU var sar SubjectAccessReviewAPI err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { - fmt.Println("JSON decoding error: ", err.Error()) - http.Error(w, "JSON decoding error: "+err.Error(), http.StatusBadRequest) + errString = "JSON decoding error: " + err.Error() + fmt.Println(errString) + http.Error(w, errString, http.StatusBadRequest) return } diff --git a/src/main_test.go b/src/main_test.go index a72fd0b..b3dc5cd 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -381,7 +381,9 @@ func accessTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Req req := httptest.NewRequest(http.MethodPost, "/authorize", data) req.Header.Set("Content-Type", "application/json") resp := httptest.NewRecorder() + authorizer(resp, req) + var sarResponse SubjectAccessReviewHTTPResponse _ = json.NewDecoder(resp.Body).Decode(&sarResponse) if sarResponse.Status.Denied != expectDenied { From 4fa40af4b4593e8540b293541bd221b0029e6134 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 10:21:29 +0100 Subject: [PATCH 22/30] added gofmt check --- .github/workflows/unit-test.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index e0e52ec..b6a1944 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -15,6 +15,14 @@ jobs: uses: actions/setup-go@v5 with: go-version: '1.24.x' + - name: Check gofmt compliant + run: | + cd src + if [[ $(gofmt -l .) ]]; then + exit 1 + else + exit 0 + fi - name: Install dependencies run: | cd src From 16a5e3ddddd5c86ca2b9b23bbdc26649ccb8dc42 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 10:23:57 +0100 Subject: [PATCH 23/30] ci output + fixed formatting --- .github/workflows/unit-test.yml | 2 ++ src/main_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index b6a1944..a1a1263 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -19,6 +19,8 @@ jobs: run: | cd src if [[ $(gofmt -l .) ]]; then + echo "Files not gofmt compliant:" + gofmt -l . exit 1 else exit 0 diff --git a/src/main_test.go b/src/main_test.go index b3dc5cd..606a453 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -383,7 +383,7 @@ func accessTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Req resp := httptest.NewRecorder() authorizer(resp, req) - + var sarResponse SubjectAccessReviewHTTPResponse _ = json.NewDecoder(resp.Body).Decode(&sarResponse) if sarResponse.Status.Denied != expectDenied { From e4b5a15d34baf2f017b4933e15379e329c67723f Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 10:27:57 +0100 Subject: [PATCH 24/30] typo --- src/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main.go b/src/main.go index a2d27bd..8444393 100644 --- a/src/main.go +++ b/src/main.go @@ -107,9 +107,9 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU var sar SubjectAccessReviewAPI err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { - errString = "JSON decoding error: " + err.Error() - fmt.Println(errString) - http.Error(w, errString, http.StatusBadRequest) + jsonErrString := "JSON decoding error: " + err.Error() + fmt.Println(jsonErrString) + http.Error(w, jsonErrString, http.StatusBadRequest) return } From 97028f8719bb942f1262be268e18edc8a9ebdb9d Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 12:29:28 +0100 Subject: [PATCH 25/30] Logging improvements --- src/main.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main.go b/src/main.go index 8444393..33bd0b3 100644 --- a/src/main.go +++ b/src/main.go @@ -3,9 +3,9 @@ package main import ( "encoding/json" "flag" - "fmt" authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "log" "net/http" "net/http/httputil" "os" @@ -100,7 +100,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU dump, dumperr := httputil.DumpRequest(r, true) if dumperr != nil { - fmt.Println("Error dumping request:", dumperr) + log.Println("Error dumping request:", dumperr) return } @@ -108,7 +108,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU err := json.NewDecoder(r.Body).Decode(&sar) if err != nil { jsonErrString := "JSON decoding error: " + err.Error() - fmt.Println(jsonErrString) + log.Println(jsonErrString) http.Error(w, jsonErrString, http.StatusBadRequest) return } @@ -138,16 +138,16 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU } else { deniedLogOutput = "Allowed" } + + // TODO: find way to map cluster IPs from X-Forward headers to clusters if logLevel >= 1 && sar.Spec.NonResourceAttributes != nil { - fmt.Printf("%s non-resource request from \"%s\". Reason: %s\n", deniedLogOutput, sar.Spec.User, status.Reason) + log.Println("[Cluster: " + r.Header.Get("X-Forwarded-For") + "] " + deniedLogOutput + " non-resource request from " + sar.Spec.User + ". Reason: " + status.Reason) } if logLevel >= 1 && sar.Spec.ResourceAttributes != nil { - fmt.Printf("%s request from \"%s\" to \"%s\" \"%s\" in namespace \"%s\". Reason: %s \n", - deniedLogOutput, sar.Spec.User, sar.Spec.ResourceAttributes.Verb, sar.Spec.ResourceAttributes.Resource, sar.Spec.ResourceAttributes.Namespace, status.Reason) + log.Println("[Cluster: " + r.Header.Get("X-Forwarded-For") + "] " + deniedLogOutput + " request from " + sar.Spec.User + " to " + sar.Spec.ResourceAttributes.Verb + " " + sar.Spec.ResourceAttributes.Resource + " in namespace " + sar.Spec.ResourceAttributes.Namespace + ". Reason: " + status.Reason) } if logLevel >= 2 { - fmt.Println("HTTP Dump:") - fmt.Println(string(dump)) + log.Printf("HTTP Dump: \n%s\n", string(dump)) } w.Header().Set("Content-Type", "application/json") @@ -166,10 +166,10 @@ func main() { additionalPrivilegedUsers := strings.Split(*additionalPrivilegedUsersCSL, ",") http.HandleFunc("/authorize", CreateWebhookAuthorizer(protectedNamespaces, additionalPrivilegedUsers, *opinionMode, *logLevel)) - fmt.Printf("Server started\n") + log.Printf("Server started\n") err := http.ListenAndServe(":8080", nil) if err != nil { - fmt.Printf("error starting server: %s\n", err) + log.Printf("error starting server: %s\n", err) os.Exit(1) } } From 0acc2bb9e893e766a6f0dcb920a09cc51a62df34 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 13:21:52 +0100 Subject: [PATCH 26/30] debug --- src/main.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main.go b/src/main.go index 33bd0b3..90d73ce 100644 --- a/src/main.go +++ b/src/main.go @@ -45,11 +45,19 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} +var debuglist = []string{} + // Returns true if user is a service account with correct privileges or a privileged internal K8s system user func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { systemAccountRegex, _ := regexp.Compile("system:.+") serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") + nodeAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") + + if(!nodeAccountRegex.MatchString(user) && !serviceAccountRegex.MatchString(user) && !slices.Contains(debuglist, user)){ + debuglist = append(debuglist, user) + log.Println(user) + } if user == "system:anonymous" { return false From e9f1d62856281534dd2e7cc8db6b372d8aca7e7d Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Wed, 18 Jun 2025 15:54:02 +0100 Subject: [PATCH 27/30] made allowed system users more explicit --- README.md | 2 +- chart/values.yaml | 4 +-- src/main.go | 22 +++++--------- src/main_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 82 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index a52d77c..585e916 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,6 @@ Policy: | Flag | Arguments | | --- | --- | | `--allow-opinion-mode` | Specifies if the webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to `true` in SubjectAccessReview response. Default: `false` | -| `--additional-privileged-users` | Comma separate listed of users to be given read/write access to protected namespaces. Default: `kubernetes-admin,kube-apiserver-kubelet-client` | +| `--additional-privileged-users` | Comma separate listed of users to be given read/write access to protected namespaces. Default: `""` | | `--log-level` | Verbosity of logs
`0`: Internal errors only.
`1`: Logs high level requests info.
`2`: Logs HTTP dumps of requests.
Default: `1` | | `--protected-namespaces` | Comma separated list of protected namespaces. Default: `kube-system,openstack-system` | diff --git a/chart/values.yaml b/chart/values.yaml index 3b3b6ce..4459b73 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -5,9 +5,7 @@ logLevel: 1 protectedNamespaces: - kube-system - openstack-system -additionalPrivilegedUsers: - - kubernetes-admin - - kube-apiserver-kubelet-client +additionalPrivilegedUsers: [] allowOpinionMode: false ingress: diff --git a/src/main.go b/src/main.go index 90d73ce..84d4986 100644 --- a/src/main.go +++ b/src/main.go @@ -45,28 +45,22 @@ type SubjectAccessReviewHTTPResponse struct { var readonlyVerbs = []string{"get", "list", "watch", "proxy"} -var debuglist = []string{} - // Returns true if user is a service account with correct privileges or a privileged internal K8s system user func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { - systemAccountRegex, _ := regexp.Compile("system:.+") + requiredUsers := []string{"system:kube-controller-manager", "system:kube-scheduler", "kubernetes-admin", "kube-apiserver-kubelet-client"} serviceAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") - nodeAccountRegex, _ := regexp.Compile("system:serviceaccount:.+") - - if(!nodeAccountRegex.MatchString(user) && !serviceAccountRegex.MatchString(user) && !slices.Contains(debuglist, user)){ - debuglist = append(debuglist, user) - log.Println(user) - } + nodeAccountRegex, _ := regexp.Compile("system:node:.+") + bootstrapAccountRegex, _ := regexp.Compile("system:bootstrap:.+") - if user == "system:anonymous" { - return false + if slices.Contains(requiredUsers, user) { + return true } else if serviceAccountRegex.MatchString(user) { // Allows service accounts if they originate from protected namespaces serviceAccountNamespace := strings.Split(user, ":")[2] return slices.Contains(protectedNamespaces, serviceAccountNamespace) - } else if systemAccountRegex.MatchString(user) { - // All other system accounts allowed + } else if nodeAccountRegex.MatchString(user) || bootstrapAccountRegex.MatchString(user) { + // All node and bootstrap accounts allowed return true } @@ -164,7 +158,7 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU } func main() { - var additionalPrivilegedUsersCSL = flag.String("additional-privileged-users", "kubernetes-admin,kube-apiserver-kubelet-client", "Comma separated list of users that should be allowed to write to protected namespaces, excluding 'system:*' users") + var additionalPrivilegedUsersCSL = flag.String("additional-privileged-users", "", "Comma separated list of users that should be allowed to write to protected namespaces, excluding 'system:*' users") var protectedNamespacesCSL = flag.String("protected-namespaces", "kube-system,openstack-system", "Comma separated list of namespaces which unprivileged users will have limited permissions for") var logLevel = flag.Int("log-level", 1, "Verbosity of logs. Values: [0-2]") var opinionMode = flag.Bool("allow-opinion-mode", false, "Specifies if this webhook should give its opinion on requests which it doesn't deny. If true, will set 'allowed' to true in SubjectAccessReview.") diff --git a/src/main_test.go b/src/main_test.go index 606a453..a43870e 100644 --- a/src/main_test.go +++ b/src/main_test.go @@ -59,6 +59,52 @@ func TestNamespaceServiceAccountAllowed(t *testing.T) { }`)) } +func TestNodeAccountAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:node:my-node", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestBootstrapAccountAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:bootstrap:my-bootstrap", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + func TestCrossProtectedNamespaceServiceAccountAccessAllowed(t *testing.T) { accessTest(t, defaultAuthorizer, false, []byte( @@ -122,9 +168,8 @@ func TestInvalidJSONDenied(t *testing.T) { } } -func TestPrivilegedUserAllowed(t *testing.T) { - authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"kubernetes-admin"}, false, 0) - accessTest(t, authorizer, false, +func TestRequiredUserAllowed(t *testing.T) { + accessTest(t, defaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -146,6 +191,30 @@ func TestPrivilegedUserAllowed(t *testing.T) { }`)) } +func TestAdditionalPrivilegedUserAllowed(t *testing.T) { + authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"special-user"}, false, 0) + accessTest(t, authorizer, false, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"special-user", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + func TestUnprivilegedUserDenied(t *testing.T) { accessTest(t, defaultAuthorizer, true, []byte( From b93c71364bb9f0045f1a849a14c92f97a7bac1c4 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 19 Jun 2025 09:10:11 +0100 Subject: [PATCH 28/30] Switched to multistage build --- Dockerfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index ab5a260..79cc204 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.24 +FROM golang:1.24 AS build-stage WORKDIR /app @@ -9,6 +9,12 @@ COPY src/*.go ./ RUN CGO_ENABLED=0 GOOS=linux go build -o /azimuth-authorization-webhook +FROM gcr.io/distroless/base-debian11 AS build-release-stage + +WORKDIR / + +COPY --from=build-stage /azimuth-authorization-webhook /azimuth-authorization-webhook + EXPOSE 8080 ENTRYPOINT ["/azimuth-authorization-webhook"] From c52cab4662bb419f76ffe88d6bc6b434caf2acc0 Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 19 Jun 2025 13:57:02 +0100 Subject: [PATCH 29/30] Updated tests --- src/{main_test.go => access_test.go} | 77 +++++++++------- src/input_test.go | 131 +++++++++++++++++++++++++++ src/main.go | 25 +++++ src/test_utils.go | 10 ++ 4 files changed, 208 insertions(+), 35 deletions(-) rename src/{main_test.go => access_test.go} (86%) create mode 100644 src/input_test.go create mode 100644 src/test_utils.go diff --git a/src/main_test.go b/src/access_test.go similarity index 86% rename from src/main_test.go rename to src/access_test.go index a43870e..d56def4 100644 --- a/src/main_test.go +++ b/src/access_test.go @@ -8,13 +8,8 @@ import ( "testing" ) -var defaultProtectedNamespaces = []string{"kube-system", "openstack-system"} -var defaultAdditionalPrivilegedUsers = []string{} - -var defaultAuthorizer func(w http.ResponseWriter, r *http.Request) = CreateWebhookAuthorizer(defaultProtectedNamespaces, defaultAdditionalPrivilegedUsers, false, 0) - func TestSystemUserAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -37,7 +32,7 @@ func TestSystemUserAllowed(t *testing.T) { } func TestNamespaceServiceAccountAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -60,7 +55,7 @@ func TestNamespaceServiceAccountAllowed(t *testing.T) { } func TestNodeAccountAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -83,7 +78,7 @@ func TestNodeAccountAllowed(t *testing.T) { } func TestBootstrapAccountAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -106,13 +101,13 @@ func TestBootstrapAccountAllowed(t *testing.T) { } func TestCrossProtectedNamespaceServiceAccountAccessAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{"kind":"SubjectAccessReview","apiVersion":"authorization.k8s.io/v1","metadata":{"creationTimestamp":null},"spec":{"resourceAttributes":{"namespace":"openstack-system","verb":"create","group":"apps","version":"v1","resource":"controllerrevisions"},"user":"system:serviceaccount:kube-system:daemon-set-controller","groups":["system:serviceaccounts","system:serviceaccounts:kube-system","system:authenticated"],"extra":{"authentication.kubernetes.io/credential-id":["JTI=3cf7d9de-5324-4df7-9447-47adb900f846"]},"uid":"cb35e0b5-1cfb-432f-acdf-5b5a0f924211"},"status":{"allowed":false}}`)) } func TestWrongNamespaceServiceAccountDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -135,7 +130,7 @@ func TestWrongNamespaceServiceAccountDenied(t *testing.T) { } func TestSystemAnonymousDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -157,19 +152,8 @@ func TestSystemAnonymousDenied(t *testing.T) { }`)) } -func TestInvalidJSONDenied(t *testing.T) { - data := bytes.NewBuffer([]byte("{ bad json }")) - req := httptest.NewRequest(http.MethodPost, "/authorize", data) - req.Header.Set("Content-Type", "application/json") - resp := httptest.NewRecorder() - defaultAuthorizer(resp, req) - if resp.Code != http.StatusBadRequest { - t.Error("Expected 400 error for invalid JSON") - } -} - func TestRequiredUserAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -192,7 +176,7 @@ func TestRequiredUserAllowed(t *testing.T) { } func TestAdditionalPrivilegedUserAllowed(t *testing.T) { - authorizer := CreateWebhookAuthorizer(defaultProtectedNamespaces, []string{"special-user"}, false, 0) + authorizer := CreateWebhookAuthorizer(DefaultProtectedNamespaces, []string{"special-user"}, false, 0) accessTest(t, authorizer, false, []byte( `{ @@ -216,7 +200,7 @@ func TestAdditionalPrivilegedUserAllowed(t *testing.T) { } func TestUnprivilegedUserDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -239,7 +223,7 @@ func TestUnprivilegedUserDenied(t *testing.T) { } func TestReadUnprotectedSecretsAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -262,7 +246,7 @@ func TestReadUnprotectedSecretsAllowed(t *testing.T) { } func TestReadProtectedNonSecretsAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -285,7 +269,7 @@ func TestReadProtectedNonSecretsAllowed(t *testing.T) { } func TestWriteProtectedNonSecretsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -308,7 +292,7 @@ func TestWriteProtectedNonSecretsDenied(t *testing.T) { } func TestWriteUnprotectedResourcesAllowed(t *testing.T) { - accessTest(t, defaultAuthorizer, false, + accessTest(t, DefaultAuthorizer, false, []byte( `{ "kind":"SubjectAccessReview", @@ -331,7 +315,7 @@ func TestWriteUnprotectedResourcesAllowed(t *testing.T) { } func TestEmptyNamespacesRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -354,7 +338,7 @@ func TestEmptyNamespacesRequestsDenied(t *testing.T) { } func TestProtectedReadAllRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -377,7 +361,7 @@ func TestProtectedReadAllRequestsDenied(t *testing.T) { } func TestProtectedWriteAllRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -400,7 +384,7 @@ func TestProtectedWriteAllRequestsDenied(t *testing.T) { } func TestProtectedAllVerbRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -423,7 +407,7 @@ func TestProtectedAllVerbRequestsDenied(t *testing.T) { } func TestProtectedAllVerbNonSecretRequestsDenied(t *testing.T) { - accessTest(t, defaultAuthorizer, true, + accessTest(t, DefaultAuthorizer, true, []byte( `{ "kind":"SubjectAccessReview", @@ -445,6 +429,29 @@ func TestProtectedAllVerbNonSecretRequestsDenied(t *testing.T) { }`)) } +func TestAllowedTrueInRequestDenied(t *testing.T) { + accessTest(t, DefaultAuthorizer, true, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"system:anonymous", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":true + } + }`)) +} + func accessTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Request), expectDenied bool, jsonData []byte) { data := bytes.NewBuffer(jsonData) req := httptest.NewRequest(http.MethodPost, "/authorize", data) diff --git a/src/input_test.go b/src/input_test.go new file mode 100644 index 0000000..12ac066 --- /dev/null +++ b/src/input_test.go @@ -0,0 +1,131 @@ +package main + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" +) + +func TestInvalidJSON(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte(`{bad json}`)) +} + +func TestInvalidResourceKind(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte( + `{ + "kind":"NotASubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"kubernetes-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestInvalidAPIVersion(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"v0", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":"kubernetes-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestEmptySpec(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{}, + "status":{ + "allowed":false + } + }`)) +} + +func TestBadAttributesFields(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":0, + "verb":0, + "version":0, + "resource":0, + "name":0 + }, + "user":"kubernetes-admin", + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func TestBadUser(t *testing.T) { + inputTest(t, DefaultAuthorizer, + []byte( + `{ + "kind":"SubjectAccessReview", + "apiVersion":"authorization.k8s.io/v1", + "spec":{ + "resourceAttributes":{ + "namespace":"kube-system", + "verb":"get", + "version":"v1", + "resource":"secrets", + "name":"important-creds" + }, + "user":0, + "groups":["system:authenticated"] + }, + "status":{ + "allowed":false + } + }`)) +} + +func inputTest(t *testing.T, authorizer func(w http.ResponseWriter, r *http.Request), jsonData []byte) { + data := bytes.NewBuffer(jsonData) + req := httptest.NewRequest(http.MethodPost, "/authorize", data) + req.Header.Set("Content-Type", "application/json") + resp := httptest.NewRecorder() + + authorizer(resp, req) + + if resp.Code != http.StatusBadRequest { + t.Error("Expected 400 error") + } +} diff --git a/src/main.go b/src/main.go index 84d4986..21af39b 100644 --- a/src/main.go +++ b/src/main.go @@ -96,6 +96,27 @@ func isRequestAuthorized(sar SubjectAccessReviewAPI, protectedNamespaces []strin return authorized, denyReason } +func inputIsSanitised(sar SubjectAccessReviewAPI, httpWriter http.ResponseWriter) bool { + inputError := false + var errString string + if sar.APIVersion != "authorization.k8s.io/v1" { + errString = sar.APIVersion + " not supported. Currently support apiVersions: 'authorization.k8s.io/v1'" + inputError = true + } + // Most other issues will have been caught as JSON decoding errors + if sar.Kind != "SubjectAccessReview" || sar.Spec.User == "" { + errString = "Malformed SubjectAccessReview" + inputError = true + } + if inputError { + log.Println(errString) + http.Error(httpWriter, errString, http.StatusBadRequest) + return false + } else { + return true + } +} + // Returns HTTP request handler to handle SubjectAccessReview API requests func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedUsers []string, opinionMode bool, logLevel int) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -117,6 +138,10 @@ func CreateWebhookAuthorizer(protectedNamespaces []string, additionalPrivilegedU defer r.Body.Close() + if !inputIsSanitised(sar, w) { + return + } + authorized, denyReason := isRequestAuthorized(sar, protectedNamespaces, additionalPrivilegedUsers) status := new(authorizationv1.SubjectAccessReviewStatus) diff --git a/src/test_utils.go b/src/test_utils.go new file mode 100644 index 0000000..c7eccf4 --- /dev/null +++ b/src/test_utils.go @@ -0,0 +1,10 @@ +package main + +import ( + "net/http" +) + +var DefaultProtectedNamespaces = []string{"kube-system", "openstack-system"} +var DefaultAdditionalPrivilegedUsers = []string{} + +var DefaultAuthorizer func(w http.ResponseWriter, r *http.Request) = CreateWebhookAuthorizer(DefaultProtectedNamespaces, DefaultAdditionalPrivilegedUsers, false, 0) From 972899389b8d3554280e90ee89f2096b8592c7eb Mon Sep 17 00:00:00 2001 From: wtripp180901 Date: Thu, 19 Jun 2025 14:04:03 +0100 Subject: [PATCH 30/30] bumped image --- chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/values.yaml b/chart/values.yaml index 4459b73..4676312 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1,5 +1,5 @@ port: 8080 -version: 13a5067 +version: 706400d logLevel: 1 protectedNamespaces: