Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add security considerations #909

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

burgerdev
Copy link
Contributor

Outlines a couple of DOs and DON'Ts when writing apps for Contrast. The idea is that, after reading the recommendations, an author of a Kubernetes app can tell what's safe to do in YAML and what needs scrutiny.

@burgerdev burgerdev added the documentation Improvements for user docs label Oct 4, 2024
Copy link

github-actions bot commented Oct 4, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-10-04 14:50 UTC

docs/docs/security-considerations.md Outdated Show resolved Hide resolved
Thus, it's not possible to disambiguate, for example, pods spawned from a deployment, or to limit the amount of certificates issued per policy.

Container image references from Kubernetes resource definitions are taken into account when generating the policy.
A mutable reference may lead to policy failures or unverified image content, depending on the Contrast runtime.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably try to catch this and either log a warning or abort entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I'd lean towards keeping the advice here for now and remove it once we agreed on a path forward (which is not obvious, considering other methods of integrity protection, like the tardev snapshotter or signed images).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure, implementing that would be way out of the scope of this pr.

docs/docs/security-considerations.md Outdated Show resolved Hide resolved
docs/docs/security-considerations.md Outdated Show resolved Hide resolved
docs/docs/security-considerations.md Outdated Show resolved Hide resolved
@burgerdev burgerdev force-pushed the burgerdev/security-considerations branch 2 times, most recently from 69fb22d to 35ff1cb Compare October 4, 2024 12:36
@burgerdev
Copy link
Contributor Author

All feedback addressed.

Container image references from Kubernetes resource definitions are taken into account when generating the policy.
A mutable reference may lead to policy failures or unverified image content, depending on the Contrast runtime.
Reliability and security can only be ensured with a full image reference, including digest.
The [`docker pull` documentation] explains pinned image references in detail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a link missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a shortcut reference link. I like that it does not interrupt the sentence in the markdown source.

Copy link
Member

@m1ghtym0 m1ghtym0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Did you envision this as a high-level section below the "workload deployment"?
Remember to add the sidebar entry before merging.

Co-authored-by: Tom Dohrmann <[email protected]>
Co-authored-by: Moritz Eckert <[email protected]>
@burgerdev burgerdev force-pushed the burgerdev/security-considerations branch from 35ff1cb to a72a9b6 Compare October 4, 2024 13:41
@burgerdev
Copy link
Contributor Author

Thanks! Did you envision this as a high-level section below the "workload deployment"? Remember to add the sidebar entry before merging.

I reconsidered and moved it into Architecture for now - don't think this needs to be a top level item.

@burgerdev burgerdev merged commit c0b2daa into main Oct 4, 2024
7 checks passed
@burgerdev burgerdev deleted the burgerdev/security-considerations branch October 4, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements for user docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants