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

Cluster API Security Self Assessment #40

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

PushkarJ
Copy link
Member

@PushkarJ PushkarJ commented Mar 2, 2022

Description

SIG Cluster Lifecycle and SIG Security worked together over several months, inspired from https://github.com/cncf/tag-security self-assessment process to write this self-assessment that identifies feature and documentation that needs work to improve the security posture of the sub-project.

Note

This is the first of it's kind effort at CNCF where a sub-project of a graduated project was assessed via collaboration between Security experts and maintainers of this sub-project. Past self-assessments via CNCF TAG Security have been focussed on graduating projects.

If you would like to request self-assessment for your sub-project, please create an issue by clicking on this link

Fixes #8

xref cncf/tag-security#603 and kubernetes-sigs/cluster-api#4446

Project Tracker

All the issues that are an outcomes of the self-assessment are tracked here: https://github.com/orgs/kubernetes/projects/83/views/1

/sig security cluster-lifecycle
/area security
/kind feature
/cc @sbueringer @Ankitasw @rficcaglia @randomvariable
/hold for #48

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 2, 2022
@k8s-ci-robot k8s-ci-robot requested a review from sbueringer March 2, 2022 20:29
@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/security Categorizes an issue or PR as relevant to SIG Security. labels Mar 2, 2022
@k8s-ci-robot k8s-ci-robot requested a review from rficcaglia March 2, 2022 20:29
@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Mar 2, 2022
@k8s-ci-robot
Copy link
Contributor

@PushkarJ: The label(s) area/security cannot be applied, because the repository doesn't have them.

In response to this:

Fixes #8

Note for Reviewers
Biggest help needed when reviewing is to look at "status" for each threats and update it to one of the available options as appropriate, if it's not already complete.

Note: Please tag any other relevant Cluster Lifecycle SIG contributors for review

/sig security cluster-lifecycle
/area security
/kind feature
/cc @sbueringer @Ankitasw @rficcaglia @randomvariable
/hold for kubernetes/community#6512

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.

@k8s-ci-robot
Copy link
Contributor

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

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

In response to this:

Fixes #8

Note for Reviewers
Biggest help needed when reviewing is to look at "status" for each threats and update it to one of the available options as appropriate, if it's not already complete.

Note: Please tag any other relevant Cluster Lifecycle SIG contributors for review

/sig security cluster-lifecycle
/area security
/kind feature
/cc @sbueringer @Ankitasw @rficcaglia @randomvariable
/hold for kubernetes/community#6512

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.

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 2, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 2, 2022
@sbueringer
Copy link
Member

I'll try to find some time to go over the doc.

(/cc @fabriziopandini fyi)

@neolit123
Copy link
Member

Biggest help needed when reviewing is to look at "status" for each threats

i already provided feedback on some of these while there were in google docs form, i believe.
but more than interested to see what the CAPI maintainers think.

Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

@PushkarJ this is great work! thank you!
If I can give a small suggestion is to improve a little bit the report/accompany it with a google sheet or something where it will be easier to have a quick glance at the list of things to do.
Expanding this a little bit, ideally required actions should be prioritized, so we can start planning how to improve CAPI security posture incrementally.

sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
Copy link
Member Author

@PushkarJ PushkarJ 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 again for the exhaustive feedback Stefan, Fabrizio, Lubomir !! Tried to address most comments. Will update the doc soon with relevant changes where needed

sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
sig-security-assessments/cluster-api/self-assessment.md Outdated Show resolved Hide resolved
@PushkarJ
Copy link
Member Author

Hi @sbueringer @fabriziopandini 👋🏼

Just did another walkthrough on the comments and resolved a few with updates.

Next step: Out of total 58 recommendations, we need to still update status of about 31 of them. Best way to find those 31 would be to:

  1. Go here: https://github.com/kubernetes/sig-security/pull/40/files#diff-67678031cfcd98b988ace5dcdbed2b46cec1ed1c68ca2bf48a9b6906662e76ab
  2. Click "Load diff" for self-assessment.md file
  3. Search for "Status: implemented / planned / to be implemented / End User Guidance" without double quotes

Once you find them please update select one of the options from:

  1. implemented: Please link to a merged PR that implements this
  2. planned: Please link to an open issue that implements this
  3. to be implemented: Please link to an unmerged PR that implements this
  4. End User guidance: Please remove other options in front of status and replace them with [End user guidance to be written](#cluster-api-end-user-guide)

Let me know if doing this together in a working session would be helpful and I would be happy to set it up!!

Add reviewers name and handles

Updates to resolve review feedback

Further updates to status of recommended mitigations
@PushkarJ PushkarJ force-pushed the add-cluster-api-sec-assessment branch from 552493a to eed0b78 Compare May 13, 2022 21:49
@PushkarJ PushkarJ changed the title Initial Draft for Cluster API Security Assessment Cluster API Security Self Assessment May 13, 2022
@PushkarJ PushkarJ marked this pull request as ready for review May 13, 2022 21:51
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2022
@PushkarJ
Copy link
Member Author

We are now ready for review. Let me know if I missed linking a tracking issue to any of the threats in the doc.

@PushkarJ
Copy link
Member Author

/assign aladewberry

(For review / feedback and overall structure as new lead of security self-assessments)

@aladewberry
Copy link
Contributor

@PushkarJ Ack! WIll try and get this read through before I'm off next week.

@aladewberry
Copy link
Contributor

/lgtm

I like the layout with the description of the project, personas, main flows, and then the security examination. I imagine we'll iterate incrementally as we continue assessing other projects, but this seems like an excellent place to start.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 14, 2022
@PushkarJ
Copy link
Member Author

/label tide/merge-method-squash
/remove-hold

@k8s-ci-robot k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 14, 2022
@aladewberry
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aladewberry, PushkarJ

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 Jul 14, 2022
@aladewberry
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 Jul 14, 2022
@k8s-ci-robot k8s-ci-robot merged commit 1e6f14d into kubernetes:main Jul 14, 2022
@aladewberry
Copy link
Contributor

/approve

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/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/security Categorizes an issue or PR as relevant to SIG Security. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cluster-API Security Self-Assessment Initiative
6 participants