feat: validation only mode

[xopham]

relates to #712

Description

allow configuration to not mutate image references but simply validate whether signed digest exists

Checklist

• PR is rebased to/aimed at branch develop
• PR follows Contributing Guide
• Added tests (if necessary)
• Extended README/Documentation (if necessary)
• Adjusted versions of image and Helm chart in values.yaml and Chart.yaml (if necessary)

[codecov-commenter]

Codecov Report

Merging #759 (9b5ce7a) into develop (365ac06) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #759   +/-   ##
========================================
  Coverage    96.81%   96.82%           
========================================
  Files           22       22           
  Lines         1226     1227    +1     
========================================
+ Hits          1187     1188    +1     
  Misses          39       39           

Impacted Files	Coverage Δ
connaisseur/flask_application.py	92.70% <100.00%> (+0.07%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[phbelitz]

lgtm

[Starkteetje]

This does not actually fix #270 as

but fail if the a reference would require mutation.

is not actually what happens here. This lowers the security pretty significantly.

[Starkteetje]

Would add a disclaimer that this actually lowers security (significantly I would argue, since the threat model for Connaisseur basically is that the attacker has control over the registry or connection to registry).

[xopham]

there is a disclaimer in the docs, you think it must be in the helm as well?

[Starkteetje]

It basically undermines every security guarantee that Conny provides. So yes. I'm also not sure whether it's something we should allow in the first place, because it so significantly undermines it, but if so, there should be as disclaimers at every point this is mentioned

[Starkteetje]

Thinking about it, I would prefix the config var with unsafe

[Starkteetje]

like unsafeOnlyValidateTags: false

edit: in addition to the comment

[xopham]

I disagree in that it undermines EVERY security guarantee. It significantly complicates a practical attack.
Moreover, detectionMode is in some ways worse than that as it gives a warning but that may never be read and we do not denote it as unsafe

[xopham]

also namespacedValidation can have similarly detrimental effects

[Starkteetje]

detectionMode on the other hand has a very clear interface. It "detects" but doesn't do more. It has very clear applications for testing and thus can improve security by enabling to get the tool into a pre-existing infrastructure without breakage.

validationMode on the other hand doesn't have a super clear-cut interface. It only validates, but somewhat wants to provide security. The distinction is not apparent from naming alone that it will validate existence of a digest, but make no pull guarantees. While it does enable some infrastructures to validate digests, it doesn't give them the actual guarantees that one would assume to get from an admission controller such as Conny

If you feel strongly about not adding the unsafe prefix, how about something like

security-enforcement (kinda bad name, let's pick another):
  - manifest-security: true # whether to enforce that the manifest corresponds to a validly signed (not signed, because could be allowlisted but you know i mean) (basically this = false equivalent to detectionMode -> unify with next major)
  - runtime-security: true # whether to enforce the pulled image is actually the signed (not signed, because could be allowlisted but you know i mean) image and not another one (this = false is your validation only mode)

(if you find the general idea reasonable, let's sit a few minutes for proper names)

[xopham]

@Starkteetje it does fail if there is no trusted digest:

$ kubectl run unsigned --image=docker.io/securesystemsengineering/testimage:unsigned
Error from server: admission webhook "connaisseur-svc.connaisseur.svc" denied the request: Unable to find signed digest for image docker.io/securesystemsengineering/testimage:unsigned.

You are correct in that it does not fix #270 or at least a requirement therein. Will highlight more strictly in the docs that it may be better to stick to mutation and use digests. Otherwise, I see no way to solve #712

[Starkteetje]

Suggested change

	In consequence, a mutated image reference causes an error for such tools, as actual and requested resource differ.
	In consequence, a mutated image reference causes an error for such tools, as actual and requested resource references differ.

?

[Starkteetje]

Suggested change

	This supposedly small time window might be significantly larger if images are re-pulled by the container runtime at a later time for some reason.
	This supposedly small time window might be significantly larger if images are re-pulled by the container runtime at a later time for some reason or if the attacker completely controls the image registry.

[phbelitz]

already implemented in golang version