-
Notifications
You must be signed in to change notification settings - Fork 783
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
Off-line verification of signatures against policy.json #2435
Comments
Yeah but many cases that start off as "fork off a binary" eventually grow more complex needs that turn into wanting an API.
We can't in practice break it today because doing so would break rpm-ostree and bootc, so...I dunno, maybe we should rename it to drop the experimental flag.
Yeah but I think that's up to us (i.e. including you). Another alternative is standardizing on varlink for this...what we have is somewhat close to that already. Would that help? |
(Just for the record; ACK, let’s figure this out.)
If we did do that, I’m not sure about hard-coding
The difference in implementation effort between a new subcommand and a new option is fairly trivial — if we are adding a new kind of functionality, a new subcommand seems clearer to me.
I think a lot of that is that’s just C… and, partially, this library not being optimized for one-off use cases (but for GObject, which is so inconvenient that it is not used in that example). Hypothetically, I think a varargs-style json_root_parse_fields(root, &error,
"success", G_TYPE_BOOLEAN, &response->success,
"pipeid", G_TYPE_INT64, &response->pipeID …
NULL,
)
root = json_root_from_fields(&error,
"method", G_TYPE_STRING, method",
"args", …
NULL
) would not be too bad (except, of course, that it doesn’t exist in the library right now) — and, most importantly, whatever structured format we use, it’s not going to get much easier. Compare (Also, uh, the method calls can be constructed using
I do think that having a structure beyond just strings is valuable. If nothing else, ~every such automated use case eventually asks for differentiating some error causes; that’s hard to promise as a stable API in human-oriented text output, so we are left with either the 7-bit exit code (and an inability to classify an error along two dimensions), or, again, something like JSON.
Pragmatically it ~has to be kept stable by now, and I don’t anticipate breaking changes.
You’re right that users who already have an enforcing policy in the primary consumer are not at much risk. But they aren’t the ones most frequently asking for such a command. The kinds of things users ask for are
or
(And then was is the very legitimate mirroring-tool docker://vendor.example/product docker://mirror.internal/vendor/product
# Sanity-check that the images really are signed correctly, and that they are readable
for image in $(images_for_that_product); do
skopeo [config to set up mirrors and policy like in production] verify-policy $image || exit
done
trigger-deployment-with-policy-enforcement [config to set up mirrors] docker://vendor.example/product which really makes sense to want.) If nothing else, the error handling area requires structured responses. And I think that, when |
I'd be happier about that - Flatpak already does extensive JSON handling and carrying an implementation of varlink using json-glib is more palatable than bringing in another IDL / type-system, etc. It doesn't seem a lot more complicated than the current image-proxy protocol. |
This came up when we were discussing a RHEL AI use case for instructlab, where it would actually make sense to have the image for that work by default in a similar way - pulled to the root storage, but executed unprivileged. This whole problem domain isn't just "download as user", it's also "user can hold a runtime ref to avoid image GC" for example. I think it would very much make sense to teach the c/image-alike stack to support things like this. And that whole topic then goes into a much deeper thing around sharing more around container image storage. Anyways, such changes to flatpak would be huge, so let's just dial in on the image fetching part for now, but keeping in mind larger structural changes.
Do you see an adaption of the existing IPC to be varlink to be more "standard" as aligning with that or from your PoV would it be closer to "another one"? I think it's likely we could deprecate the non-varlink |
Yes, the idea of hardcoding
Some of it's just C, some of it is me being paranoid and wanting to "fail open".
Generally agree - calling remote API's from C/C++ is never beautiful unless you have an IDL compiler. My issue wasn't really that it was particularly hard to use this API, it's just that there's a lot of overhead (from C especially) to use a "machine interface" to Skopeo when you just want a single "yes" / "no"
Thinking about this some, I'm not sure there's a meaningful error to display in a user interface (like GNOME Software) other than "Could not install application because it failed verification against system signature policy, see system logs for details". We can't really report up the information about the error in structured form, because meaningful failure reasons are really closely tied to the structure of policy.json - a meaningful error message that goes beyond the above looks like:
Which only containers/image has the knowledge to create. (Currently you get something like:
which is probably good enough if logged, but definitely isn't user consumable.) So, my summary here is:
Perhaps the main question should be to what extent
because while Flatpak can deal with a machine interface, you can't do that if you are writing a shell script or doing something on the CLI. |
I think that’s fine — we need to be able to evolve things, I’d just like to avoid having two entirely separate “proxy-for-bootc” and “proxy-for-flatpak” subsystems.
I’ll fully defer to you in that, I am not aware of any other user. Specifically on varlink, Podman used to be varlink-based and that was abandoned. Reported concerns:
Personally … I’d like to run an IDL compiler and not think about it much at all, and I have no opinion on the specific protocol / implementation. And a IDL is not really a blocker for adding one more |
Maybe:
I can see an argument that all of these could, on systems with few user-exposed options and good automated retry/recovery logic, be classified as “something went wrong, this is never supposed to happen, get an expert help”. Also, c/image does not currently expose all of these things as API that could be detected, anyway. |
That one isn't relevant here as we're always operating on a local I thought about this a bit more, and I lean towards the CLI verb in the short term (no strong opinion here on adding a new "productizing" the proxy code more (varlink, dropping BTW I do want to highlight the overlap between the proxy API and things like #658 ...which could be done by adding e.g. a high level |
A friendly reminder that this issue had no activity for 30 days. |
I'd like to have a way to provide a manifest (or manifest digest) and a set of previously downloaded signatures and have skopeo check that the manifest is correctly signed according to
policy.json
. The identity to check againstpolicy.json
would be provided separately, and correspond to the original source of the manifest.The use case I need this for is checking downloaded content before installation with a tool not based on
containers/image
. In particular I'd like to be able to check signatures of Flatpaks stored as OCI images in an OCI registry.It doesn't just work to do the download using
skopeo copy
orskopeo experimental-image-proxy
because, among other things:policy.json
) is done as root.containers/image
(Support layer deltas image#902, though there would be work beyond that to make it useful for Flatpak.)As a new subcommand
Proposal:
Where
is (possibly incomplete) directory indir:
format withmanifest.json
andsignature-<n>
files.There's a proof of concept implementation of something similar as a separate binary https://gist.github.com/owtaylor/8658c6c22714ee4ec3d0353480062c9d
skopeo standalone-verify
There is of course, an existing
standalone-verify
subcommand. It, however, is not usable for this, because it only handles GPG signatures, and not sigstore signatures, and doesn't respect policy.json. It might be possible to compatibly extend it to this use case, maybe, along with the existing:We could have:
Using
experimental-image-proxy
@mtrmac suggested that
skopeo experimental-image-proxy
almost does the right thing already - calling theOpenImage
JSON method already checks againstpolicy.json
. The only missing thing is being able to separate the identity being checked - which could be done as another argument toOpenImage
or as a separate extended method.An example of how calling
OpenImage
from GLib/Flatpak style C looks like can be found at: https://gist.github.com/owtaylor/06b9bbec53967677d4833ffa54d30598From my perspective, problems with this are:
experimental-image-proxy
might involve depending on external protocol libraries (Cap'n Proto was proposed in proxy: Add optional flags toOpenImage
#1844)Is this liable to be misused (TOCTOU)
A concern that @mtrmac raised is that a "check signatures" API can be misused because someone might check that the signatures on
docker://registry.example.com/some/image:latest
are valid, then download and install it (via noncontainers/image
means) without realizing thatlatest
has been changed to some other image. I don't think either of the proposed commands above are suitable for misusing that way. You could, in theory, do:But the
skopeo copy
command already does thepolicy.json
check, so that you could just leave out theskopeo verify-installable
entirely. And for that matter, just install whatskopeo copy
downloaded and avoid the TOCTOU problem.experimental-image-proxy/OpenImage
could already be misused in this fashion, but it's so much work to use it hopefully anybody doing that would think it through carefully.The text was updated successfully, but these errors were encountered: