Skip to content

Commit

Permalink
fix pruning binary packages when considering ELF packages (#2862)
Browse files Browse the repository at this point in the history 
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
  • Loading branch information
@wagoodman
wagoodman committed May 9, 2024
1 parent 4194a2c commit c200896
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 59 deletions.
6 changes: 4 additions & 2 deletions cmd/syft/internal/test/integration/package_binary_elf_relationships_test.go
View file
@@ -1,10 +1,12 @@
package integration

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/source"
"github.com/stretchr/testify/require"
"testing"
)

func TestBinaryElfRelationships(t *testing.T) {
Expand Down
63 changes: 40 additions & 23 deletions internal/relationship/binary/binary_dependencies.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"path"

"github.com/anchore/syft/internal/log"

"github.com/anchore/syft/internal/sbomsync"
"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/file"
Expand Down Expand Up @@ -67,41 +66,59 @@ func generateRelationships(resolver file.Resolver, accessor sbomsync.Accessor, i
func PackagesToRemove(resolver file.Resolver, accessor sbomsync.Accessor) []artifact.ID {
pkgsToDelete := make([]artifact.ID, 0)
accessor.ReadFromSBOM(func(s *sbom.SBOM) {
// OTHER > ELF > Binary
// OTHER package type > ELF package type > Binary package type
pkgsToDelete = append(pkgsToDelete, getBinaryPackagesToDelete(resolver, s)...)
pkgsToDelete = append(pkgsToDelete, compareElfBinaryPackages(resolver, s)...)
pkgsToDelete = append(pkgsToDelete, compareElfBinaryPackages(s)...)
})
return pkgsToDelete
}

func compareElfBinaryPackages(resolver file.Resolver, s *sbom.SBOM) []artifact.ID {
func compareElfBinaryPackages(s *sbom.SBOM) []artifact.ID {
pkgsToDelete := make([]artifact.ID, 0)
for _, p := range s.Artifacts.Packages.Sorted(pkg.BinaryPkg) {
for _, loc := range p.Locations.ToSlice() {
if loc.Annotations[pkg.EvidenceAnnotationKey] != pkg.PrimaryEvidenceAnnotation {
continue
}
locations, err := resolver.FilesByPath(loc.RealPath)
if err != nil {
log.WithFields("error", err).Trace("unable to find path for owned file")
continue
}
for _, ownedL := range locations {
for _, pathPkg := range s.Artifacts.Packages.PackagesByPath(ownedL.RealPath) {
// we only care about comparing binary packages to each other (not other types)
if pathPkg.Type != pkg.BinaryPkg {
continue
}
if _, ok := pathPkg.Metadata.(pkg.ELFBinaryPackageNoteJSONPayload); !ok {
pkgsToDelete = append(pkgsToDelete, pathPkg.ID())
}
for _, elfPkg := range allElfPackages(s) {
for _, loc := range onlyPrimaryEvidenceLocations(elfPkg) {
for _, otherPkg := range s.Artifacts.Packages.PackagesByPath(loc.RealPath) {
// we only care about comparing binary packages to each other (not other types)
if otherPkg.Type != pkg.BinaryPkg {
continue
}
if !isElfPackage(otherPkg) {
pkgsToDelete = append(pkgsToDelete, otherPkg.ID())
}
}
}
}
return pkgsToDelete
}

func onlyPrimaryEvidenceLocations(p pkg.Package) []file.Location {
var locs []file.Location
for _, loc := range p.Locations.ToSlice() {
if loc.Annotations[pkg.EvidenceAnnotationKey] != pkg.PrimaryEvidenceAnnotation {
continue
}
locs = append(locs, loc)
}

return locs
}

func allElfPackages(s *sbom.SBOM) []pkg.Package {
var elfPkgs []pkg.Package
for _, p := range s.Artifacts.Packages.Sorted(pkg.BinaryPkg) {
if !isElfPackage(p) {
continue
}
elfPkgs = append(elfPkgs, p)
}
return elfPkgs
}

func isElfPackage(p pkg.Package) bool {
_, ok := p.Metadata.(pkg.ELFBinaryPackageNoteJSONPayload)
return ok
}

func getBinaryPackagesToDelete(resolver file.Resolver, s *sbom.SBOM) []artifact.ID {
pkgsToDelete := make([]artifact.ID, 0)
for p := range s.Artifacts.Packages.Enumerate() {
Expand Down
82 changes: 50 additions & 32 deletions internal/relationship/binary/binary_dependencies_test.go
View file
Expand Up @@ -34,13 +34,11 @@ func TestPackagesToRemove(t *testing.T) {
glibCPackage.SetID()

glibCBinaryELFPackage := pkg.Package{
Name: "glibc",
Version: "",
Name: "glibc",
Locations: file.NewLocationSet(
file.NewLocation(glibcCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation),
),
Language: "",
Type: pkg.BinaryPkg,
Type: pkg.BinaryPkg,
Metadata: pkg.ELFBinaryPackageNoteJSONPayload{
Type: "testfixture",
Vendor: "syft",
Expand All @@ -52,17 +50,25 @@ func TestPackagesToRemove(t *testing.T) {
glibCBinaryELFPackage.SetID()

glibCBinaryClassifierPackage := pkg.Package{
Name: "glibc",
Version: "",
Name: "glibc",
Locations: file.NewLocationSet(
file.NewLocation(glibcCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation),
),
Language: "",
Type: pkg.BinaryPkg,
Metadata: pkg.BinarySignature{},
}
glibCBinaryClassifierPackage.SetID()

libCBinaryClassifierPackage := pkg.Package{
Name: "libc",
Locations: file.NewLocationSet(
file.NewLocation(glibcCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation),
),
Type: pkg.BinaryPkg,
Metadata: pkg.BinarySignature{},
}
libCBinaryClassifierPackage.SetID()

tests := []struct {
name string
resolver file.Resolver
Expand All @@ -72,21 +78,33 @@ func TestPackagesToRemove(t *testing.T) {
{
name: "remove packages that are overlapping rpm --> binary",
resolver: file.NewMockResolverForPaths(glibcCoordinate.RealPath),
accessor: newAccesor([]pkg.Package{glibCPackage, glibCBinaryELFPackage}, map[file.Coordinates]file.Executable{}, nil),
accessor: newAccessor([]pkg.Package{glibCPackage, glibCBinaryELFPackage}, map[file.Coordinates]file.Executable{}, nil),
want: []artifact.ID{glibCBinaryELFPackage.ID()},
},
{
name: "remove no packages when there is a single binary package",
resolver: file.NewMockResolverForPaths(glibcCoordinate.RealPath),
accessor: newAccesor([]pkg.Package{glibCBinaryELFPackage}, map[file.Coordinates]file.Executable{}, nil),
accessor: newAccessor([]pkg.Package{glibCBinaryELFPackage}, map[file.Coordinates]file.Executable{}, nil),
want: []artifact.ID{},
},
{
name: "remove packages when there is a single binary package and a classifier package",
resolver: file.NewMockResolverForPaths(glibcCoordinate.RealPath),
accessor: newAccesor([]pkg.Package{glibCBinaryELFPackage, glibCBinaryClassifierPackage}, map[file.Coordinates]file.Executable{}, nil),
accessor: newAccessor([]pkg.Package{glibCBinaryELFPackage, glibCBinaryClassifierPackage}, map[file.Coordinates]file.Executable{}, nil),
want: []artifact.ID{glibCBinaryClassifierPackage.ID()},
},
{
name: "ensure we're considering ELF packages, not just binary packages (supporting evidence)",
resolver: file.NewMockResolverForPaths(glibcCoordinate.RealPath),
accessor: newAccessor([]pkg.Package{glibCBinaryClassifierPackage}, map[file.Coordinates]file.Executable{}, nil),
want: []artifact.ID{},
},
{
name: "ensure we're considering ELF packages, not just binary packages (primary evidence)",
resolver: file.NewMockResolverForPaths(glibcCoordinate.RealPath),
accessor: newAccessor([]pkg.Package{libCBinaryClassifierPackage}, map[file.Coordinates]file.Executable{}, nil),
want: []artifact.ID{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -103,7 +121,7 @@ func TestNewDependencyRelationships(t *testing.T) {
glibcCoordinate := file.NewCoordinates("/usr/lib64/libc.so.6", "")
secondGlibcCoordinate := file.NewCoordinates("/usr/local/lib64/libc.so.6", "")
nestedLibCoordinate := file.NewCoordinates("/usr/local/bin/elftests/elfbinwithnestedlib/bin/elfbinwithnestedlib", "")
parrallelLibCoordinate := file.NewCoordinates("/usr/local/bin/elftests/elfbinwithsisterlib/bin/elfwithparallellibbin1", "")
parallelLibCoordinate := file.NewCoordinates("/usr/local/bin/elftests/elfbinwithsisterlib/bin/elfwithparallellibbin1", "")

// rpm package that was discovered in linked section of the ELF binary package
glibCPackage := pkg.Package{
Expand Down Expand Up @@ -151,7 +169,7 @@ func TestNewDependencyRelationships(t *testing.T) {
FoundBy: "",
Locations: file.NewLocationSet(
file.NewLocation(nestedLibCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation),
file.NewLocation(parrallelLibCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation),
file.NewLocation(parallelLibCoordinate.RealPath).WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation),
),
Language: "",
Type: pkg.BinaryPkg,
Expand Down Expand Up @@ -214,13 +232,13 @@ func TestNewDependencyRelationships(t *testing.T) {
resolver: file.NewMockResolverForPaths(
glibcCoordinate.RealPath,
nestedLibCoordinate.RealPath,
parrallelLibCoordinate.RealPath,
parallelLibCoordinate.RealPath,
),
// path -> executable (above mock resolver needs to be able to resolve to paths in this map)
coordinateIndex: map[file.Coordinates]file.Executable{
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parrallelLibCoordinate: syftTestFixtureExecutable2,
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parallelLibCoordinate: syftTestFixtureExecutable2,
},
packages: []pkg.Package{glibCPackage, syftTestFixturePackage},
want: []artifact.Relationship{
Expand All @@ -237,13 +255,13 @@ func TestNewDependencyRelationships(t *testing.T) {
glibcCoordinate.RealPath,
secondGlibcCoordinate.RealPath,
nestedLibCoordinate.RealPath,
parrallelLibCoordinate.RealPath,
parallelLibCoordinate.RealPath,
),
coordinateIndex: map[file.Coordinates]file.Executable{
glibcCoordinate: glibcExecutable,
secondGlibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parrallelLibCoordinate: syftTestFixtureExecutable2,
glibcCoordinate: glibcExecutable,
secondGlibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parallelLibCoordinate: syftTestFixtureExecutable2,
},
packages: []pkg.Package{glibCPackage, glibCustomPackage, syftTestFixturePackage},
want: []artifact.Relationship{
Expand All @@ -264,12 +282,12 @@ func TestNewDependencyRelationships(t *testing.T) {
resolver: file.NewMockResolverForPaths(
glibcCoordinate.RealPath,
nestedLibCoordinate.RealPath,
parrallelLibCoordinate.RealPath,
parallelLibCoordinate.RealPath,
),
coordinateIndex: map[file.Coordinates]file.Executable{
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parrallelLibCoordinate: syftTestFixtureExecutable2,
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parallelLibCoordinate: syftTestFixtureExecutable2,
},
packages: []pkg.Package{glibCPackage, glibCustomPackage, syftTestFixturePackage},
prexistingRelationships: []artifact.Relationship{
Expand All @@ -285,17 +303,17 @@ func TestNewDependencyRelationships(t *testing.T) {
name: "given a package that imports a library that is not tracked by the resolver, expect no relationships to be created",
resolver: file.NewMockResolverForPaths(),
coordinateIndex: map[file.Coordinates]file.Executable{
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parrallelLibCoordinate: syftTestFixtureExecutable2,
glibcCoordinate: glibcExecutable,
nestedLibCoordinate: syftTestFixtureExecutable,
parallelLibCoordinate: syftTestFixtureExecutable2,
},
packages: []pkg.Package{glibCPackage, syftTestFixturePackage},
want: []artifact.Relationship{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
accessor := newAccesor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
accessor := newAccessor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
// given a resolver that knows about the paths of the packages and executables,
// and given an SBOM accessor that knows about the packages and executables,
// we should be able to create a set of relationships between the packages and executables
Expand All @@ -316,7 +334,7 @@ func relationshipComparer(x, y []artifact.Relationship) string {
))
}

func newAccesor(pkgs []pkg.Package, coordinateIndex map[file.Coordinates]file.Executable, prexistingRelationships []artifact.Relationship) sbomsync.Accessor {
func newAccessor(pkgs []pkg.Package, coordinateIndex map[file.Coordinates]file.Executable, preexistingRelationships []artifact.Relationship) sbomsync.Accessor {
sb := sbom.SBOM{
Artifacts: sbom.Artifacts{
Packages: pkg.NewCollection(),
Expand All @@ -329,8 +347,8 @@ func newAccesor(pkgs []pkg.Package, coordinateIndex map[file.Coordinates]file.Ex
accessor := builder.(sbomsync.Accessor)
accessor.WriteToSBOM(func(s *sbom.SBOM) {
s.Artifacts.Executables = coordinateIndex
if prexistingRelationships != nil {
s.Relationships = prexistingRelationships
if preexistingRelationships != nil {
s.Relationships = preexistingRelationships
}
})
return accessor
Expand Down
4 changes: 2 additions & 2 deletions internal/relationship/binary/shared_library_index_test.go
View file
Expand Up @@ -27,7 +27,7 @@ func Test_newShareLibIndex(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
accessor := newAccesor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
accessor := newAccessor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
sharedLibraryIndex := newShareLibIndex(tt.resolver, accessor)
if sharedLibraryIndex == nil {
t.Errorf("newShareLibIndex() = %v, want non-nil", sharedLibraryIndex)
Expand Down Expand Up @@ -93,7 +93,7 @@ func Test_sharedLibraryIndex_build(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
accessor := newAccesor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
accessor := newAccessor(tt.packages, tt.coordinateIndex, tt.prexistingRelationships)
sharedLibraryIndex := newShareLibIndex(tt.resolver, accessor)
sharedLibraryIndex.build(tt.resolver, accessor)
pkgs := sharedLibraryIndex.owningLibraryPackage(path.Base(glibcCoordinate.RealPath))
Expand Down

0 comments on commit c200896

Please sign in to comment.