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

Add CVE Information to Release Notes #1441

Merged
merged 4 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ require (
github.com/sirupsen/logrus v1.7.0
github.com/spf13/cobra v1.1.1
github.com/spf13/pflag v1.0.5
github.com/spiegel-im-spiegel/go-cvss v0.4.0
github.com/stretchr/testify v1.7.0
github.com/yuin/goldmark v1.3.1
golang.org/x/net v0.0.0-20201224014010-6772e930b67b
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,10 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM
github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
github.com/spf13/viper v1.6.1/go.mod h1:t3iDnF5Jlj76alVNuyFBk5oUMCvsrkbvZK0WQdfDi5k=
github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg=
github.com/spiegel-im-spiegel/errs v1.0.2 h1:v4amEwRDqRWjKHOILQnJSovYhZ4ZttEnBBXNXEzS6Sc=
github.com/spiegel-im-spiegel/errs v1.0.2/go.mod h1:UoasJYYujMcdkbT9USv8dfZWoMyaY3btqQxoLJImw0A=
github.com/spiegel-im-spiegel/go-cvss v0.4.0 h1:A+S9I01wkiEo6e66xFbxtz9LfXdsGUr5FppYozUAmmI=
github.com/spiegel-im-spiegel/go-cvss v0.4.0/go.mod h1:VrcmtyfJ7OJF3/O08MQnxq+kqyPMjstExKdv02TPx+M=
github.com/src-d/gcfg v1.4.0 h1:xXbNR5AlLSA315x2UO+fTSSAXCDf+Ar38/6oyGbDKQ4=
github.com/src-d/gcfg v1.4.0/go.mod h1:p/UMsR43ujA89BJY9duynAwIpvqEujIH/jFlfL7jWoI=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
26 changes: 17 additions & 9 deletions pkg/notes/document/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,24 +231,32 @@ func New(
if val, ok := cvedata.(map[interface{}]interface{})["title"].(string); ok {
cve.Title = val
}
if val, ok := cvedata.(map[interface{}]interface{})["linkedPRs"].([]interface{}); ok {
cve.LinkedPRs = []int{}
for _, prid := range val {
cve.LinkedPRs = append(cve.LinkedPRs, prid.(int))
}
if val, ok := cvedata.(map[interface{}]interface{})["issue"].(string); ok {
cve.TrackingIssue = val
}
if val, ok := cvedata.(map[interface{}]interface{})["published"].(string); ok {
cve.Published = val
if val, ok := cvedata.(map[interface{}]interface{})["vector"].(string); ok {
cve.CVSSVector = val
}
if val, ok := cvedata.(map[interface{}]interface{})["score"].(float64); ok {
cve.Score = float32(val)
cve.CVSSScore = float32(val)
}
if val, ok := cvedata.(map[interface{}]interface{})["rating"].(string); ok {
cve.Rating = val
cve.CVSSRating = val
}
if val, ok := cvedata.(map[interface{}]interface{})["description"].(string); ok {
cve.Description = val
}
// Linked PRs is a list of the PR IDs
if val, ok := cvedata.(map[interface{}]interface{})["linkedPRs"].([]interface{}); ok {
cve.LinkedPRs = []int{}
for _, prid := range val {
cve.LinkedPRs = append(cve.LinkedPRs, prid.(int))
}
}
// Verify that CVE data has the minimum fields defined
if err := cve.Validate(); err != nil {
return nil, errors.Wrapf(err, "checking CVE map file for PR #%d", pr)
}
doc.CVEList = append(doc.CVEList, cve)
}

Expand Down
18 changes: 18 additions & 0 deletions pkg/notes/document/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@ filename | sha512 hash
{{- end -}}
# Changelog since {{$PreviousRevision}}

{{with .CVEList -}}
## Important Security Information

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

{{.Description}}

**CVSS Rating:** {{.CVSSRating}} ({{.CVSSScore}}) [{{.CVSSVector}}]({{.CalcLink}})
{{- if .TrackingIssue -}}
<br>
**Tracking Issue:** {{.TrackingIssue}}
{{- end }}

{{ end }}
{{- end -}}

{{with .NotesWithActionRequired -}}
## Urgent Upgrade Notes

Expand Down
78 changes: 71 additions & 7 deletions pkg/notes/notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/nozzle/throttler"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
cvss "github.com/spiegel-im-spiegel/go-cvss/v3/metric"
"gopkg.in/yaml.v2"

"k8s.io/release/pkg/github"
Expand All @@ -51,6 +52,9 @@ const (
// maxParallelRequests is the maximum parallel requests we shall make to the
// GitHub API
maxParallelRequests = 10

// Regexp to check CVE IDs
cveIDRegExp = `^CVE-\d{4}-\d+$`
)

type (
Expand All @@ -60,13 +64,15 @@ type (

// CVEData Information of a linked CVE vulnerability
type CVEData struct {
ID string `json:"id"`
Title string `json:"title"`
Published string `json:"published"`
Score float32 `json:"score"`
Rating string `json:"rating"`
LinkedPRs []int `json:"linkedPRs"`
Description string `json:"description"`
ID string `json:"id"` // CVE ID, eg CVE-2019-1010260
Title string `json:"title"` // Title of the vulnerability
Description string `json:"description"` // Description text of the vulnerability
TrackingIssue string `json:"issue"` // Link to the vulnerability tracking issue (url, optional)
CVSSVector string `json:"vector"` // Full CVSS vector string, CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H
CVSSScore float32 `json:"score"` // Numeric CVSS score (eg 6.2)
CVSSRating string `json:"rating"` // Severity bucket (eg Medium)
CalcLink string // Link to the CVE calculator (automatic)
LinkedPRs []int `json:"pullrequests"` // List of linked PRs (to remove them from the release notes doc)
}

const (
Expand Down Expand Up @@ -1085,3 +1091,61 @@ func (rn *ReleaseNote) ContentHash() (string, error) {
}
return fmt.Sprintf("%x", h.Sum(nil)), nil
}

// Validate checks the data defined in a CVE map is complete and valid
func (cve *CVEData) Validate() error {
// Verify that rating is defined and a known string
if cve.CVSSRating == "" {
return errors.New("CVSS rating missing from CVE data")
}

// Check rating is a valid string
if _, ok := map[string]bool{
"None": true, "Low": true, "Medium": true, "High": true, "Critical": true,
}[cve.CVSSRating]; !ok {
return errors.New("Invalid CVSS rating")
}

// Check vector string is not empty
if cve.CVSSVector == "" {
return errors.New("CVSS vector string missing from CVE data")
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

}

// Parse the vector string to make sure it is well formed
bm, err := cvss.NewBase().Decode(cve.CVSSVector)
if err != nil {
return errors.Wrap(err, "parsing CVSS vector string")
}
cve.CalcLink = fmt.Sprintf(
"https://www.first.org/cvss/calculator/%s#%s", bm.Ver.String(), cve.CVSSVector,
)

if cve.CVSSScore == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

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

return errors.New("CVSS score missing from CVE data")
}
if cve.CVSSScore < 0 || cve.CVSSScore > 10 {
return errors.New("CVSS score pit of range, should be 0-10")
}

// Check that the CVE ID is not empty
if cve.ID == "" {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

✔️ Added a basic regexp validation

return errors.New("ID missing from CVE data")
}

// Verify that the CVE ID is well formed
cvsre := regexp.MustCompile(cveIDRegExp)
if !cvsre.MatchString(cve.ID) {
return errors.New("CVS ID is not well formed")
}

// Title and description must not be empty
if cve.Title == "" {
return errors.New("Title missing from CVE data")
}

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

return nil
}
71 changes: 71 additions & 0 deletions pkg/notes/notes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,74 @@ func TestApplyMap(t *testing.T) {
}
}
}

func TestCVEDataValidation(t *testing.T) {
var sut CVEData
cve := CVEData{
ID: "CVE-2020-8559",
Title: "Privilege escalation from compromised node to cluster",
Description: "If an attacker is able to intercept certain requests to the Kubelet, they",
TrackingIssue: "https://github.com/kubernetes/kubernetes/issues/92914",
CVSSVector: "CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H",
CVSSScore: 6.4,
CVSSRating: "Medium",
LinkedPRs: []int{92941, 92969, 92970, 92971},
}

// As is, the CVE should validate
require.Nil(t, cve.Validate())

// Check each value
sut = cve
sut.ID = "CVE-123"
require.NotNil(t, sut.Validate(), "checking cve id")

sut = cve
sut.Title = ""
require.NotNil(t, sut.Validate(), "checking title")

sut = cve
sut.Description = ""
require.NotNil(t, sut.Validate(), "checking description")

sut = cve
for _, testVector := range []string{
"CVSS:3.1/AV:N/AC:H/P", // too short
"", // empty
"CVSS:3.1/AV:N/AC:Á/PR:H/UI:R/S:U/C:H/I:H/A:H", // invalid value
} {
sut.CVSSVector = testVector // too short
require.NotNil(t, sut.Validate(), "checking vector string")
}

sut = cve
for _, testScore := range []float32{
0, // cannot be 0 (nil value)
-1, // under
10.1, // over
} {
sut.CVSSScore = testScore
require.NotNil(t, sut.Validate(), "checking vector string")
}

sut = cve
for _, tc := range []struct {
Valid bool
Value string
}{
{true, "None"},
{true, "Low"},
{true, "Medium"},
{true, "High"},
{true, "Critical"},
{false, ""},
{false, "Superbad"},
} {
sut.CVSSRating = tc.Value // too short
if tc.Valid {
require.Nil(t, sut.Validate(), "checking valid rating string")
} else {
require.NotNil(t, sut.Validate(), "checking invalid rating string")
}
}
}