[WIP] Security Config for services #1944

ehazlett · 2017-02-10T02:23:48Z

The security config provides a way to add security settings
that may or may not be host specific. This is stored as part
of the ContainerSpec and is applied as necessary.

This is needed for things like userns and credentialspec.

For more details see moby/moby#30894

The security config provides a way to add security settings that may or may not be host specific. This is stored as part of the ContainerSpec and is applied as necessary. Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

aaronlehmann · 2017-02-10T02:25:47Z

api/specs.proto

+
+	message SecurityConfig {
+	    string userns = 30;
+	    string credential_spec = 31 [(gogoproto.customname) = "CredentialSpec"];


Why 30 and 31?

Basically because...

In all seriousness, I saw that registry_auth was high so I put this one to start at 30. Completely arbitrary and I defer to you for what it should be :)

This is a separate message and the fields can start from 1.

registry_auth was a bit of an exception ...

We added it knowing in advance that we'd deprecate it (in favor of using secrets to store registry auth) soooo we picked a high number we'd be unlikely to need in the future. Sorry you had to witness that :) We were trying to hide it under the rug.

aaronlehmann · 2017-02-10T02:25:54Z

api/specs.proto

+	    string userns = 30;
+	    string credential_spec = 31 [(gogoproto.customname) = "CredentialSpec"];
+	}
+	SecurityConfig security_config = 40;


see above :D 👼

I think our general practice has been to avoid gaps between field numbers.

aaronlehmann · 2017-02-10T02:28:01Z

protobuf/plugin/deepcopy/test/deepcopy.pb.go

@@ -46,93 +46,93 @@ var _ = math.Inf
 const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package

 type BasicScalar struct {
-	Field1  float64 `protobuf:"fixed64,1,opt,name=Field1,json=field1,proto3" json:"Field1,omitempty"`


These changes look unrelated. Perhaps you are using a different version of protoc. CI is using https://github.com/google/protobuf/releases/download/v3.0.2/protoc-3.0.2-linux-x86_64.zip

ah crap yes -- i'll remove that and grab the 3.0.2

cyli · 2017-02-10T02:34:37Z

api/specs.proto

@@ -258,6 +258,12 @@ message ContainerSpec {
 	// task will exit and a new task will be rescheduled elsewhere. A container
 	// is considered unhealthy after `Retries` number of consecutive failures.
 	HealthConfig healthcheck = 16;
+
+	message SecurityConfig {
+	    string userns = 30;


userns and credential_spec seem somewhat specific to moby/moby#23389 - wondering if we should have different types of SecurityConfig, and this could be one type?

Are there other different security config types do you think go into the SecurityConfig? @diogomonica can security profiles go here, for instance? (not sure if there are windows specific capabilities that should translate to instead).

Ya I should have brought that comment here. In the Engine PR I mention that I would like to also add --capabilities and a few others to this as well as anything else that other think makes sense.

aluzzardi · 2017-02-10T02:38:11Z

Thanks for chipping in, @ehazlett :)

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

aluzzardi · 2017-02-10T03:03:28Z

api/specs.proto

+
+	message SecurityConfig {
+	    string userns = 1;
+	    string credential_spec = 2 [(gogoproto.customname) = "CredentialSpec"];


nit: customname is not required, I believe that's how it's going to get generated anyway.

We use custom names for stuff like DNSConfig (otherwise it would get generated as DnsConfig)

ah right -- i was thinking i had credentialspec where it would make Credentialspec -- i'll update thx

What will this field contain?

It should contain a string that is something like file:///... or registry:// I believe (https://github.com/docker/docker/blob/c0a1d2e0d88ff3cae6802dfbd128c7739e8c2bcc/daemon/start_windows.go#L135). @johnstep can you confirm?

Is this windows specific? How does this relate to security profiles?

Yes it's windows specific. I believe it specifies a path to a "credential blob" (moby/moby#23389). This PR was supposed to be about security configuration and not specific to security profiles (although full disclosure I'm not sure what exactly security profiles mean as related to docker). Since they are authentication related and point to what seems like an authentication blob I thought it would be classified as "security".

Based upon the latest comment moby/moby#30894 (comment) this whole thing might be moot anyway.

I think maybe that particular objection was to the docker service create CLI flags. (cc @diogomonica)

AFAIK the current plan is for low level capabilities to go into the spec and the API. Possibly in a versioned object (or typed objects, like "custom" for the low level capabilities), and for the something else (like the CLI) to translate from high level grants to groups low level capabilities. This is the current tentative plan for security profiles, because it's likely we won't get the first iteration of high level profiles right.

Even if the CLI is not the correct place to put the mapping, and the API needs to accept the higher level API instead, having a version of the profile that contains the low level capabilities in case someone wanted to specify a custom profile could be useful.

But to reiterate @stevvooe's question - I wasn't sure how the other security-related options would relate to security profiles in this object model (should profiles be a separate object in a security configuration for instance, etc.)

Correct.

SecurityProfile being distinct from SecurityConfig makes sense in my head.

aluzzardi · 2017-02-10T03:04:59Z

This is @cyli and @diogomonica's area

ehazlett · 2017-02-14T15:54:08Z

I'm closing this in favor of "security profiles". I will with the security team to get that started.

cyli · 2017-02-15T22:14:22Z

@diogomonica @ehazlett Seems like the security profiles are a separate concern/object from these particular options (user namespace , credential specs), which seem like they can move forward if we are happy with this config object?

I think we just needed another version/typed object representing security profiles, the first version of which contains a full list of capabilities.

Another question I had was whether we also wanted to support app armor and seccomp profiles too? If so, those should probably be specified here in the config object as well.

diogomonica · 2017-02-15T22:46:42Z

app armor and seccomp are definitely part of security profiles, no?
I'm not sure if adding an OS-spoecific "credential specs" is the right thing to do here, since it will only work on windows.
Same for user namespace. Specific for linux. ideally this would all go into the security profile that would be generic enough to contain all of these?

cyli · 2017-02-15T23:08:37Z

@diogomonica I see... then maybe something like (I may be way overusing oneof here, and this may be overly redundant):

message SecurityProfile {
	oneof arch_profile {
		WindowsSecurityProfile windows = 1;
		LinuxSecurityProfile linux = 2;
	}
}

message WindowsSecurityProfile {
       string credential_spec = 1;
       // ?????
}

message LinuxSecurityProfile {
       string userns = 1;
       oneof profile_version {
            LinuxRawSecurityProfile raw = 2;
       }
}

message LinuxRawSecurityProfile {
       repeated string capabilities = 1;
       string app_armor = 2;
       string seccomp = 3;
}

diogomonica · 2017-02-15T23:24:44Z

@cyli yeah, we might not have a one-off. Same container might run in different arch's w/ different profiles.

In general that sort of what I'm thinking about. The current work from @justincormack would get folded into LinuxRawSecurityProfile as much as possible, and then we would have to do the hard work of splitting high-level grands to the low-level perms.

/cc @dsampath

cyli · 2017-02-15T23:32:42Z

@ehazlett Would you like to continue that work here, or alternately if you prefer I can submit a new PR with just a proposed object model for security profiles?

ehazlett · 2017-02-15T23:34:47Z

You have a better understanding than I do. I would appreciate if you submitted. Thanks

…

On Feb 15, 2017 6:32 PM, "Ying Li" ***@***.***> wrote: @ehazlett <https://github.com/ehazlett> Would you like to continue that work here, or alternately if you prefer I can submit a new PR with just the object model for security profiles? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1944 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAP6InjRz6Yga-ZV78Ru7Vy0HNO_BZ-4ks5rc4sdgaJpZM4L87CR> .

diogomonica · 2017-02-15T23:42:55Z

@cyli I'm all for unblocking this particular feature (cred specs), but I'm slightly concerned we don't know exactly how this fits in the larger picture.

Anyway, maybe the discussion on a new PR brings some more clarity on whether we're missing something, or this is a good path.

add Security Config for services

88668ea

The security config provides a way to add security settings that may or may not be host specific. This is stored as part of the ContainerSpec and is applied as necessary. Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

ehazlett mentioned this pull request Feb 10, 2017

[WIP] Service Security Config moby/moby#30894

Closed

aaronlehmann reviewed Feb 10, 2017

View reviewed changes

cyli reviewed Feb 10, 2017

View reviewed changes

revert deepcopy.pb.go; fix tags

f618f60

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

aluzzardi reviewed Feb 10, 2017

View reviewed changes

ehazlett closed this Feb 14, 2017

cyli mentioned this pull request Feb 17, 2017

specs: Add minimum raw security profiles needed by plugins to ContainerSpec and PluginSpec #1964

Closed

aluzzardi mentioned this pull request Mar 30, 2017

Privilege Profile support #2075

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] Security Config for services #1944

[WIP] Security Config for services #1944

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[WIP] Security Config for services #1944

[WIP] Security Config for services #1944

Uh oh!

Conversation

Uh oh!

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!

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

C 9E88 hoose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!