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

To fix CMP-2548 #12008

Merged
merged 6 commits into from
Jun 25, 2024
Merged

To fix CMP-2548 #12008

merged 6 commits into from
Jun 25, 2024

Conversation

rutvik23
Copy link
Contributor

Description:

This rule controller_rotate_kubelet_server_certs is no longer valid as it still checking for feature-gate parameter which does not exist in any supported versions of OpenShift 4.

$ ./oc get ccr | grep cert | grep -i fail

ocp4-high-controller-rotate-kubelet-server-certs FAIL medium

Reproducible in Compliance Operator v1.4.0 & OpenShift v4.14.z

Rationale:

  • Fixes #CMP-2548
  • The existing rules like kubelet_enable_cert_rotation, kubelet_enable_server_cert_rotation already satisfy this check.

Review Hints:

  • Another rule that rely on feature-gate parameter which does not exist, was disabled recently through CMP-2331

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label May 20, 2024
Copy link

openshift-ci bot commented May 20, 2024

Hi @rutvik23. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

github-actions bot commented May 20, 2024

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:12008
This image was built from commit: 3beb2f6

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:12008

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:12008 make deploy-local

@marcusburghardt marcusburghardt added the OpenShift OpenShift product related. label May 21, 2024
@yuumasato
Copy link
Member

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Used by openshift-ci bot. and removed needs-ok-to-test Used by openshift-ci bot. labels May 22, 2024
@yuumasato
Copy link
Member

yuumasato commented May 22, 2024

/test 4.13-e2e-aws-ocp4-moderate
/test 4.14-e2e-aws-ocp4-moderate
/test 4.15-e2e-aws-ocp4-moderate
/test 4.16-e2e-aws-ocp4-moderate
/test e2e-aws-ocp4-moderate

@xiaojiey
Copy link
Collaborator

/hold for test

@openshift-ci openshift-ci bot added the do-not-merge/hold Used by openshift-ci-robot bot. label May 27, 2024
@BhargaviGudi
Copy link
Collaborator

Verification passed with 4.16.0-0.nightly-2024-05-23-173505 + https://github.com/ComplianceAsCode/compliance-operator code + PR #12008 code

$ oc get pb
NAME              CONTENTIMAGE                                 CONTENTFILE         STATUS
ocp4              ghcr.io/complianceascode/k8scontent:latest   ssg-ocp4-ds.xml     VALID
rhcos4            ghcr.io/complianceascode/k8scontent:latest   ssg-rhcos4-ds.xml   VALID
upstream-ocp4     ghcr.io/complianceascode/k8scontent:12008    ssg-ocp4-ds.xml     VALID
upstream-rhcos4   ghcr.io/complianceascode/k8scontent:12008    ssg-rhcos4-ds.xml   VALID
$ oc get rule upstream-ocp4-controller-rotate-kubelet-server-certs -ojsonpath={.instructions}
RotateKubeletServerCertificate is no longer a valid check, instead an user should run below commands and confirm the rotation settings are enabled as default.
$ for node in $(oc get nodes -ojsonpath='{.items[*].metadata.name}'); do oc get --raw /api/v1/nodes/$node/proxy/configz | jq '.kubeletconfig.serverTLSBootstrap'; done
$ for NODE_NAME in $(oc get nodes -ojsonpath='{.items[*].metadata.name}'); do oc get --raw /api/v1/nodes/$NODE_NAME/proxy/configz | jq '.kubeletconfig|.kind="KubeletConfiguration"|.apiVersion="kubelet.config.k8s.io/v1beta1"' | grep rotateCertificates; done
The output should return true
      Is it the case that <tt>serverTLSBootstrap</tt> argument is set to <tt>false</tt> in the 
$ for node in $(oc get nodes -ojsonpath='{.items[*].metadata.name}'); do oc get --raw /api/v1/nodes/$node/proxy/configz | jq '.kubeletconfig.serverTLSBootstrap'; done
true
true
true
true
true
true
$ for NODE_NAME in $(oc get nodes -ojsonpath='{.items[*].metadata.name}'); do oc get --raw /api/v1/nodes/$NODE_NAME/proxy/configz | jq '.kubeletconfig|.kind="KubeletConfiguration"|.apiVersion="kubelet.config.k8s.io/v1beta1"' | grep rotateCertificates; done
  "rotateCertificates": true,
  "rotateCertificates": true,
  "rotateCertificates": true,
  "rotateCertificates": true,
  "rotateCertificates": true,
  "rotateCertificates": true,

@BhargaviGudi
Copy link
Collaborator

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label May 27, 2024
@xiaojiey
Copy link
Collaborator

/hold for test

@openshift-ci openshift-ci bot added the do-not-merge/hold Used by openshift-ci-robot bot. label May 29, 2024
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Let's unselect the rule so that no profile uses this rule anymore.

grep -r controller_rotate_kubelet_server_certs controls/ products/
controls/srg_ctr/SRG-APP-000516-CTR-001325.yml:  - controller_rotate_kubelet_server_certs
controls/nist_ocp4.yml:  - controller_rotate_kubelet_server_certs
controls/nist_ocp4.yml:  - controller_rotate_kubelet_server_certs
controls/nist_ocp4.yml:  - controller_rotate_kubelet_server_certs
controls/nist_ocp4.yml:  - controller_rotate_kubelet_server_certs
products/ocp4/profiles/stig-v1r1.profile:    - controller_rotate_kubelet_server_certs

And let's add a general warning noting that this rule is deprecated.
Example warning in the rule:https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/auditing/auditd_configure_rules/audit_rules_etc_passwd_openat/rule.yml#L37
Inspiration for warning text: https://github.com/ComplianceAsCode/content/blob/master/shared/macros/10-warning.jinja#L30

@yuumasato yuumasato self-assigned this Jun 17, 2024
@BhargaviGudi
Copy link
Collaborator

Verification passed with 4.16.0-0.nightly-2024-06-14-130320 + https://github.com/ComplianceAsCode/compliance-operator code + PR #12008 code

  1. Install CO
$ oc get pb
NAME              CONTENTIMAGE                                 CONTENTFILE         STATUS
ocp4              ghcr.io/complianceascode/k8scontent:latest   ssg-ocp4-ds.xml     VALID
rhcos4            ghcr.io/complianceascode/k8scontent:latest   ssg-rhcos4-ds.xml   VALID
upstream-ocp4     ghcr.io/complianceascode/k8scontent:12008    ssg-ocp4-ds.xml     VALID
upstream-rhcos4   ghcr.io/complianceascode/k8scontent:12008    ssg-rhcos4-ds.xml   VALID
  1. Verify rule controller-rotate-kubelet-server-certs does not exists
$ oc get rule | grep controller-rotate-kubelet-server-certs | grep upstream
$ 
$ oc compliance bind -N test -S default-auto-apply profile/upstream-ocp4-high
Creating ScanSettingBinding test
$ oc get scan
NAME                 PHASE   RESULT
upstream-ocp4-high   DONE    NON-COMPLIANT
$ oc get ccr | grep controller-rotate-kubelet-server-certs
$

@BhargaviGudi
Copy link
Collaborator

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label Jun 18, 2024
@BhargaviGudi BhargaviGudi reopened this Jun 18, 2024
@rutvik23 rutvik23 requested a review from yuumasato June 18, 2024 11:45
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

From what I understand, what makes this rule fails is that feature-gates doesn't list RotateKubeletServerCertificate=true any more. At some point OCP stopped listing it and this rule became not applicable.

And for the rule to result in NOT APPLICABLE the platform needs to be updated.
For example:
platform: not ocp4-on-hypershift-hosted and (ocp4.10 or ocp4.11)

I tried checking which versions have RotateKubeletServerCertificate listed in feature-gates but my clusters installs are failing... I can try getting the list of versions tomorrow.

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Hi, @rutvik23 sorry for the back and forth.

The RotateKubeletServerCertificate=true featture gate is available on 4.12, 4.13:

$ oc version
Client Version: 4.14.1
Kustomize Version: v5.0.1
Server Version: 4.12.0-0.nightly-2024-06-20-082732
Kubernetes Version: v1.25.16+306a47e

$ oc get configmaps config -n openshift-kube-controller-manager -ojson | jq -r '.data["config.yaml"]' | jq -r '.extendedArguments["feature-gates"]'                                                                                                                                        
[
  "APIPriorityAndFairness=true",
  "RotateKubeletServerCertificate=true",
  "DownwardAPIHugePages=true",
  "CSIMigrationAzureFile=false",
  "CSIMigrationvSphere=false"
]

So I think the rule should be available and working for these versions.
keep the rule selected in the profiles. The following changes should be undone:

  • controls/nist_ocp4.yml
  • controls/srg_ctr/SRG-APP-000516-CTR-001325.yml
  • products/ocp4/profiles/stig-v1r1.profile

To make the rules not applicable on 4.13 and 4.14:
platform: not ocp4-on-hypershift-hosted and (ocp4.12 or ocp4.13)

Then override the e2e test results for 4.12 and 4.13. Copy the e2e.yml and name it 4.12.yml and 4.13.yml.
For inspiration: https://github.com/ComplianceAsCode/content/blob/master/applications/openshift/worker/file_permissions_proxy_kubeconfig/tests/ocp4/4.12.yml

@xiaojiey
Copy link
Collaborator

/hold for test

@openshift-ci openshift-ci bot added the do-not-merge/hold Used by openshift-ci-robot bot. label Jun 21, 2024
@rutvik23 rutvik23 requested a review from yuumasato June 24, 2024 10:11
@yuumasato
Copy link
Member

/test 4.12-e2e-aws-ocp4-high
/test 4.13-e2e-aws-ocp4-high
/test 4.14-e2e-aws-ocp4-high
/test 4.15-e2e-aws-ocp4-high
/test 4.16-e2e-aws-ocp4-high
/test e2e-aws-ocp4-high

Copy link

codeclimate bot commented Jun 24, 2024

Code Climate has analyzed commit 3beb2f6 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 59.4% (0.0% change).

View more on Code Climate.

@yuumasato
Copy link
Member

/retest

@yuumasato
Copy link
Member

/test 4.12-e2e-aws-ocp4-high
/test 4.13-e2e-aws-ocp4-high
/test 4.14-e2e-aws-ocp4-high
/test 4.15-e2e-aws-ocp4-high
/test 4.16-e2e-aws-ocp4-high
/test e2e-aws-ocp4-high

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Thank you @rutvik23

@yuumasato
Copy link
Member

Skipping the ansible hardening tests.

@yuumasato yuumasato merged commit 9758517 into ComplianceAsCode:master Jun 25, 2024
72 of 119 checks passed
@yuumasato yuumasato added this to the 0.1.74 milestone Jun 25, 2024
@yuumasato
Copy link
Member

@xiaojiey Apologies. I just noticed the hold for merge label.
Let me know if you run into any issues with this PR..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Used by openshift-ci-robot bot. ok-to-test Used by openshift-ci bot. OpenShift OpenShift product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants