Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allowed seccomp specific config media type #2306

Closed

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Feb 21, 2024

Adding the new seccomp related config media type application/vnd.cncf.seccomp-profile.config.v1+json to the list of allowed configurations.

Alternative to #2279
Testing in cri-o/cri-o#7804

cc @sftim @mtrmac

@sftim
Copy link

sftim commented Feb 21, 2024

application/vnd.cncf.seccomp-profile.config.v1+json

Any plan to register this?

@saschagrunert
Copy link
Member Author

application/vnd.cncf.seccomp-profile.config.v1+json

Any plan to register this?

In the image spec? Hm, I'm not sure if that would be accepted. 🤔

@saschagrunert
Copy link
Member Author

@containers/image-maintainers PTAL

@mtrmac
Copy link
Collaborator

mtrmac commented Feb 22, 2024

As discussed before in #2279

  • I think the larger question of “where / how we store artifacts” is still undecided. The primary design question is “do we use a single namespace in c/storage for artifacts and images, such that we can’t have both an artifact and an image with the same name?”. (And then there’s the implementation effort to somehow store contents of artifacts which are not image-like tar.) I think the first impression is that the answer to this must be “no” (or at least “not always”), for artifacts that store SBOMs and other image-related data which is directly linked to a specific image and should, e.g., be deleted alongside the image.)
  • I don’t think this makes semantic sense for this particular artifact. The JSON artifact is not an image, and does not “really” have image config values that make sense to return from .OCIConfig. If we really wanted to write the seccomp profiles things to c/storage, it would be more reasonable for the c/image/copy code that calls .OCIConfig to avoid doing that for the seccomp profile artifacts. (… But after activity around storage, dest: clarify when TOCDigest is used #2218 , the c/storage “destination” requires! that the image has a ~valid config in the image sense.) What we do here really depends on the local storage design above, but it’s already a point of concern.
  • I’d also say that the approach of Add support for OCI artifact seccomp profiles cri-o/cri-o#7719 , where we need to distribute a single file and end up creating and mounting a possibly-many-layer overlay filesystem, and walk that filesystem to find some file, feels like a prototype that just shouldn’t end up being the production design. There’s so much unnecessary complexity and overhead. I appreciate that this can be done right now, and waiting for a “local artifact storage” design is unattractive, but I don’t like the longer-term cost of forging ahead now.

@mtrmac
Copy link
Collaborator

mtrmac commented Feb 22, 2024

Cc: @nalind

@saschagrunert
Copy link
Member Author

Okay, I'm going to elaborate on the file based pull approach rather than full c/common libimage. I guess this would also simplify the whole media type topic by accepting anything.

@saschagrunert saschagrunert deleted the seccomp-media-type branch February 22, 2024 18:31
@mtrmac
Copy link
Collaborator

mtrmac commented Feb 22, 2024

I do think we need to arrive at a design for “where / how we store artifacts”. I don’t know what’s the best forum and process for that.

@saschagrunert
Copy link
Member Author

saschagrunert commented Feb 22, 2024

@mtrmac generally agree, for seccomp itself an in memory solution would be enough because the profile does not need to be on disk. We can create a more sustainable design later, when we want to fulfill more use cases like wasm.

What do we need to do?

  1. Parse the user given reference
  2. Create an image source, then retrieve the blobs/layers
  3. Pull the blob directly (which can be the seccomp JSON for example) and parse it

I’m not exactly sure how to do that right now with c/image but I’ll find out.

@mtrmac
Copy link
Collaborator

mtrmac commented Feb 22, 2024

3. Pull the blob directly (which can be the seccomp JSON for example) and parse it

⚠️💀 Critical: verify each blob’s digest against the data in the manifest.
Recommended: If reading into memory (and possibly even to disk), impose a hard-coded size limit.

The digest verification alone suggests that c/image should probably provide a convenience helper doing that. (Right now c/image does get care of that, but only in c/image/copy. Individual transports should not be in that business.)

@mtrmac
Copy link
Collaborator

mtrmac commented Feb 22, 2024

As for how to do that, the very rough outline is

src := something.NewReference().NewImageSource()
// m, mt := src.GetManifest()
// parse m, either using the generic code, or a specific format, in c/image/manifest
stream := src.GetBlob(digest)
// buffer AND digest stream
// if digest matches, consume

Alternatively, IIRC podman machine does the equivalent of skopeo copy $src oci:$tmp via the Go API; that still costs strictly-speaking-unnecessary disk accesses, but not nearly as many as c/storage (no locking, no syncs), and it handles digest validation.

saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 26, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 26, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
@saschagrunert
Copy link
Member Author

See cri-o/cri-o#7814

saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 26, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 26, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert added a commit to saschagrunert/cri-o that referenced this pull request Feb 27, 2024
CRI-O now pulls OCI artifacts directly in-memory after the discussions
in containers/image#2306 and
kubernetes/website#45121.

CRI-O also enforces the config media type
`application/vnd.cncf.seccomp-profile.config.v1+json` for seccomp
profiles.

Signed-off-by: Sascha Grunert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants