kserve enforce authentication#3180
Conversation
2416e85 to
a2b976e
Compare
| @@ -0,0 +1,66 @@ | |||
| # KServe External Access Configuration | |||
There was a problem hiding this comment.
lets move this to the other kserve test files. Do not create a new /examples folder.
| # Apply cluster-local-gateway with JWT authentication (with retry) | ||
| echo "Applying cluster-local-gateway JWT authentication policies..." | ||
| for ((i=1; i<=3; i++)); do | ||
| if kustomize build common/istio/cluster-local-gateway/overlays/m2m-auth | kubectl apply -f -; then | ||
| echo "cluster-local-gateway JWT auth applied successfully" | ||
| break | ||
| else | ||
| echo "Attempt $i failed to apply cluster-local-gateway JWT auth, retrying..." | ||
| sleep 5 | ||
| fi | ||
| done | ||
|
|
There was a problem hiding this comment.
why are you adding this ?
kustomize build common/istio/cluster-local-gateway/overlays/m2m-auth | kubectl apply -f - should be enough instead of 12 lines
| @@ -0,0 +1,209 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
tests/knative_authENTICATION_test.sh
| # Wait for InferenceService to be ready | ||
| log_info "Waiting for InferenceService to be ready..." | ||
| kubectl wait --for=condition=Ready inferenceservice/secure-sklearn -n $PRIMARY_NAMESPACE --timeout=300s || { | ||
| log_info "InferenceService not ready, continuing with tests..." |
There was a problem hiding this comment.
Lets remove most of the logging stuff. We just want to fail directly if something is wrong.
|
|
||
| # Wait for InferenceService to be ready | ||
| log_info "Waiting for InferenceService to be ready..." | ||
| kubectl wait --for=condition=Ready inferenceservice/secure-sklearn -n $PRIMARY_NAMESPACE --timeout=300s || { |
There was a problem hiding this comment.
300s is a bit much, lets use 180s
| @@ -0,0 +1,257 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Please fix the filename here as well
| @@ -0,0 +1,148 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
what is the difference to the other tests? I think also the filename is not clear enough to explain what you are doing.
| @@ -0,0 +1,58 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
tests/kserve_setup_external_access.sh
|
Please also fix the tests. |
|
Does this also secure from this issue mentioned in KServe docs?
My understanding is that this is actually not feasible to tackle on kubeflow's side, so users will still be able to access ISVCs in other users' namespaces, when activator intercepts them. |
I agree, the core problem: When Knative's activator is in the request path (during cold starts or scale-to-zero), it bypasses namespace-based JWT authentication because the activator has broad access permissions and the original request identity is lost. From the issue: knative-extensions/net-istio#554 "services in different namespace will be able to communicate with each other whenever activator is in path, because activator is allowed to access any service" I also thinnk this is not fixable at the manifests level - it's an architectural limitation that requires changes to Knative/KServe itself. |
So namespace level wont be possible, but we can try enforcing JWT, atleast that is doable, but im facing errors hard to find whats causing the tests failing. |
|
IMO It 'd be good to mention this architectural limitation in the new setup/overlay provided, given that people using it should be aware of this quite important limitation. |
Like in the manifest Readme for Istio, we should update that? |
|
@juliusvonkohout can you please re run these 2 tests? |
You should be able to do so with /retest if not we have to add you as member to Kubeflow. Do you mind creating a PR to add the 4 GSOC students as member to the kubeflow organization @madmecodes ? |
Yep exactly, preferably with sth like a warning box if it's aligned with the style of readmes in this repo |
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
3298b84 to
fb53c49
Compare
|
Thank you, that significantly improves the status quo. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: juliusvonkohout The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✏️ Summary of Changes
KServe JWT Authentication PR Analysis
Core Security Features
JWT-based authentication for
cluster-local-gateway: AddedRequestAuthenticationandAuthorizationPolicyto secure the gateway that KServe uses by default.Two authentication overlays:
m2m-auth: Basic JWT authentication requiring valid Kubernetes service account tokens.m2m-auth-strict: Enhanced version with namespace-level isolation.Comprehensive test suite: Multiple test scripts to validate authentication scenarios.
Documentation: Added
KSERVE_JWT_AUTHENTICATION.mdwith a detailed implementation guide.Technical Implementation Details
RequestAuthentication: Validates JWT tokens from Kubernetes API server with proper issuer configuration.AuthorizationPolicy (DENY): Blocks all requests without valid JWT principals, except health checks.AuthorizationPolicy (ALLOW): Permits requests with valid JWT principals and exempts health check endpoints.istio-ingressgateway.2. How It Addresses the Core Security Issue
The implementation directly addresses issue #2811 by:
cluster-local-gatewayhad no authentication whileistio-ingressgatewayhadoauth2-proxy.oauth2-proxyfor ingress).knative-cniinstall script.The PR represents a significant security improvement but leaves room for further enhancements based on community feedback and production usage patterns.
📦 Dependencies
None
🐛 Related Issues
#2811
✅ Contributor Checklist