Add CVE Information to Release Notes #1441

puerco · 2020-07-27T03:47:44Z

What type of PR is this?

/kind design

What this PR does / why we need it:

This PR introduces a minor change to the release notes template to add a security section with CVE data read from a map file (see #1373).

This is a WIP intended to discuss the way the information will look like when presented in the release notes document.

When applied, this test will add a new section to the release notes markdown output with information added from a CVE map. A sample of the output can be viewed here.

Tests are not included, they will be written and added to this PR once we settle on a design.

Which issue(s) this PR fixes:

Related to #1373 and kubernetes/enhancements#1833
Closes #1354

Special notes for your reviewer:

/hold for final design, tests and ~~until #1373 merges~~

Does this PR introduce a user-facing change?

* Release notes libraries will now recognize CVE information in the `datafields` section of the release notes map files.
* Add CVE vulnerability info to template.go to be rendered when a map defines a `cve` section

justaugustus · 2020-10-20T14:39:39Z

/retest

justaugustus · 2020-10-20T14:44:25Z

@kubernetes/release-engineering -- please review!

saschagrunert

/lgtm

cpanato

/lgtm

xmudrii

/lgtm

tpepper · 2020-10-20T16:01:49Z

/lgtm

justaugustus · 2020-10-20T16:06:32Z

@kubernetes/product-security-committee for review as well

tallclair · 2020-11-02T18:48:16Z

pkg/notes/document/template.go

+
+{{.Description}}
+
+__Rating:__ {{.Rating}} — __Score:__ {{.Score}} — __Published:__ {{.Published}}


What are Rating and Score? Is this referring to the severity bucket, and the numerical severity rating?
If so, we usually format those like:

Severity: Medium (CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H)

If you don't want to include the full CVSS rating, you could just put the numeric score in parentheses.

OK, I've modified the template to report following your suggestion. Added a new field for the vector too.

tallclair · 2020-11-02T18:50:08Z

pkg/notes/document/template.go

+
+__Rating:__ {{.Rating}} — __Score:__ {{.Score}} — __Published:__ {{.Published}}
+
+{{if .LinkedPRs -}}


I would prefer to just link to the tracking issue (e.g. kubernetes/kubernetes#92914) rather than the specific PRs. The tracking issue should have all the detail needed, and is generally what we consider the source of truth for vulnerability details.

👍 I've dropped the PRs from the output in favor of a new link to the tracking issue

tallclair · 2020-11-02T18:51:25Z

pkg/notes/document/template.go

+
+This release contains changes that address the following vulnerabilities:
+{{range .}}
+### {{.Title}}


I would like the CVE number to be included in the title, e.g. CVE-2020-8559: Privilege escalation from compromised node to cluster

I'm not sure if we just want to make it convention to set the title like that, or explicitly break the CVE into a separate field.

saschagrunert · 2021-01-12T09:42:11Z

@puerco are you still working on this or do you need help to carry it over the finishing line?

puerco · 2021-02-05T19:12:55Z

Are you planning to work on this or should we move the topic back into the open discussion?

Yes I'll retake this now as my secondary work.

puerco · 2021-02-22T21:14:14Z

OK, Ive modified the CVE data structure and the output format to better reflect @tallclair 's suggestions. Here's a sample of the output. And the corresponding map to generate:

---
pr: 97407
datafields:
  cve:
    id: CVE-2020-8559
    vector: CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H
    rating: Medium
    score: 6.4
    title: Privilege escalation from compromised node to cluster
    issue: https://github.com/kubernetes/kubernetes/issues/92914
    issues:
      - 92941
      - 92969
      - 92970
      - 92971
    description: >
      If an attacker is able to intercept certain requests to the Kubelet, they
      can send a redirect response that may be followed by a client using the
      credentials from the original request. This can lead to compromise of
      other nodes.
      
      If multiple clusters share the same certificate authority trusted by the
      client, and the same authentication credentials, this vulnerability may
      allow an attacker to redirect the client to another cluster. In this
      configuration, this vulnerability should be considered High severity.

This PR adds the capability to read CVE data from a map and uses it when rendering the release notes document. It incoroporates tallclair suggestions. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

tpepper · 2021-02-23T16:53:37Z

pkg/notes/notes.go

+		cve.CVSSRating != "Medium" &&
+		cve.CVSSRating != "High" &&
+		cve.CVSSRating != "Critical" {
+		return errors.New("Invalida CVSS rating")


Typo -> Invalid

✔️ Thanks!

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

justaugustus · 2021-02-23T17:01:51Z

cc: @kubernetes/sig-security

tpepper

Looks like good start, but suggest stricter validation. Since this is a critical bit of text in the notes, it is important to rigorously enforce the data is well formatted for the readers.

tpepper · 2021-02-23T16:58:47Z

pkg/notes/notes.go

+	if cve.Description == "" {
+		return errors.New("CVE description missing from CVE data")
+	}
+


I can see the new TrackingIssue perhaps being empty, but can't tell if that's intended. A couple changes to consider depending on if it is required or not

in notes.go CVEData comment the TrackingIssue as "Optional link to the vulnerability tracking issues"

add a validator here to require it to be non-empty (or set it to "n/a"?)

conditionally print the "Tracking Issue: {{.TrackingIssue}}" portion of the template only if non-empty

I've modified it to make the field optional. Once it is in use we can decide if we enforce it.

tpepper · 2021-02-23T17:04:23Z

pkg/notes/notes.go

+		return errors.New("CVSS score missing from CVE data")
+	}
+
+	if cve.ID == "" {


Should this validate more that the id is of the expected form for a CVE?

✔️ Added a basic regexp validation

tpepper · 2021-02-23T17:04:57Z

pkg/notes/notes.go

+		return errors.New("CVSS vector string missing from CVE data")
+	}
+
+	if cve.CVSSScore == 0 {


Should validate versus the known range of 0..10.

tpepper · 2021-02-23T17:06:55Z

pkg/notes/notes.go

+	}
+
+	if cve.CVSSVector == "" {
+		return errors.New("CVSS vector string missing from CVE data")


Is there a library that can validate the form is the expected CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H sort of form?

I'm trying this one and seems to do the job just fine: https://github.com/spiegel-im-spiegel/go-cvss

puerco · 2021-02-23T19:44:47Z

I've addressed @tpepper 's remarks and added a small test for the CVE data validation function.

puerco · 2021-02-23T21:40:48Z

/test pull-release-verify

…ang 1.16) Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

tpepper · 2021-02-25T21:15:43Z

Update lgtm.

saschagrunert

/lgtm

k8s-ci-robot · 2021-03-01T10:03:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, puerco, saschagrunert, xmudrii

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:

~~OWNERS~~ [cpanato,puerco,saschagrunert,xmudrii]

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

saschagrunert · 2021-03-01T10:03:11Z

/hold cancel

k8s-ci-robot requested review from jeefy and saschagrunert July 27, 2020 03:47

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jul 27, 2020

puerco mentioned this pull request Jul 27, 2020

Release notes mappings libraries and in krel release-notes #1373

Merged

puerco mentioned this pull request Aug 4, 2020

Adding security details to release notes #1354

Closed

tallclair self-assigned this Oct 19, 2020

saschagrunert approved these changes Oct 20, 2020

View reviewed changes

k8s-ci-robot assigned saschagrunert Oct 20, 2020

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 20, 2020

cpanato approved these changes Oct 20, 2020

View reviewed changes

k8s-ci-robot assigned cpanato Oct 20, 2020

xmudrii approved these changes Oct 20, 2020

View reviewed changes

k8s-ci-robot assigned xmudrii Oct 20, 2020

k8s-ci-robot assigned tpepper Oct 20, 2020

tallclair reviewed Nov 2, 2020

View reviewed changes

saschagrunert mentioned this pull request Jan 12, 2021

Update documentation around the security release process kubernetes/sig-release#896

Closed

3 tasks

puerco force-pushed the cve-template branch from 43976ac to de1e513 Compare February 22, 2021 21:10

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 22, 2021

puerco added 2 commits February 22, 2021 18:21

[WIP] Add CVE Information to template.go

d3fa4b7

This PR adds the capability to read CVE data from a map and uses it when rendering the release notes document. It incoroporates tallclair suggestions. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

Add CVEData verification function and check data is complete

15d64a3

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

puerco force-pushed the cve-template branch from de1e513 to 4db5687 Compare February 23, 2021 00:47

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 23, 2021

puerco changed the title ~~[WIP] Add CVE Information to template.go~~ Add CVE Information to Release Notes Feb 23, 2021

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 Feb 23, 2021

tpepper reviewed Feb 23, 2021

View reviewed changes

Add link to CVE calculator on release notes CVE data

2163900

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

puerco force-pushed the cve-template branch from 4db5687 to 2163900 Compare February 23, 2021 16:56

tpepper suggested changes Feb 23, 2021

View reviewed changes

Add additional validation for CVE data and test (go mod tidy with gol…

181a262

…ang 1.16) Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

puerco force-pushed the cve-template branch from f1132f4 to 181a262 Compare February 23, 2021 23:02

saschagrunert approved these changes Mar 1, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 1, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2021

k8s-ci-robot merged commit d22c94c into kubernetes:master Mar 1, 2021

k8s-ci-robot added this to the v1.21 milestone Mar 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CVE Information to Release Notes #1441

Add CVE Information to Release Notes #1441

puerco commented Jul 27, 2020 •

edited

Loading

justaugustus commented Oct 20, 2020

justaugustus commented Oct 20, 2020

saschagrunert left a comment

cpanato left a comment

xmudrii left a comment

tpepper commented Oct 20, 2020

justaugustus commented Oct 20, 2020

tallclair Nov 2, 2020

puerco Feb 22, 2021

tallclair Nov 2, 2020

puerco Feb 22, 2021

tallclair Nov 2, 2020

puerco Feb 22, 2021

saschagrunert commented Jan 12, 2021

puerco commented Feb 5, 2021 •

edited

Loading

puerco commented Feb 22, 2021

tpepper Feb 23, 2021

puerco Feb 23, 2021

justaugustus commented Feb 23, 2021

tpepper left a comment

tpepper Feb 23, 2021

puerco Feb 23, 2021

tpepper Feb 23, 2021

puerco Feb 23, 2021

tpepper Feb 23, 2021

tpepper Feb 23, 2021

puerco Feb 23, 2021

puerco commented Feb 23, 2021

puerco commented Feb 23, 2021

tpepper commented Feb 25, 2021

saschagrunert left a comment

k8s-ci-robot commented Mar 1, 2021

saschagrunert commented Mar 1, 2021


		{{.Description}}

		__Rating:__ {{.Rating}} — __Score:__ {{.Score}} — __Published:__ {{.Published}}


		__Rating:__ {{.Rating}} — __Score:__ {{.Score}} — __Published:__ {{.Published}}

		{{if .LinkedPRs -}}

Add CVE Information to Release Notes #1441

Add CVE Information to Release Notes #1441

Conversation

puerco commented Jul 27, 2020 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

justaugustus commented Oct 20, 2020

justaugustus commented Oct 20, 2020

saschagrunert left a comment

Choose a reason for hiding this comment

cpanato left a comment

Choose a reason for hiding this comment

xmudrii left a comment

Choose a reason for hiding this comment

tpepper commented Oct 20, 2020

justaugustus commented Oct 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saschagrunert commented Jan 12, 2021

puerco commented Feb 5, 2021 • edited Loading

puerco commented Feb 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justaugustus commented Feb 23, 2021

tpepper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puerco commented Feb 23, 2021

puerco commented Feb 23, 2021

tpepper commented Feb 25, 2021

saschagrunert left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 1, 2021

saschagrunert commented Mar 1, 2021

puerco commented Jul 27, 2020 •

edited

Loading

puerco commented Feb 5, 2021 •

edited

Loading