[Security Hardened Shoot Cluster] Rule 2005 Implementation #363

georgibaltiev · 2024-11-18T15:07:26Z

What this PR does / why we need it:
This PR implements rule 2005 of the Security Hardened Shoot Cluster Ruleset. It evaluates the kubelet configurations of the main and worker nodes by checking the value of their connection timeouts. The checkResults are in accordance with the DISA STIG guide for rule 245541 ref.

Which issue(s) this PR fixes:
Part of #304

Special notes for your reviewer:

Release note:

Implementation for rule `2005` from the `security-hardened-shoot-cluster` ruleset for provider `garden`.

dimityrmirchev

Mainly wording suggestions, but overall looks good

dimityrmirchev · 2024-11-18T19:45:26Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+		timeoutDuration := *shoot.Spec.Kubernetes.Kubelet.StreamingConnectionIdleTimeout
+		switch {
+		case timeoutDuration.Minutes() < 5:
+			checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is not set to a valid value (< 5m).", rule.NewTarget()))


Please use allowed/not allowed in the other messages as well.

Suggested change

checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is not set to a valid value (< 5m).", rule.NewTarget()))

checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is set to a not allowed value (< 5m).", rule.NewTarget()))

dimityrmirchev · 2024-11-18T19:46:23Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+		checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))
+	} else {
+		timeoutDuration := *shoot.Spec.Kubernetes.Kubelet.StreamingConnectionIdleTimeout
+		switch {


This switch can be reused as a function that accepts the timeout duration and the target. The function can return a single check result.

dimityrmirchev · 2024-11-18T19:47:22Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+		case timeoutDuration.Minutes() < 5:
+			checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is not set to a valid value (< 5m).", rule.NewTarget()))
+		case timeoutDuration.Minutes() == 5:
+			checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))


Suggested change

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the recommended value (5m).", rule.NewTarget()))

dimityrmirchev · 2024-11-18T19:55:00Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	} else {
+		timeoutDuration := *shoot.Spec.Kubernetes.Kubelet.StreamingConnectionIdleTimeout
+		switch {
+		case timeoutDuration.Minutes() < 5:


Suggested change

case timeoutDuration.Minutes() < 5:

case timeoutDuration < 5*time.Minute:

dimityrmirchev · 2024-11-18T19:55:19Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+			checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is not set to a valid value (< 5m).", rule.NewTarget()))
+		case timeoutDuration.Minutes() == 5:
+			checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))
+		case timeoutDuration.Hours() <= 4:


Suggested change

case timeoutDuration.Hours() <= 4:

case timeoutDuration <= 4*time.Hour:

dimityrmirchev · 2024-11-18T20:01:06Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+		case timeoutDuration.Minutes() == 5:
+			checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))
+		case timeoutDuration.Hours() <= 4:
+			checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to a valid value ([5m; 4h]).", rule.NewTarget()))


Suggested change

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to a valid value ([5m; 4h]).", rule.NewTarget()))

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to an allowed, but not recommended value (should be 5m).", rule.NewTarget()))

dimityrmirchev · 2024-11-18T20:05:26Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	var checkResults = []rule.CheckResult{}
+
+	if shoot.Spec.Kubernetes.Kubelet == nil || shoot.Spec.Kubernetes.Kubelet.StreamingConnectionIdleTimeout == nil {
+		checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))


Suggested change

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", rule.NewTarget()))

checkResults = append(checkResults, rule.PassedCheckResult("The connection timeout is not set and therefore will be defaulted to the recommended value (5m).", rule.NewTarget()))

dimityrmirchev · 2024-11-19T08:08:25Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	return rule.Result(r, checkResults...), nil
+}
+
+func evaluateTimeoutDuration(timeoutDuration metav1.Duration, target rule.Target) rule.CheckResult {


Please move this function definition in the Run func so we do not pollute the whole package.

dimityrmirchev · 2024-11-19T08:08:43Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	case timeoutDuration.Duration <= 4*time.Hour:
+		return rule.PassedCheckResult("The connection timeout is set to an allowed, but not recommended value (should be 5m).", target)
+	default:
+		return rule.FailedCheckResult("The connection timeout is not set to a valid value (> 4h).", target)


Suggested change

return rule.FailedCheckResult("The connection timeout is not set to a valid value (> 4h).", target)

return rule.FailedCheckResult("The connection timeout is not set to an allowed value (> 4h).", target)

dimityrmirchev · 2024-11-19T08:09:04Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	case timeoutDuration.Duration < 5*time.Minute:
+		return rule.FailedCheckResult("The connection timeout is set to a not allowed value (< 5m).", target)
+	case timeoutDuration.Duration == 5*time.Minute:
+		return rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", target)


Suggested change

return rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", target)

return rule.PassedCheckResult("The connection timeout is set to the recommended value (5m).", target)

dimityrmirchev · 2024-11-19T08:25:48Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+			case timeoutDuration.Duration <= 4*time.Hour:
+				return rule.PassedCheckResult("The connection timeout is set to an allowed, but not recommended value (should be 5m).", target)
+			default:
+				return rule.FailedCheckResult("The connection timeout is not set to an allowed value (> 4h).", target)


Suggested change

return rule.FailedCheckResult("The connection timeout is not set to an allowed value (> 4h).", target)

return rule.FailedCheckResult("The connection timeout is set to a not allowed value (> 4h).", target)

I just saw that we use the following wording when the duration is <5m. Let's make this the same across the two cases.

AleksandarSavchev

Thanks! Just one nit.

AleksandarSavchev · 2024-11-19T09:13:25Z

pkg/provider/garden/ruleset/securityhardenedshoot/rules/2005.go

+	}
+
+	var (
+		checkResults            = []rule.CheckResult{}


Suggested change

checkResults = []rule.CheckResult{}

checkResults []rule.CheckResult

AleksandarSavchev

/lgtm

dimityrmirchev

/lgtm

Add Rule 2005

38feb4a

georgibaltiev requested a review from a team as a code owner November 18, 2024 15:07

gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Nov 18, 2024

dimityrmirchev requested changes Nov 18, 2024

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Nov 18, 2024

Refactor with suggestions

dae3f44

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 19, 2024

Add target to the errored check result

bd74772

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 19, 2024

dimityrmirchev requested changes Nov 19, 2024

View reviewed changes

Refactored suggestions

7992cd3

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 19, 2024

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 19, 2024

dimityrmirchev reviewed Nov 19, 2024

View reviewed changes

Add suggestions

d115fa8

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 19, 2024

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 19, 2024

Typo fix

4a088ba

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 19, 2024

AleksandarSavchev requested changes Nov 19, 2024

View reviewed changes

Slice initialization

b27910d

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 19, 2024

AleksandarSavchev approved these changes Nov 19, 2024

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review labels Nov 19, 2024

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 19, 2024

dimityrmirchev approved these changes Nov 19, 2024

View reviewed changes

dimityrmirchev merged commit dbceeac into gardener:main Nov 19, 2024
9 checks passed

gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Nov 19, 2024

AleksandarSavchev mentioned this pull request Nov 20, 2024

☂ Implement Garden provider & Security Hardened Shoot Cluster ruleset #304

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Hardened Shoot Cluster] Rule 2005 Implementation #363

[Security Hardened Shoot Cluster] Rule 2005 Implementation #363

georgibaltiev commented Nov 18, 2024

dimityrmirchev left a comment

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 18, 2024

dimityrmirchev Nov 19, 2024

dimityrmirchev Nov 19, 2024

dimityrmirchev Nov 19, 2024

dimityrmirchev Nov 19, 2024

dimityrmirchev Nov 19, 2024

AleksandarSavchev left a comment

AleksandarSavchev Nov 19, 2024

AleksandarSavchev left a comment

dimityrmirchev left a comment

	checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is not set to a valid value (< 5m).", rule.NewTarget()))
	checkResults = append(checkResults, rule.FailedCheckResult("The connection timeout is set to a not allowed value (< 5m).", rule.NewTarget()))

	case timeoutDuration.Minutes() < 5:
	case timeoutDuration < 5*time.Minute:

	case timeoutDuration.Hours() <= 4:
	case timeoutDuration <= 4*time.Hour:

	return rule.FailedCheckResult("The connection timeout is not set to a valid value (> 4h).", target)
	return rule.FailedCheckResult("The connection timeout is not set to an allowed value (> 4h).", target)

	return rule.PassedCheckResult("The connection timeout is set to the reccomended value (5m).", target)
	return rule.PassedCheckResult("The connection timeout is set to the recommended value (5m).", target)

	checkResults = []rule.CheckResult{}
	checkResults []rule.CheckResult

[Security Hardened Shoot Cluster] Rule 2005 Implementation #363

[Security Hardened Shoot Cluster] Rule 2005 Implementation #363

Conversation

georgibaltiev commented Nov 18, 2024

dimityrmirchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AleksandarSavchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AleksandarSavchev left a comment

Choose a reason for hiding this comment

dimityrmirchev left a comment

Choose a reason for hiding this comment