Refactor helm deployment

[annekebr]

When touching the helm deployment the next time, please consider the following things:

• The way how we allow configurations of Kubernetes resource settings differs from resource to resource. We have an own section service to configure the Kubernetes Service, but configuration for the webhook is handled in the deployment section (which I would have assumed to cover only settings for the Kubernetes Deployment resource judging from the additional section for the Service resource. I'd suggest to decide for one option for consistency reasons.

• Why do we allow the Service type to be configurable? Did we ever test Connaisseur with another type than ClusterIP? Are we sure that our security assumptions hold for all service types equally?

• In certain circumstances (with a high probability), Connaisseur deploys empty resources (e.g. env or alertconfig secret, there may be more). Are we sure that we want that? -> https://github.com/sse-secure-systems/connaisseur/pull/958 ✔️

• Do we want to explicitly enable features by adding an extra yaml subfield enabled: true like we do for e.g. automaticChildApproval? However we decide, I'd suggest to do it consistently - right now, detectionMode behaves differently than automaticChildApproval for example. -> https://github.com/sse-secure-systems/connaisseur/pull/984 ✔️

• Tying into the point above: How do we want to handle keys, that are commented out? Do we want to be able to achieve the same behaviour with explicit values? Right now, there is no way for e.g. alerting to be turned off if not having the config block commented out.

• We once decided to allow no trust root in notaryv1 validator instances for the preconfig to work. This results in inconsistent trust root validations. I'd like to question the decision again, maybe we should rethink that.

• [Naming] Alerting has a templates list. I'd suggest to rename it to receivers. -> https://github.com/sse-secure-systems/connaisseur/pull/984 ✔️

• ~[Naming] Should the policy section rather be policies?~ -> https://github.com/sse-secure-systems/connaisseur/issues/496#issuecomment-1495810148

[xopham]

Consider deprecating default for trust roots in light of #428

[xopham]

Consider dropping dockerhub official public key

[annekebr]

See also https://github.com/sse-secure-systems/connaisseur/issues/383 - might become more relevant when users can configure their own side cars etc.

[xopham]

normalize naming either camel-case or snake case

[annekebr]

• Check whether reinvocationPolicy: Never could lead to the scenario that another admission controller e.g. adds a sidecar container to a Pod after Connaisseur did its mutation, thus, leading to unverified containers in a deployment. If so, change default such that this cannot happen by default

[xopham]

improve hierarchy and summarize all features under a feature key

[Starkteetje]

Refactor version mgmt:

• Version should probably only be in Chart.yml, bc valid values.yml depends on which helm version it is, so should put it there -> 〰️ decided to default to Chart, but allow config in helm/values.yaml
• Split off tag from image? -> ✔️
• Single version instead of app and helm version?

[Starkteetje]

• Check whether reinvocationPolicy: Never could lead to the scenario that another admission controller e.g. adds a sidecar container to a Pod after Connaisseur did its mutation, thus, leading to unverified containers in a deployment. If so, change default such that this cannot happen by default

Since we're not technically idempotent due to a change in image tag potentially resulting in a different policy being applied, changing the reinvocationPolicy by default seems risky. However, I agree that this is a potential vector to bypass validation if there's neglience in another mutating admission controller (an attacker controlling with direct access to admission controllers may also be able to forge Connaisseurs controller instead of hijacking another one) @annekebr what do you think about documenting this as a limitation instead? Other option I'd see is if user could set a flag validateMutationSafe or the like, which then employs a validating admission controller and whose documentation suggests using policies that lend themselves to idempotent execution through Connaisseur, e.g. policies don't specify human-readable tags and instead either * or digests

[Starkteetje]

[Naming] Should the policy section rather be policies?

Talked with @phbelitz and we both agreed that it is a single policy with multiple rules below it (which is also the notion in the explaining comments), so would not rename

[Starkteetje]

Should we move to explicitly support only currently supported k8s versions? (while not purposefully bricking old ones of course)

E.g. since start of April the legacy tests fail since their runners were deprecated for good and I don't see that we should invest more time into fixing up these tests.

Afaik @phbelitz agrees. @xopham Thoughts?