-
Notifications
You must be signed in to change notification settings - Fork 12
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 Istio request handler and business logics for fetching Istio CVEs #984
Changes from all commits
0da7c8e
1190612
95c7bb3
0672f36
7f0e4ad
8002d1c
8b08c6e
39f9200
de7a831
b48d8ad
0254d0c
807c953
abc1e5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package convert | ||
|
||
import ( | ||
"github.com/hashicorp/go-version" | ||
log "github.com/sirupsen/logrus" | ||
"github.com/stackrox/istio-cves/types" | ||
"github.com/stackrox/rox/pkg/stringutils" | ||
v1 "github.com/stackrox/scanner/generated/scanner/api/v1" | ||
"github.com/stackrox/scanner/pkg/istioutil" | ||
pkgtypes "github.com/stackrox/scanner/pkg/types" | ||
) | ||
|
||
// IstioVulnerabilities converts istio cve schema to vulnerability. | ||
func IstioVulnerabilities(vStr string, istioVulns []types.Vuln) []*v1.Vulnerability { | ||
res := make([]*v1.Vulnerability, 0, len(istioVulns)) | ||
v, err := version.NewVersion(vStr) | ||
if err != nil { | ||
log.Infof("Failed to get version: %s", vStr) | ||
return nil | ||
} | ||
for _, istioVuln := range istioVulns { | ||
m, err := pkgtypes.ConvertMetadataFromIstio(istioVuln) | ||
if err != nil { | ||
log.Errorf("unable to convert metadata for %s: %istioVuln", istioVuln.Name, err) | ||
continue | ||
} | ||
if m.IsNilOrEmpty() { | ||
log.Warnf("nil or empty metadata for %s", istioVuln.Name) | ||
continue | ||
} | ||
|
||
link := stringutils.OrDefault(istioVuln.Link, "https://istio.io/latest/news/security/") | ||
_, fixedBy, err := istioutil.IsAffected(v, istioVuln) | ||
if err != nil { | ||
log.Errorf("unable to get fixedBy for %s: %istioVuln", istioVuln.Name, err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you meant |
||
continue | ||
} | ||
|
||
res = append(res, &v1.Vulnerability{ | ||
Name: istioVuln.Name, | ||
Description: istioVuln.Description, | ||
Link: link, | ||
MetadataV2: Metadata(m), | ||
FixedBy: fixedBy, | ||
Severity: string(DatabaseSeverityToSeverity(m.GetDatabaseSeverity())), | ||
}) | ||
} | ||
return res | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
"github.com/stackrox/scanner/api/v1/convert" | ||
"github.com/stackrox/scanner/database" | ||
v1 "github.com/stackrox/scanner/generated/scanner/api/v1" | ||
istioCache "github.com/stackrox/scanner/istio/cache" | ||
k8scache "github.com/stackrox/scanner/k8s/cache" | ||
"github.com/stackrox/scanner/pkg/version" | ||
"google.golang.org/grpc" | ||
|
@@ -27,20 +28,22 @@ type Service interface { | |
} | ||
|
||
// NewService returns the service for scanning | ||
func NewService(db database.Datastore, k8sCache k8scache.Cache) Service { | ||
func NewService(db database.Datastore, k8sCache k8scache.Cache, istioCache istioCache.Cache) Service { | ||
return &serviceImpl{ | ||
version: version.Version, | ||
db: db, | ||
k8sCache: k8sCache, | ||
version: version.Version, | ||
db: db, | ||
k8sCache: k8sCache, | ||
istioCache: istioCache, | ||
} | ||
} | ||
|
||
type serviceImpl struct { | ||
v1.UnimplementedOrchestratorScanServiceServer | ||
RTann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
version string | ||
db database.Datastore | ||
k8sCache k8scache.Cache | ||
version string | ||
db database.Datastore | ||
k8sCache k8scache.Cache | ||
istioCache istioCache.Cache | ||
} | ||
|
||
func filterInvalidVulns(vulns []*v1.Vulnerability) []*v1.Vulnerability { | ||
|
@@ -107,6 +110,29 @@ func (s *serviceImpl) GetKubeVulnerabilities(_ context.Context, req *v1.GetKubeV | |
return resp, nil | ||
} | ||
|
||
// GetIstioVulnerabilities returns Istio vulnerabilities for requested Kubernetes version. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Istio version |
||
func (s *serviceImpl) GetIstioVulnerabilities(_ context.Context, req *v1.GetIstioVulnerabilitiesRequest) (*v1.GetIstioVulnerabilitiesResponse, error) { | ||
resp := &v1.GetIstioVulnerabilitiesResponse{ | ||
ScannerVersion: s.version, | ||
} | ||
|
||
if req.GetIstioVersion() == "" { | ||
return nil, errors.New("Can't get vulnerabilities for empty version.") | ||
} | ||
version, err := convert.TruncateVersion(req.GetIstioVersion()) | ||
if err != nil { | ||
log.Warnf("Unable to convert Istio version of %s - %v. Skipping...", version, err) | ||
return nil, nil | ||
} | ||
|
||
vulns := s.istioCache.GetVulnsByVersion(version) | ||
converted := convert.IstioVulnerabilities(version, vulns) | ||
|
||
resp.Vulnerabilities = filterInvalidVulns(converted) | ||
|
||
return resp, nil | ||
} | ||
|
||
func (s *serviceImpl) getOpenShiftVulns(version *openShiftVersion) ([]*database.RHELv2Vulnerability, error) { | ||
pkg := &database.RHELv2Package{ | ||
Name: version.CreatePkgName(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,60 @@ var ( | |
patchRegex = regexp.MustCompile(`^[0-9]+\.[0-9]+\.([0-9]+)$`) | ||
) | ||
|
||
func TestGRPCGetIstioVulnerabilities(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really understand what this is testing. What do you ultimately want to test here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just want to make sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it may be more worthwhile to give an Istio version and ensure we find specific vulnerabilities. For example, we know Istio verison 1.13.6 is affected by ISTIO-SECURITY-2022-007, which has a specific description and CVSS and is fixed by 1.13.9 |
||
conn := connectToScanner(t) | ||
client := v1.NewOrchestratorScanServiceClient(conn) | ||
|
||
type istionVulnStruct struct { | ||
version string | ||
name string | ||
fixedBy string | ||
severity string | ||
score float32 | ||
} | ||
|
||
testCases := []istionVulnStruct{ | ||
{ | ||
version: "1.13.6", | ||
name: "ISTIO-SECURITY-2022-006", | ||
fixedBy: "1.13.7", | ||
severity: "Moderate", | ||
score: 5.9, | ||
}, | ||
{ | ||
version: "1.14.0", | ||
name: "ISTIO-SECURITY-2022-007", | ||
fixedBy: "1.14.5", | ||
severity: "Important", | ||
score: 7.5, | ||
}, | ||
} | ||
|
||
testSet := make(map[string]istionVulnStruct) | ||
|
||
for _, c := range testCases { | ||
t.Run(fmt.Sprintf("case-%s", c.version), func(t *testing.T) { | ||
req := &v1.GetIstioVulnerabilitiesRequest{IstioVersion: c.version} | ||
resp, err := client.GetIstioVulnerabilities(context.Background(), req) | ||
assert.NoError(t, err) | ||
|
||
for _, vuln := range resp.GetVulnerabilities() { | ||
assert.NotNil(t, vuln.GetMetadataV2().GetCvssV3()) | ||
assert.NotEmpty(t, vuln.Name) | ||
assert.NotEmpty(t, vuln.FixedBy) | ||
assert.NotEmpty(t, vuln.Severity) | ||
sample := istionVulnStruct{version: c.version, name: vuln.Name, fixedBy: vuln.FixedBy, severity: vuln.Severity, score: vuln.GetMetadataV2().GetCvssV3().Score} | ||
testSet[vuln.Name] = sample | ||
} | ||
|
||
assert.NotEmpty(t, testSet[c.name]) | ||
assert.Equal(t, c.fixedBy, testSet[c.name].fixedBy) | ||
assert.Equal(t, c.severity, testSet[c.name].severity) | ||
assert.Equal(t, c.score, testSet[c.name].score) | ||
}) | ||
} | ||
} | ||
|
||
func TestGRPCGetOpenShiftVulnerabilities(t *testing.T) { | ||
conn := connectToScanner(t) | ||
client := v1.NewOrchestratorScanServiceClient(conn) | ||
|
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.
Can we leave a comment saying we know this version is affected, which is why we can ignore that return value?