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

Fix ownership related to Calico #8072

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

oomichi
Copy link
Contributor

@oomichi oomichi commented Oct 11, 2021

What type of PR is this?

/kind bug

What this PR does / why we need it:

kube-bench scan outputs warning related to Calico like:

  • text: "Ensure that the Container Network Interface file permissions are set to 644 or more restrictive (Manual)"
  • text: "Ensure that the Container Network Interface file ownership is set to root:root (Manual)"

Which issue(s) this PR fixes:

Fixes #8070

Does this PR introduce a user-facing change?:

[Calico] Fix Kube-bench security warnings on calico controller (file ownership/permissions)

kube-bench scan outputs warning related to Calico like:

* text: "Ensure that the Container Network Interface file
  permissions are set to 644 or more restrictive (Manual)"
* text: "Ensure that the Container Network Interface file
  ownership is set to root:root (Manual)"

This fixes these warnings.
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 11, 2021
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 11, 2021
@oomichi
Copy link
Contributor Author

oomichi commented Oct 11, 2021

/cc @cristicalin

@k8s-ci-robot
Copy link
Contributor

@oomichi: GitHub didn't allow me to request PR reviews from the following users: cristicalin.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @cristicalin

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/test-infra repository.

@cristicalin
Copy link
Contributor

I'm not a sig member yet so I can't do more than 👍 but it looks good to me.

@champtar
Copy link
Contributor

I'm not a sig member yet so I can't do more than 👍 but it looks good to me.

You just need 2 +1 to become a member and given your contributions you can count on mine no problem ;)

@oomichi
Copy link
Contributor Author

oomichi commented Oct 11, 2021

I'm not a sig member yet so I can't do more than 👍 but it looks good to me.

I am happy to support you for the membership if necessary ! :-)

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, oomichi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2021
@oreillymj
Copy link

Just wondering why the kube user was created? And also, do the cni configs require 755 permissions.

  • name: Prepare generic
    hosts: all
    become: true
    roles:
    • role: kubespray-defaults
    • role: bootstrap-os
    • role: adduser
      user: "{{ addusers.kube }}"
      tasks:
    • include_tasks: "../../../../download/tasks/download_file.yml"
      vars:
      download: "{{ download_defaults | combine(downloads.cni) }}"

@cristicalin
Copy link
Contributor

Just wondering why the kube user was created? And also, do the cni configs require 755 permissions.

The kube user is actually quite a pain and I think a legacy we carry around. The main issue is we never allocated a stable UID for it but it was required since some components refuse to access files owned by root and generally it was accepted as a best practice to not have root own or run certain stuff.

Regarding 0755 access patterns, it is there to mitigate situations where we have UID mismatch with things that are running in containers but with a different UID than kube user UID.

@jayonlau
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2021
@k8s-ci-robot k8s-ci-robot merged commit 19d07a4 into kubernetes-sigs:master Oct 20, 2021
@floryut floryut added kind/network Network section in the release note and removed kind/bug Categorizes issue or PR as related to a bug. labels Oct 20, 2021
@tboevil
Copy link

tboevil commented Oct 26, 2021

maybe missing a place

roles/kubernetes/preinstall/tasks/0050-create_directories.yml:74

@floryut floryut mentioned this pull request Dec 21, 2021
sakuraiyuta pushed a commit to sakuraiyuta/kubespray that referenced this pull request Apr 16, 2022
kube-bench scan outputs warning related to Calico like:

* text: "Ensure that the Container Network Interface file
  permissions are set to 644 or more restrictive (Manual)"
* text: "Ensure that the Container Network Interface file
  ownership is set to root:root (Manual)"

This fixes these warnings.
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Jun 27, 2023
kube-bench scan outputs warning related to Calico like:

* text: "Ensure that the Container Network Interface file
  permissions are set to 644 or more restrictive (Manual)"
* text: "Ensure that the Container Network Interface file
  ownership is set to root:root (Manual)"

This fixes these warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/network Network section in the release note lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kube-bench security warnings on controller deployed with kubespray 2.1.6+
8 participants