Set secure-pod-defaults to "enabled" by default

[nak3]

In what area(s)?

/area API

Describe the feature

As per configmap comment:

serving/config/core/configmaps/features.yaml

Lines 46 to 48 in c91f8c4

	# This value will default to "enabled" in a future release,
	# probably Knative 1.10
	secure-pod-defaults: "disabled"

It says "probably Knative 1.10" but it is still disabled by default.
I opened this for tracking issue.

/cc @evankanderson

[kauana]

/unassign @pradnyavmw

[kauana]

/assign @kauana

[dprotaso]

Pairing with @kauana - we've discovered a few things

First the migrator job is doing extra patches when it shouldn't need to filed an issue here- knative/pkg#2845

Oddly the empty patch is failing because we're hitting this validation - where the old and new spec are the same (unsure why) and the updater annotation is changing - which our webhook checks reject

https://github.com/knative/pkg/blob/6cf4b051de4f9859b0f1b80f790a83e84a75bc5b/apis/metadata_validation.go#L85

Even if we relaxed this requirement we observed the empty patch+defaulting triggers the creation of new a revision. It might make sense that this secure defaulting is only done during creation instead of update.

Finally, a more generic observation around this entire feature is that when we enable it our own hello world image doesn't work. This also applies to any image that runs as root. Hence enabling this by default will probably break ALOT of users especially for demos etc.

Curious what others think?

[nak3]

Thank you so much Kauana and Dave.

Finally, a more generic observation around this entire feature is that when we enable it our own hello world image doesn't work. This also applies to any image that runs as root. Hence enabling this by default will probably break ALOT of users especially for demos etc.

I think first of all we should start with adopting non-root for any images in our docs including hello world.
Then, it would be better to make an announcement, observe the feedback for a while, and then consider changing the default?

[dprotaso]

I think first of all we should start with adopting non-root for any images in our docs including hello world.

I agree

Then, it would be better to make an announcement, observe the feedback for a while, and then consider changing the default?

I agree - that gathering data is probably the right thing to do.

[kauana]

Sounds good. Can I create an issue to make all serving code samples run as non-root?

[dprotaso]

Sounds good. Can I create an issue to make all serving code samples run as non-root?

Yup sounds good

[dprotaso]

cc @evankanderson for input

Curious if you have thoughts about the tension here where if we enable this by default it will break a lot of existing deployments and require folks to rewrite their images (if that's even possible for some).

[evankanderson]

I wanted to enable this at some point; if you explicitly request the insecure values, they are still allowed. It's just that the defaults will point towards secure values if not set. This will break people who are depending on the insecure values but haven't explicitly specified them. The trade-off is that people who are using the insecure values by accident will move to more secure settings. This mostly helps with defense-in-depth if the application is insecure, so it also isn't a critical defense.

[evankanderson]

The non-root part is probably the most disruptive; I'd be okay putting that behind a second flag to roll out the seccompProfile and allowPrivilegeEscalation bits earlier. But I think we definitely need to do a lot of communication about this, and I definitely haven't done so yet, so I don't think we can default this to "on" until we've done so.

[kauana]

Created issue to make knative sample images run as non-root: #14566

[dprotaso]

Following up on Evan's feedback - we can make secure-pod-defaults an enum

• Disabled - do nothing
• Enabled - enable most options but it still works with non-root images - still warn about nonRoot
• Restricted - enable all options that the restricted profile requires