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

SECURITY_CONTACTS contact information #56

Open
tallclair opened this issue Nov 14, 2019 · 18 comments
Open

SECURITY_CONTACTS contact information #56

tallclair opened this issue Nov 14, 2019 · 18 comments
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@tallclair
Copy link
Member

SECURITY_CONTACTS files currently use github user names, but github doesn't have private messaging, and users don't always have public email addresses.

We need a better solution, so that the PSC is able to reach out to the security contacts through a private channel. A few ideas include:

  1. deprecate SECURITY_CONTACTS, and adopt github's new security advisory functionality
  2. maintain a private contact information database for security contacts (bleh)
  3. Just require email addresses in addition to github user IDs

/help

@k8s-ci-robot
Copy link
Contributor

@tallclair:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

SECURITY_CONTACTS files currently use github user names, but github doesn't have private messaging, and users don't always have public email addresses.

We need a better solution, so that the PSC is able to reach out to the security contacts through a private channel. A few ideas include:

  1. deprecate SECURITY_CONTACTS, and adopt github's new security advisory functionality
  2. maintain a private contact information database for security contacts (bleh)
  3. Just require email addresses in addition to github user IDs

/help

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 help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 14, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 12, 2020
@tallclair
Copy link
Member Author

/remove-lifecycle stale

Updated proposal:

  1. Migrate SECURITY_CONTACTS to SECURITY.md, with a templated security policy
  2. Security policy should include the security contacts, with both github user ID, email ID, and slack ID (email/slack optional, email strongly encouraged)

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 18, 2020
@tallclair
Copy link
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 18, 2020
@tallclair tallclair added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label May 18, 2020
@BenTheElder
Copy link
Member

+1 #56 (comment)
What blocks us from moving on this?

@tallclair
Copy link
Member Author

@BenTheElder Nothing (just someone to do the work), except for the concerns raised in the discussion thread about confusing the intent of security contacts.

xref: kubernetes/kubernetes-template-project#35

@BenTheElder
Copy link
Member

BenTheElder commented Jul 23, 2020 via email

@sfowl
Copy link
Contributor

sfowl commented Jan 12, 2021

@tallclair @joelsmith @BenTheElder

I circled back on this today and have created an example PR into my own fork of what the proposed changes could look like. I made a small script (replaced) which generates the content for the new security_contacts field, and also made a PR to update the OWNERS spec.

Some thoughts I had while making this:

  • Is it okay from a privacy perspective to add email addresses to OWNERS? I applied some minor obfuscation, but not sure if this is enough
  • Not 100% sure including all approvers by default in security_contacts is the right move, however not doing this leaves security_contacts pretty bare for some components
  • In future, we could perhaps add a slack field, but I left this out for now

Any/all feedback is very welcome. If this approach makes sense, then the next step would be to open PRs for all OWNERS in the kubernetes org. Given the size (and noise!) of that task I thought it best to start with an example for review. Thanks!

@tallclair
Copy link
Member Author

Thanks for picking this up Sam! I think we should not automatically copy the approvers into security contacts. I'd prefer it to be an explicit add, and we (PSC) can just escalate to the approvers if there aren't any security_contacts. Once the changes go in, I think it would be worth sending out a call to action to kubernetes-dev for maintainers to review their security_contacts.

re: emails - as much as it would be nice for us to have easy access to emails, I think it needs to be an explicit opt-in. We can encourage people to add an email in the call-to-action. Maybe it would be worth giving a slack option too?

@sfowl
Copy link
Contributor

sfowl commented Jan 13, 2021

/assign

@sfowl
Copy link
Contributor

sfowl commented Jan 13, 2021

Thanks for feedback @tallclair !

I've taken out the auto-inclusion of github profile email addresses and added a slack key. Values for all email and slack will be null initially, but I will leave them in by default to encourage their use.

I've updated the demo PR and spec update, and made a revised version of the script

@sfowl
Copy link
Contributor

sfowl commented Apr 22, 2021

Removed the slack field from the proposal due to slack usernames not being unique across the kubernetes.slack.com. More info in kubernetes/community/pull/5398

@BenTheElder
Copy link
Member

slack has user _id_s that are unique however, xref: kubernetes/community#2349

click on a user, click "view full profile", under the profile click "more ..." then "copy member ID"

our moderation reporting robot uses IDs.

@sfowl
Copy link
Contributor

sfowl commented Apr 23, 2021

@BenTheElder Right, that was mentioned in the kubernetes/community PR. My concern was that even if the spec declares that slack _id_s should be used, there doesn't seem to be a way to enforce that, i.e. users would likely still add their usernames, which adds the risk of mistaken identity.

The issue you've linked above is interesting. I've wondered if it's the wrong approach to be adding extra fields like email and slack only under security_contacts. Those extra fields are probably generally useful for other entries in OWNERS, not just security_contacts. Using a tool like the one proposed in kubernetes/community#2349 would also provide some contact infoimmediately, rather than relying on individual persons to add their own contact info to OWNERS at some point in the future.

I guess I'm saying that I like the idea simplifying security_contacts in OWNERs down to only github username, consistent with approvers, reviewers etc, and use a tool like the one proposed in kubernetes/community#2349 to retrieve email and slack ID. The Kube PSC could use such a tool when they need to reach out a security_contact for a security issue. There will be users who won't have contact info that's easily retrievable, but I think it's that likely those same users would be reluctant to add their contact info to an OWNERS file.

@tallclair wdyt?

@BenTheElder
Copy link
Member

My concern was that even if the spec declares that slack _id_s should be used, there doesn't seem to be a way to enforce that, i.e. users would likely still add their usernames, which adds the risk of mistaken identity.

Actually we already enforce other forms of validity in these files and the form of slack user IDs is well-known.

https://github.com/kubernetes/test-infra/blob/master/prow/plugins/verify-owners/verify-owners.go

but I think it's that likely those same users would be reluctant to add their contact info to an OWNERS file.

I don't think we should allow security contacts that don't provide a contact method. I also don't think kubernetes/community#2349 is going to happen, but it's relatively straight forward to require a user ID and we can validate the correct format.

@PushkarJ
Copy link
Member

If the intent is to have private space to communicate with SRC, would it be simpler to update the SECURITY_CONTACTS file with the following:

# Defined below are the security contacts for this repo.
#
# They are the contact point for the Product Security Committee to reach out
# to for triaging and handling of incoming issues.
#
# The below names agree to abide by the
# [Embargo Policy](https://git.k8s.io/security/private-distributors-list.md#embargo-policy)
# and will be removed and replaced if they violate that agreement.
#
# DO NOT REPORT SECURITY VULNERABILITIES DIRECTLY TO THESE NAMES, FOLLOW THE
# INSTRUCTIONS AT https://kubernetes.io/security/

mailing_list: <sub-project-name>-security@kubernetes.io
github_ids:
- bob
- alice

For all new sub-projects, we could add a step to create a private group like this before the repo is created.

For existing sub-projects, we can auto-create that group in bulk and open an automated PR to update their SECURITY_CONTACTS with this group and also ask sub-project maintainers, as part of that PR to add any relevant members (maintainers) to it.

@sfowl
Copy link
Contributor

sfowl commented Feb 21, 2022

The last round of PRs for this task languished due to my own neglect.

@tallclair suggested a more simple approach of:

  1. Add a security_contacts field to OWNERS with just the github handle, and migrate the existing SECURITY_CONTACTS files over.
  2. Adding in additional contact information

This does seem a more conservative approach, and hopefully less controversial.

To that end I created a new issue, as the comment history on the old issues is quite long and noisy:

#149

@PushkarJ I think your proposal has some merit and could fit in step 2. However, I believe the consolidation of OWNERS and SECURITY_CONTACTS is the higher priority part for the SRC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

6 participants