Add documentation for the signature verification policy files #50

mtrmac · 2016-08-10T22:27:05Z

No description provided.

mtrmac · 2016-08-10T23:06:59Z

(5) Container image signing and verification with a single key

philips · 2016-08-10T23:43:43Z

docs/policy.json.md

+Signature verification policy files are used to specify policy, e.g. trusted keys,
+applicable when deciding whether to accept an image, or individual signatures of that image, as valid.
+
+The default policy is stored (unless overridden at compile-time) at `/etc/atomic/policy.json`;


Seems odd to have a default file location for a library.

It does seem odd to have a default file location in a library. I’m thinking that having each user of the library decide on its own, very likely different, location, would be much worse.

Of course a distributor shipping several such applications can patch them to use the same location—but that’s extra work and will still lead to fragmentation if various distributors don’t patch the same applications to use the same paths.

Also, the documentation of the format should be in this repo because that’s where the implementation is maintained, and then it’s a bit awkward to have a documentation for a file format which can’t tell the users where to use the format, or even how to find that out.

If an application wants to always use a different location, all it needs to do is call signature.NewPolicyFromFile instead of signature.DefaultPolicy, or at compile-time set -ldflags '-X github.com/containers/image/signature.systemDefaultPolicyPath=/some/path'. Or there can be a local-ish override via types.SystemContext (all as of #49). There really are quite a few ways not to use the default, but I think it is very valuable to have a default.

I do see the point that this is policy which might not be appropriate for this repo, and I’d be happy to change the PRs if that is the consensus.

If the specific value of the path is the reason for not liking it in a library, I personally really care very little what the path is. A parallel discussion of that might happen in containers/skopeo#157 .

On Wed, Aug 10, 2016 at 04:53:27PM -0700, Miloslav Trmač wrote:

+The default policy is stored (unless overridden at compile-time) at /etc/atomic/policy.json;

It does seem odd to have a default file location in a library. I’m
thinking that having each user of the library decide on its own,
very likely different, location, would be much worse.

Chipping in from the peanut gallery, I think the best solution to this
sort of thing is to use a set of standard rules for looking up config
files. For example 1.

is to use a set of standard rules for looking up config files.

The basedir spec is essentially the same hard-coded path as above, + extra environment variable overrides; but it still requires the hard-coded path to be defined (and in typical use of the basedir spec, the environment variables are unset and the hard-coded path applies).

I would love to use a platform-defined API for configuration storage (~ Windows registry or GConf2-like structured storage with documented schema), but I am not really aware of any in sufficiently wide use on Linux, especially outside the desktop space.

Still, even deferring to a third-party standard or configuration management API does not automatically resolve the question whether the signature verification policy “belongs” to the application or to the library — and my primary concern here is for all applications to use the same policy, i.e. for this repo to hard-code the location of the configuration within the configuration management API, whatever API is in use (where our API happens to be ioutil.ReadFile("/etc/$something")).

I just don't think any library should read any file. We can encourage a default like /etc/containers/fetch-policy.d or something through a const package though. I am also very very hesitant to trust a single file configuration at this point. It places too much burden on tooling to do the right thing. I would rather have systemd style .d directories where you can drop N policies in place.

I like the idea of users being able to define policy in their home dirs. We are working on atomic pull --user as we speak for running containers fully without privileges. But I worry about system policy being overridden by user policy.

On Thu, Aug 11, 2016 at 05:15:44AM -0700, Daniel J Walsh wrote:

I like the idea of users being able to define policy in their home
dirs. We are working on atomic pull --user as we speak for running
containers fully without privileges. But I worry about system
policy being overridden by user policy.

The only situation I can think of where you'd be concerned about user
overrides is:

The library is compiled into a binary that is setuid root or
otherwise enhances the caller's privileges, and

The user-configurable settings would allow the user to abuse those
elevated privileges.

Without a setuid binary, the user could accomplish the same override
by recompiling the program/library with their alternative
configuration. That's more work than dropping some files in
$XDG_CONFIG_HOME/containers/signature-verification.d/, but “it's a bit
more work for the user to override these” shouldn't be part of your
sysadmin policy ;).

Yes I figure it is easy to ignore see "rpm --nogpgcheck"

I just don't think any library should read any file.

Literally no io.Open calls? That seems both horrifying and gloriously great at the same time to me—but, I think, hardly tenable without a dependency injection framework.

Or do you just mean “should not hardcode any path”? If so, then…

We can encourage a default like /etc/containers/fetch-policy.d or something through a const package though.

… AFAICS there the following is exactly equivalent in its semantics and capabiliities:

policy, err = signature.DefaultPolicy(nil) // and either of policy, err = signature.NewPolicyFromFile(customPath) policy, err = signature.DefaultPolicy(&types.SystemContext{SignaturePolicyPath:customPath})

(what #49 does)

policy, err = signature.NewPolicyFromFile(consts.DefaultPolicyPath) // and policy, err = signature.NewPolicyFromFile(customPath)

(using a const package)

The only difference is that signature.DefaultPolicy makes the default case simpler, and consts.DefaultPolicyPath makes the non-default case simpler.

I am also very very hesitant to trust a single file configuration at this point. It places too much burden on tooling to do the right thing. I would rather have systemd style .d directories where you can drop N policies in place.

That’s a very reasonable feature request; can we merge documentation for what the code currently does first, and not block that on implementing new features, please?

I like the idea of users being able to define policy in their home dirs. We are working on atomic pull --user as we speak for running containers fully without privileges. But I worry about system policy being overridden by user policy.

As of containers/skopeo#157 users can do that explicitly via skopeo --policy $any_path.

As @wking says, this should mostly not give users any new privileges, though there are some a possibly worrisome cases to consider (sudo skopeo copy silently using the unprivileged $HOME might defeat attempts to audit changes to the system-wide policy file).

Again, please let’s not block adding this documentation on new feature requests. The documented semantics of signature.DefaultPolicy in #49 does gives us the leeway to implement this later, and for all users of the library to benefit transparently.

runcom · 2016-08-25T10:05:14Z

looks solid, @philips @rhatdan wanna make another pass at this?

mtrmac · 2016-08-25T16:25:18Z

Updated for #49 moving the default policy path to /etc/containers/policy.json.

mtrmac · 2016-09-07T13:07:59Z

How can we move this forward?

mtrmac · 2016-09-14T17:29:45Z

Note to self: Need to somehow improve the examples to that they don’t use a comment syntax invalid in JSON.

runcom · 2016-09-21T13:47:54Z

@mtrmac this fully LGTM - as a bonus I would add just another example based on the situation in #99 but just skip this comment if you don't think that add will be needed

runcom · 2016-09-21T13:48:05Z

lgtm

runcom · 2016-09-21T13:48:22Z

@rhatdan PTAL at this

mtrmac · 2016-09-21T14:34:40Z

as a bonus I would add just another example based on the situation in #99 but just skip this comment if you don't think that add will be needed

I’d rather add that together with the #99 solution.

runcom · 2016-09-21T14:36:14Z

I’d rather add that together with the #99 solution.

sure we'll do that, I was hoping to have an example which is just an admin workaround in #99 (a workaround for that issue ofc)

aweiteka

This is important documentation for users of the library. Any other issues?

aweiteka · 2016-10-05T14:48:49Z

docs/policy.json.md

@@ -0,0 +1,251 @@
+<!-- NOTE: Keep this in sync with signature/policy_types.go! -->


We want to support md2man format. HTML comment syntax breaks conversion. Need to add man headers per #121. For example:

% POLICY.JSON(5) policy.json Man Page % Miloslav Trmac % September 2016

mtrmac · 2016-10-05T19:39:34Z

Updated:

Added a go-md2man header
Removed the initial “keep in sync” comment (a few other comments left)
Added an “A temporary work-around to allow accessing any image by digest” section

The result of processing this with go-md2man is definitely not perfect, especially WRT processing of lists, but it is mostly readable and definitely better than nothing.

runcom · 2016-10-11T15:32:24Z

docs/policy.json.md

+    "transports": {
+        "docker": {
+            /* Allow installing images from a specific repository namespace, without cryptographic verification */
+            "docker.io/openshift": [{"type": "insecureAcceptAnything"}]


This should be maybe docker.io/library/openshift as per https://github.com/containers/image/pull/50/files#diff-47e5cc71f8440761aa6de049b5f54e8cR87

I just tried with:

"docker.io/fedora": [{"type": "insecureAcceptAnything"}]

and it fails to verify the image unless I use:

"docker.io/library/fedora": [{"type": "insecureAcceptAnything"}]

No, this would apply to images like openshift/hello-openshift and openshift/origin.

(There is no docker.io/library/openshift.)

The comment does say “repository namespace” but perhaps it needs to be clarified further?

I get that but for images on Docker Hub that are under the hidden library ns, you need to specify library in that as shown in my example above. So better change this example with an image like library/fedora.

Sorry don't change this example just add another if you can

Updated to document example images under /openshift/ and added a docker.io/library/busybox example (using busybox because busybox has already been used to demonstrate the same thing earlier in the scopes section).

Signed-off-by: Miloslav Trmač <[email protected]>

runcom · 2016-10-12T11:00:47Z

@mtrmac @aweiteka what's missing here? can we merge? I'm also going to update this after #99

mtrmac · 2016-10-12T12:43:56Z

👍

runcom · 2016-10-12T12:44:23Z

🎉 #99 will update this whole doc when it lands

mtrmac mentioned this pull request Aug 10, 2016

Verify signatures on pull containers/skopeo#157

Merged

philips reviewed Aug 10, 2016
View reviewed changes

mtrmac force-pushed the policy-docs branch 2 times, most recently from 4c7be70 to d38b7b3 Compare August 25, 2016 16:20

mtrmac force-pushed the policy-docs branch from d38b7b3 to 578d392 Compare August 25, 2016 17:23

mtrmac mentioned this pull request Aug 25, 2016

Image signature format #59

Closed

mtrmac force-pushed the policy-docs branch 3 times, most recently from 3c0be09 to b2f7e2b Compare September 1, 2016 20:29

mtrmac force-pushed the policy-docs branch 2 times, most recently from 2ec313f to 1e2e0e3 Compare September 6, 2016 14:37

mtrmac mentioned this pull request Sep 7, 2016

Refer to the policy file as a trust policy file. containers/skopeo#192

Merged

mtrmac force-pushed the policy-docs branch 3 times, most recently from 4829d34 to d7bb82e Compare September 13, 2016 17:30

mtrmac force-pushed the policy-docs branch from d7bb82e to 62245f3 Compare September 19, 2016 14:51

mtrmac force-pushed the policy-docs branch 2 times, most recently from dc5538c to 34f0a11 Compare September 26, 2016 16:10

mtrmac force-pushed the policy-docs branch from 34f0a11 to 888f007 Compare September 27, 2016 13:33

aweiteka suggested changes Oct 5, 2016

View reviewed changes

mtrmac force-pushed the policy-docs branch 3 times, most recently from 33f8708 to 10217fe Compare October 11, 2016 13:42

runcom reviewed Oct 11, 2016

View reviewed changes

Add documentation for the signature verification policy files

dd0bf87

Signed-off-by: Miloslav Trmač <[email protected]>

mtrmac force-pushed the policy-docs branch from 10217fe to dd0bf87 Compare October 11, 2016 17:06

aweiteka approved these changes Oct 12, 2016

View reviewed changes

mtrmac merged commit 5ce985a into containers:master Oct 12, 2016

mtrmac deleted the policy-docs branch October 12, 2016 12:44

runcom mentioned this pull request Oct 12, 2016

Match a signature for any tag when pulling/coping by digest #99

Closed

		@@ -0,0 +1,251 @@
		<!-- NOTE: Keep this in sync with signature/policy_types.go! -->

Add documentation for the signature verification policy files #50

Add documentation for the signature verification policy files #50

Uh oh!

Conversation

mtrmac commented Aug 10, 2016

Uh oh!

mtrmac commented Aug 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac Aug 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac Aug 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

runcom commented Aug 25, 2016

Uh oh!

mtrmac commented Aug 25, 2016

Uh oh!

mtrmac commented Sep 7, 2016

Uh oh!

mtrmac commented Sep 14, 2016

Uh oh!

runcom commented Sep 21, 2016

Uh oh!

runcom commented Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

runcom commented Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrmac commented Sep 21, 2016

Uh oh!

runcom commented Sep 21, 2016

Uh oh!

aweiteka left a comment

Choose a reason for hiding this comment

Uh oh!

aweiteka Oct 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Oct 5, 2016

Uh oh!

runcom Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

runcom commented Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

mtrmac Aug 11, 2016 •

edited

Loading

mtrmac Aug 11, 2016 •

edited

Loading

runcom commented Sep 21, 2016 •

edited

Loading

runcom commented Sep 21, 2016 •

edited

Loading

aweiteka Oct 5, 2016 •

edited

Loading

runcom Oct 11, 2016 •

edited

Loading

runcom commented Oct 12, 2016 •

edited

Loading

mtrmac commented Oct 12, 2016 •

edited by runcom

Loading

runcom commented Oct 12, 2016 •

edited

Loading