defenseunicorns / uds-core

A FOSS secure runtime platform for mission-critical capabilities
https://uds.defenseunicorns.com
GNU Affero General Public License v3.0
50 stars 21 forks source link

There should be something akin to `excluded-paths` for authservice backed apps #645

Open corang opened 3 months ago

corang commented 3 months ago

Currently authservice apps are protected via 2 APs: send requester to authservice flow

  action: CUSTOM
  provider:
    name: authservice
  rules:
  - when:
    - key: request.headers[authorization]
      notValues:
      - '*'

guarantee token is present:

  action: ALLOW
  rules:
  - from:
    - source:
        requestPrincipals:
        - https://sso.uds.dev/realms/uds/*

Because of how AP matching works in istio there would have to be a somewhat major change to allow for excluded paths: send requester to authservice flow if path isn't x:

  action: CUSTOM
  provider:
    name: authservice
  rules:
  - to:
    - operation:
        notPaths:
        - /metrics/

guarantee token is present:

  action: ALLOW
  rules:
  - from:
    - source:
        requestPrincipals:
        - https://sso.uds.dev/realms/uds/*

unless path is x:

  action: ALLOW
  rules:
  - to:
    - operation:
        paths:
        - /metrics/

My question is how valuable from a security perspective is this?

  - when:
    - key: request.headers[authorization]
      notValues:
      - '*'

There may be a way to add request.headers[authorization] back to the CUSTOM ap but no matter what I tried to add it would always redirect to authservice if it was present.

Describe the solution you'd like

OR

Additional context

As in the acceptance criteria I'm not sure how valuable a user-settable path exclusion list is, for my use case the automation of allowing prometheus to hit the metrics endpoint is enough.

bburky commented 3 months ago

I would like to suggest NOT supporting excludedPaths: that completely skips authservice and JWT enforcement an specific paths. Instead, could we implement a feature specific to Prometheus metrics scraping? I think we should have a secure prometheus identity principal: we can use in AuthorizationPolicy ALLOW rules.

We could even automatically generate exclusions for any Package CR generated metrics rules, and let the package author add additional excludeMetricsPaths: for anything used in a ServiceMonitor/etc.

...however, the boolean logic in CUSTOM AuthorizationPolicies is terrible. And combining notPaths with the existing request.headers[authorization] rule can be quite hard.

bburky commented 3 months ago

An alternative idea is to revisit authservice-at-the-gateway. For full zero trust, we should still enforce authservice JWT checking at the workload sidecar too, but we can move the authservice redirect and CUSTOM authorization policy to the gateway only.

What this allows is that any in-cluster network request that doesn't go through gateway, implicitly bypasses the authservice redirect (which usually isn't desirable anyway in-cluster). However we can keep the JWT enforcement at the sidecar.

For specific paths like metrics, all that would be required would be an AuthorizationPolicy ALLOW rule on the specific path (preferably restricted to the prometheus principal:). Because AuthorizationPolicies are merged, this could be a separate resource if that's easier.

I'd need to do a little bit more thought for this, but I kind of like it. I kind of do want to turn authservice on by default more often, and doing it at the gateway probably helps and minimizes double-authservice.

bburky commented 3 months ago

My question is how valuable from a security perspective is this?

  - when:
    - key: request.headers[authorization]
      notValues:
      - '*'

The request.headers[authorization] rule ends up saying "allow requests with an Authorization header to skip the authservice redirect" and the ALLOW rule says "require a valid JWT from the correct issuer, etc"

One reason to do request.headers[authorization] is to support mission apps that implement an API authenticated by a JWT. I don't think we really support this officially, but it is kind of a reasonable thing to do. If we actually want to support this, we should make getting JWTs out of Keycloak to support this use case easier.

The more likely short term use case for request.headers[authorization] is to allow authservice-protecting both the frontend and backend of a mission app. But if the backend is internal-only reachable only in-cluster, then the authservice redirect would be an issue, right? Well if if you tell the mission app devs they must forward the JWT they receive on the frontend to the backend, it works with this header.
There's at least some environments that are deployed this way. I think @ntwkninja said their environment had this requirement.

Requiring authservice for backend Pods is also nice to protect backend apps from in-cluster SSRF: if you somehow trick a Pod into making a request for you in-cluster, it will fail because it was unauthenticated.

For previous discussion see: https://github.com/defenseunicorns/uds-core/pull/201/files#r1643449596 https://github.com/defenseunicorns/uds-core/pull/201/files#r1643450893

corang commented 3 months ago

Example of implementation that doesn't lose any features but allows metrics to speak as long as they're on a different port: CUSTOM policy that controls authservice redirection:

  rules:
  - when:
    - key: request.headers[authorization]
      notValues:
      - '*'
    - key: destination.port
      notValues:
      - "8081"

New ALLOW policy that scopes port access to prometheus:

  action: ALLOW
  rules:
  - to:
    - operation:
        ports:
        - "8081"
  - when:
    - key: source.principal
      values:
      - cluster.local/ns/monitoring/sa/kube-prometheus-stack-prometheus

This can all be automated through pepr with existing CRD fields and be cloned for multiple monitoring endpoints if needed. Port would be referenced from targetPort in the monitor field

corang commented 3 months ago

@bburky Thoughts on above?

bburky commented 3 months ago

Yes, this is what I was discussing in https://github.com/defenseunicorns/uds-core/issues/645#issuecomment-2271676425. If we take this approach i'd want to make it built into the UDS operator somehow to write these AuthorizationPolicies for you.

Thanks for the example using a port rule instead of a path. That's actually probably more secure. It's also different enough that a feature that only supports paths (like a hypothetical excludeMetricsPaths:) is too narrow, we should support port too at least.

Can you confirm that CUSTOM rule actually works? I forget how the terrible double-negative that is CUSTOM rules work. Can you test that requests to the main app are still authservice protected, but requests with an Authorization header bypass authservice (just test with anything, you should get an RBAC denied error), and the 8081 port also bypasses authservice?

I think this is an equivalent ALLOW rule that is a little clearer:

  - from:
       principals:
       - "cluster.local/ns/monitoring/sa/kube-prometheus-stack-prometheus"
    to:
    - operation:
        ports:
        - "8081"