Skip to content

Ensure that states are set correctly based on remaining vuln reqs #388

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.

Sign up for GitHub

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

Merged
md2119 merged 4 commits into from
Jan 20, 2022
Merged

Ensure that states are set correctly based on remaining vuln reqs #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f3aa3a
Ensure that states are set correctly based on remaining vuln reqs
md2119 Jan 20, 2022
75b574f
gen
md2119 Jan 20, 2022
011e55a
do the same on snooze
md2119 Jan 20, 2022
04fd860
graphql
md2119 Jan 20, 2022
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
2 changes: 1 addition & 1 deletion central/graphql/resolvers/vulnerabilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func init() {
"suppressExpiry: Time",
"activeState(query: String): ActiveState",
"vulnerabilityState: String!",
"effectiveVulnerabilityRequest: VulnerabilityRequest!",
"effectiveVulnerabilityRequest: VulnerabilityRequest",
Copy link
Contributor Author

@md2119 md2119 Jan 20, 2022

Choose a reason for hiding this comment

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

have to do this since the earlier #384 removes the restriction of non-nil response

}),
schema.AddQuery("vulnerability(id: ID): EmbeddedVulnerability"),
schema.AddQuery("vulnerabilities(query: String, scopeQuery: String, pagination: Pagination): [EmbeddedVulnerability!]!"),
Expand Down
2 changes: 2 additions & 0 deletions central/vulnerabilityrequest/cache/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ type VulnReqCache interface {
Remove(requestID string) bool
RemoveMany(requestIDs ...string) bool
GetVulnsWithState(registry, remote, tag string) map[string]storage.VulnerabilityState
// GetEffectiveVulnStateForImage returns the effective state of the vulnerabilities within the given image.
GetEffectiveVulnStateForImage(cves []string, registry, remote, tag string) map[string]storage.VulnerabilityState
// GetEffectiveVulnState returns the effective state of the vulnerabilities within the given requests.
GetEffectiveVulnState(cves []string, requestIDs ...string) map[string]storage.VulnerabilityState
// GetEffectiveVulnReqID returns the vuln request in effect on given image+cve combination.
Expand Down
30 changes: 30 additions & 0 deletions central/vulnerabilityrequest/cache/cache_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,36 @@ func (c *vulnReqCacheImpl) GetEffectiveVulnState(cves []string, requestIDs ...st
return ret
}

func (c *vulnReqCacheImpl) GetEffectiveVulnStateForImage(cves []string, registry, remote, tag string) map[string]storage.VulnerabilityState {
if registry == "" || remote == "" || tag == "" {
return nil
}
ret := make(map[string]storage.VulnerabilityState)
for _, cve := range cves {
ret[cve] = storage.VulnerabilityState_OBSERVED
}
cvesSet := set.NewStringSet(cves...)

c.lock.RLock()
defer c.lock.RUnlock()

for _, scope := range []string{
// Start with the smallest scope because req from the smallest scope takes precedence.
imageNameToScopeStr(registry, remote, tag),
imageNameToScopeStr(registry, remote, common.MatchAll),
common.MatchAll,
} {
reqMap := c.vulnReqByScope[scope]
if reqMap == nil {
continue
}
for _, req := range reqMap {
processSlimRequest(req, scope, cvesSet, ret)
}
}
return ret
}

func processSlimRequest(slimReq *slimRequest, scope string, cveSet set.StringSet, result map[string]storage.VulnerabilityState) {
if slimReq == nil {
return
Expand Down
50 changes: 49 additions & 1 deletion central/vulnerabilityrequest/cache/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ func TestCache(t *testing.T) {
req3.Id = "req3"
req4 := fixtures.GetImageScopeDeferralRequest(registry, remote, tag, vuln3)
req4.Id = "req4"
req5 := fixtures.GetImageScopeDeferralRequest("reg-2", "fake", tag, vuln3)
req5.Id = "req5"
req6 := fixtures.GetImageScopeFPRequest(registry, remote, ".*", vuln3)
req5.Id = "req6"

cache := New()
for _, req := range []*storage.VulnerabilityRequest{req1, req2, req3, req4} {
for _, req := range []*storage.VulnerabilityRequest{req1, req2, req3, req4, req5, req6} {
assert.True(t, cache.Add(req))
}

Expand Down Expand Up @@ -64,4 +68,48 @@ func TestCache(t *testing.T) {
// Test that the vulnerability state is scoped to global.
response = cache.GetEffectiveVulnState([]string{vuln1})
assert.Equal(t, storage.VulnerabilityState_FALSE_POSITIVE, response[vuln1])

// Remove one of the request for vuln3, and verify that for removed scope the cve state is taken from req6 which
// applies to all tags.
cache.Remove(req4.GetId())
response = cache.GetEffectiveVulnStateForImage(
[]string{vuln3},
req4.GetScope().GetImageScope().GetRegistry(),
req4.GetScope().GetImageScope().GetRemote(),
req4.GetScope().GetImageScope().GetTag())
assert.Equal(t, storage.VulnerabilityState_FALSE_POSITIVE, response[vuln3])

// Verify that for req5 scope cve is still deferred since the request still exists.
response = cache.GetEffectiveVulnStateForImage(
[]string{vuln3},
req5.GetScope().GetImageScope().GetRegistry(),
req5.GetScope().GetImageScope().GetRemote(),
req5.GetScope().GetImageScope().GetTag())
assert.Equal(t, storage.VulnerabilityState_DEFERRED, response[vuln3])

// Verify that for req4 scope the cve is observed since the all tags request is removed.
cache.Remove(req6.GetId())
response = cache.GetEffectiveVulnStateForImage(
[]string{vuln3},
req4.GetScope().GetImageScope().GetRegistry(),
req4.GetScope().GetImageScope().GetRemote(),
req4.GetScope().GetImageScope().GetTag())
assert.Equal(t, storage.VulnerabilityState_OBSERVED, response[vuln3])

// Verify that for req5 scope, cves are still in deferred since the request still exists.
response = cache.GetEffectiveVulnStateForImage(
[]string{vuln3},
req5.GetScope().GetImageScope().GetRegistry(),
req5.GetScope().GetImageScope().GetRemote(),
req5.GetScope().GetImageScope().GetTag())
assert.Equal(t, storage.VulnerabilityState_DEFERRED, response[vuln3])

// Verify that for req5 scope, cves are in observed since no request exists.
cache.Remove(req5.GetId())
response = cache.GetEffectiveVulnStateForImage(
[]string{vuln3},
req5.GetScope().GetImageScope().GetRegistry(),
req5.GetScope().GetImageScope().GetRemote(),
req5.GetScope().GetImageScope().GetTag())
assert.Equal(t, storage.VulnerabilityState_OBSERVED, response[vuln3])
}
14 changes: 14 additions & 0 deletions central/vulnerabilityrequest/cache/mocks/cache.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 28 additions & 31 deletions central/vulnerabilityrequest/manager/manager_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,6 @@ func (m *managerImpl) SnoozeVulnerabilityOnRequest(_ context.Context, request *s
m.pendingReqCache.Remove(request.GetId())
m.activeReqCache.Add(request)

// Get all the requests still active on the cves.
vulnReqIDs, err := m.getActiveVulnReqsForCVEs(request.GetCves().GetIds()...)
if err != nil {
return errors.Wrap(err, "could not search for vulnerability requests")
}
cveStateMap := m.activeReqCache.GetEffectiveVulnState(request.GetCves().GetIds(), vulnReqIDs...)

// Search for images matching the scope instead of image+cve combination.
// Validation of image-cve existence is performed by the image-cve datastore.
imageIDs, err := m.getImagesIDsForVulnRequest(request)
Expand All @@ -108,9 +101,20 @@ func (m *managerImpl) SnoozeVulnerabilityOnRequest(_ context.Context, request *s
return nil
}

for _, cve := range request.GetCves().GetIds() {
if err := m.imageCVEEdges.UpdateVulnerabilityState(allAccessCtx, cve, imageIDs, cveStateMap[cve]); err != nil {
return errors.Wrapf(err, "could not snooze vulnerabilities for request %s", request.GetId())
for _, imageID := range imageIDs {
img, found, err := m.images.GetImageMetadata(allAccessCtx, imageID)
if err != nil {
return errors.Wrapf(err, "could not un-snooze vulnerabilities for request %s", request.GetId())
}
if !found {
continue
}
// Determine the effective for the cves in the image scope.
cveStateMap := m.activeReqCache.GetEffectiveVulnStateForImage(request.GetCves().GetIds(), img.GetName().GetRegistry(), img.GetName().GetRemote(), img.GetName().GetTag())
for _, cve := range request.GetCves().GetIds() {
if err := m.imageCVEEdges.UpdateVulnerabilityState(allAccessCtx, cve, []string{imageID}, cveStateMap[cve]); err != nil {
return errors.Wrapf(err, "could not un-snooze vulnerabilities for request %s", request.GetId())
}
}
}

Expand All @@ -123,13 +127,6 @@ func (m *managerImpl) SnoozeVulnerabilityOnRequest(_ context.Context, request *s
func (m *managerImpl) UnSnoozeVulnerabilityOnRequest(_ context.Context, request *storage.VulnerabilityRequest) error {
m.activeReqCache.Remove(request.GetId())

// Get all the requests still active on the cves.
vulnReqIDs, err := m.getActiveVulnReqsForCVEs(request.GetCves().GetIds()...)
if err != nil {
return errors.Wrap(err, "could not search for vulnerability requests")
}
cveStateMap := m.activeReqCache.GetEffectiveVulnState(request.GetCves().GetIds(), vulnReqIDs...)

// Search for images matching the scope instead of image+cve combination.
// Validation of image-cve existence is performed by the image-cve datastore.
imageIDs, err := m.getImagesIDsForVulnRequest(request)
Expand All @@ -139,10 +136,22 @@ func (m *managerImpl) UnSnoozeVulnerabilityOnRequest(_ context.Context, request
if len(imageIDs) == 0 {
return nil
}
for _, cve := range request.GetCves().GetIds() {
if err := m.imageCVEEdges.UpdateVulnerabilityState(allAccessCtx, cve, imageIDs, cveStateMap[cve]); err != nil {

for _, imageID := range imageIDs {
img, found, err := m.images.GetImageMetadata(allAccessCtx, imageID)
if err != nil {
return errors.Wrapf(err, "could not un-snooze vulnerabilities for request %s", request.GetId())
}
if !found {
continue
}
// Determine the effective for the cves in the image scope.
cveStateMap := m.activeReqCache.GetEffectiveVulnStateForImage(request.GetCves().GetIds(), img.GetName().GetRegistry(), img.GetName().GetRemote(), img.GetName().GetTag())
for _, cve := range request.GetCves().GetIds() {
if err := m.imageCVEEdges.UpdateVulnerabilityState(allAccessCtx, cve, []string{imageID}, cveStateMap[cve]); err != nil {
return errors.Wrapf(err, "could not un-snooze vulnerabilities for request %s", request.GetId())
}
}
}

go m.reprocessAffectedEntities(request.GetId(), imageIDs...)
Expand Down Expand Up @@ -299,18 +308,6 @@ func (m *managerImpl) reprocessImage(requestID string, affectedImages ...string)
return nil
}

func (m *managerImpl) getActiveVulnReqsForCVEs(cves ...string) ([]string, error) {
// Get all the requests still active on the cves.
results, err := m.vulnReqs.Search(allAccessCtx,
search.NewQueryBuilder().
AddExactMatches(search.CVE, cves...).
AddBools(search.ExpiredRequest, false).ProtoQuery())
if err != nil {
return nil, err
}
return search.ResultsToIDs(results), nil
}

func (m *managerImpl) getAffectedDeployments(affectedImages ...string) (map[string][]string, error) {
query := search.ConjunctionQuery(
search.NewQueryBuilder().AddExactMatches(search.ImageSHA, affectedImages...).ProtoQuery(),
Expand Down