Skip to content

[Kubernetes STIG v1r10] pod-files rule: add check for 242467 rule#60

Merged
dimityrmirchev merged 4 commits intogardener:mainfrom
AleksandarSavchev:require-600-key-files
Nov 21, 2023
Merged

[Kubernetes STIG v1r10] pod-files rule: add check for 242467 rule#60
dimityrmirchev merged 4 commits intogardener:mainfrom
AleksandarSavchev:require-600-key-files

Conversation

@AleksandarSavchev
Copy link
Copy Markdown
Member

@AleksandarSavchev AleksandarSavchev commented Oct 24, 2023

What this PR does / why we need it:
This PR improves the *.key files checks required from 242467 rule in pod-files rule. *.key files in mandatory components are required to have max privilege of 640. We do not enforce 600 because k8s does not provide a way to easily change the owner of a file and containers are expected to run as nonroot.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

DISA Kubernetes STIGs `pod-files` rule now expects `0640` permission setting for `*.key` files of mandatory components. This change improves the `242467` rule which requires `0600` permissions for such files. `0600` is not enforced since k8s does not provide an easy way to change the owner of a file and containers are expected to run as nonroot. 

@AleksandarSavchev AleksandarSavchev requested a review from a team as a code owner October 24, 2023 13:19
@gardener-robot gardener-robot added needs/review size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 24, 2023
Copy link
Copy Markdown
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

Some minor requests from my side

Name string
Label string
Value string
Num int
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's remove the Num field and again use a map[string]component

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We cannot use the old method of removing a found component from the map, since a mandatory component can have more than 1 pod and we want to make the key file checks for every single pod of that component. Using the old logic we would know only for the first pod of the component that we need to check the key files.

I suggest I change the Num int filed to Found bool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok then. Lets change "Num" to "found"

]`
compliantStats = `600 0 0 /destination/file1.txt
644 0 65532 /destination/bar/file2.txt`
keyFileStats = `600 0 0 /destination/file1.key
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
keyFileStats = `600 0 0 /destination/file1.key
keyFileStats = `640 0 0 /destination/file1.key

Copy link
Copy Markdown
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

/lgtm

@ghost ghost added the reviewed/ok-to-test label Nov 21, 2023
@dimityrmirchev dimityrmirchev merged commit 7e506fe into gardener:main Nov 21, 2023
@AleksandarSavchev AleksandarSavchev deleted the require-600-key-files branch February 7, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants