Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Rewrite imageAllowRules logic - Secure by Default #1549

Closed
3 tasks done
iwilltry42 opened this issue Apr 26, 2023 · 11 comments
Closed
3 tasks done

Rewrite imageAllowRules logic - Secure by Default #1549

iwilltry42 opened this issue Apr 26, 2023 · 11 comments
Assignees
Labels
kind/enhancement New feature or request
Milestone

Comments

@iwilltry42
Copy link
Contributor

iwilltry42 commented Apr 26, 2023

As discussed in yesterday's standup, this will be the new way to go with imageAllowRules:

  • Cluster's are secure by default, meaning that we have an implicit deny-all-images policy
  • When running an image for the first time and it's denied, ask if it should be added to an imageAllowRule
    • This will automatically create an initial IAR that replaces the implicit deny-all-images policy
  • The IAR CR should get a new field including a pattern (glob / prefix / auto-upgrade-pattern) to define the images covered by that rule
    • context matching
      • prefix matching: any string without ? or * in it will be treated as prefix match pattern
      • glob matching: shell filename matching with only * and ? supported
    • tag matching:
      • auto-upgrade pattern
  • IARs are there to allow images, not to deny them, meaning that if an image is not covered/targeted by any IAR in the project, then it will be denied -> "Opening doors, instead of closing them"

Extra Notes / Quotes:

I want to ensure that the way it’s implemented is similar to permissions in that you only evaluate and change rules if the server rejects it. Don’t validate before. Probably add the logic to the same place where we prompt for permissions

Concept Drawing

IAR v2 drawio

Design

Example IAR with only the image scope section (no signatures required), would allow all images just matching that pattern:

apiVersion: api.acorn.io/v1
kind: ImageAllowRule
metadata:
  name: iar-scope-only
  namespace: acorn
images:
  - ghcr.io/acorn-io/**

Extra Questions

  1. Do we want to manage images as well? I.e. I delete the only IAR allowing a specific image that exists in the project -> Do we delete that image? Or do we provide a "prune" command?
    -> A: Not for now

Answers

  • Allow Prompt Options: Exact (digest), Registry, Repo, Everything
@iwilltry42
Copy link
Contributor Author

@ibuildthecloud & @cjellick can I have your eyes on this, please?

@cjellick
Copy link
Member

I wonder if we want acorn install --default-image-allow-rule-enforcement secure|permissive where secure is the default behavior we have above and permissive means that any time a project is created, acorn writes an "allow everything" rule into the project.

@iwilltry42
Copy link
Contributor Author

That "allow everything" rule would need to be removed by a project admin before making use of any other IAR, right?
So I assume acorn project create would also need another flag to override the installation-wide flag?

@cjellick
Copy link
Member

Just a project admin. Like, the clsuter admin, when installing acorn, would set that flag so that every project got a permissive policy. If a particular project did not want the permissive policy, they would just delete it.

of course, this gets into the complexity of creating it in a "handler" and how to prevent the handler from re-creating it periodically.

...I've talked myself out of it. Don't add the install flag 😅

@ibuildthecloud
Copy link
Member

ibuildthecloud commented Apr 28, 2023

  1. The image field should be just like the image field in acorn run. Support the same pattern matching as * and # there. A string without * or # is an exact match. It would be good if you put the actual struct or yaml you plan to implement in the issue. I'm not expecting more than a simple than a field like image or images that is a string or string slice. Maybe you have a better idea.
  2. There's no such thing as a managed IAR. When you need to just add a whole new IAR. Put a generated name on the resource, doesn't really matter. There's no advantage I can think of of trying to add to an existing one.

@iwilltry42
Copy link
Contributor Author

iwilltry42 commented May 2, 2023

@ibuildthecloud

  1. The auto-upgrade pattern is specifically targeting tags and is not exactly suitable for the context (i.e. registry/repo) part of the image name. # is not really useful there and the regex part of ** does not contain : and / which are required for the context. I extended this to work with it now, but it still feels like just bending this unnecessarily.
    We also don't need to do any sorting here, but just neglecting the named matching groups would be fine.
    I'd propose we split that up into a contextPattern and a tagPattern:
    • contextPattern: Unix-style globbing with *, ** , ?and [Aa], which is pretty suitable for this path-style context part
    • tagPattern: auto-upgrade pattern
  2. Alright, I just thought that a single one would be easier to manage for an operator/admin to modify the auto-generated rules.

EDIT: I figured that (1.) is exactly what I proposed in the issue description 😅

@iwilltry42
Copy link
Contributor Author

Answer to (1.) following yesterday's meeting: We just add : and / to the double-asterisk pattern of the auto-upgrade logic and re-use it as a generic image-pattern.

@iwilltry42
Copy link
Contributor Author

@ibuildthecloud copying this over from Slack:

I was typing a long write-up here about how to create the IARs from the Client-Side when I realized that for the Acornfile implementation of IAR we will anyway need all the Client functions to handle them, right?
Am I correctly assuming that IARs, even if defined in the same Acornfile will be treated as a separate (but linked) entity to the actual app?
I.e. when running such an image, we will subsequently call client.IARCreate and then client.AppRun instead of somehow embedding it into the App spec?

iwilltry42 added a commit that referenced this issue May 31, 2023
#1549) (#1571)

- New flag `--features`, e.g. `--features image-allow-rules=true`
- If ImageAllowRules are enabled, projects are secured by default, so that you have to create an IAR to allow an image
- If an image is disallowed, we now prompt to allow it
- The IAR CR now has an `images` field that uses the Auto-Upgrade Pattern logic to scope an IAR to a specific set of images matching that pattern
@iwilltry42
Copy link
Contributor Author

iwilltry42 commented May 31, 2023

  • New install flag --features, e.g. --features image-allow-rules=true
  • If ImageAllowRules are enabled, projects are secured by default, so that you have to create an IAR to allow an image
  • If an image is disallowed, we now prompt to allow it
  • The IAR CR now has an images field that uses the Auto-Upgrade Pattern logic to scope an IAR to a specific set of images matching that pattern

@sangee2004
Copy link
Contributor

#1678 is validated and closed now.
Other issues found when testing this feature will be tracked as separate issues.

Closing this issue .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants