-
Notifications
You must be signed in to change notification settings - Fork 16
Add vulnerability management recommendations for maintainers #156
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
base: main
Are you sure you want to change the base?
Add vulnerability management recommendations for maintainers #156
Conversation
| - repository configurations - for example, a hotfix branch might not have the | ||
| proper branch protection rules, or the repository might not have the proper | ||
| security settings enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be mostly covered by org-wide config-as-code tooling by now but we don't cover custom branches beyond main there usually, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right.
This is more serving as a check list for the maintainers, while working on cleaning up the branch policies we have noticed several projects not protecting things correctly (couple of them were surprising).
| - package dependencies - for example, a package might have a known | ||
| vulnerability, or a package might be using a deprecated version of a | ||
| library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to put that one to the top of the list as it's the most frequent concern that's brought up.
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of | ||
| discovery. | ||
| - [ ] Medium and low severity vulnerabilities are patched within 4 weeks of | ||
| discovery. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it's good to document an SLA here, I think it should be defined more clearly rather than just in the checklist that's summarizing the above.
When this is done, we should likely point out that this is a best-effort guideline for the project itself but without any guarantees or legal implications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the following wording in the Collector SIG in case it helps:
We aim to provide a release that fixes security-related issues in at most 30 days since they are publicly announced; with the current release schedule this means security issues will typically not warrant a bugfix release.
Co-authored-by: Armin Ruech <[email protected]>
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of | ||
| discovery. | ||
| - [ ] Medium and low severity vulnerabilities are patched within 4 weeks of | ||
| discovery. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the following wording in the Collector SIG in case it helps:
We aim to provide a release that fixes security-related issues in at most 30 days since they are publicly announced; with the current release schedule this means security issues will typically not warrant a bugfix release.
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of | ||
| discovery. | ||
| - [ ] Medium and low severity vulnerabilities are patched within 4 weeks of | ||
| discovery. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are a bit too aggressive. You can see e.g. these ones from Gitlab: https://handbook.gitlab.com/handbook/security/product-security/vulnerability-management/sla/
where low and medium have a 90/180 day SLA. I think we should align to something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OpenSSF questionnaire says
There MUST be no unpatched vulnerabilities of medium or higher severity that have been publicly known for more than 60 days.
I would go with that maybe at least for the core repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OpenSSF questionnaire says
There MUST be no unpatched vulnerabilities of medium or higher severity that have been publicly known for more than 60 days.
I would go with that maybe at least for the core repos
I feel this is a low bar and we should put a higher one. Here is my thinking: OpenTelemetry components are widely used by other OSS components, if our bar is to get the medium or higher CVEs patched in 60 days, we are not giving sufficient time for our users (including other OSS software that depend on us) to meet their 60 days time. That's the reason why I put 4 weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a higher bar makes sense to some but not all OpenTelemetry components necessarily (e.g. I don't think a security vulnerability in, say, Weaver, is as problematic as one in opentelemetry-python: they have different use cases with different security implications and their usage is different). I am in favor of a stronger requirement in specific components, but since this is an universal standard I think we should put a lower bar here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Do you feel this is a good balance?
Core components: 30 days
Everything else: 60 days
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable to me
| - [ ] False positives are documented (e.g., by commenting on the security | ||
| advisory, by providing the dismissal reason to a code scanning alert) and | ||
| dismissed. | ||
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about wording
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of | |
| - [ ] Critical and high severity vulnerabilities (after rescoring if needed?) are patched within 2 weeks of |
| dismissed. | ||
| - [ ] Critical and high severity vulnerabilities are patched within 2 weeks of | ||
| discovery. | ||
| - [ ] Medium and low severity vulnerabilities are patched within 4 weeks of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about wording
| - [ ] Medium and low severity vulnerabilities are patched within 4 weeks of | |
| - [ ] Medium and low severity vulnerabilities (regardless of rescoring) are patched within 4 weeks of |
|
IMHO this looks good. |
| the worst case, this can be a potential end-of-life announcement of the | ||
| affected component or project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"or project" made me think catastrophic end of OpenTelemetry
| the worst case, this can be a potential end-of-life announcement of the | |
| affected component or project. | |
| the worst case, this can be a potential end-of-life announcement of the | |
| affected component. |
| - [ ] Daily scan CI/CD environment for deprecations and vulnerabilities. | ||
| - [ ] Daily scan container image dependencies for deprecations and | ||
| vulnerabilities. | ||
| - [ ] Daily scan repository configurations for deprecations and vulnerabilities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this something maintainers can do?
| - The maintainers of an OpenTelemetry project should establish a clear | ||
| accountability for security issues, including identifying the direct | ||
| responsible individual for security issues at a certain time, for | ||
| example, via a duty rotation. This should be | ||
| documented in the main README.md file of the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels a bit heavyweight for most of the repos to establish a DRI rotation where there are 2-3 maintainers
as long as the maintainers are meeting the bar we are setting for triage / fixing
if we want to keep this, maybe "... or some other process that ensures vulnerabilities are being triaged in a timely manner"
No description provided.