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

check soname: fix edge case where a package can contain multiple shar… #761

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
133 changes: 62 additions & 71 deletions pkg/checks/so_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"os"
"path/filepath"
"regexp"
"strconv"
"strings"

goapk "github.com/chainguard-dev/go-apk/pkg/apk"
"github.com/wolfi-dev/wolfictl/pkg/apk"
"github.com/wolfi-dev/wolfictl/pkg/lint"
"github.com/wolfi-dev/wolfictl/pkg/tar"
"github.com/wolfi-dev/wolfictl/pkg/versions"
)

type SoNameOptions struct {
Expand Down Expand Up @@ -134,108 +134,99 @@ func (o *SoNameOptions) diff(newPackageName string, newAPK NewApkPackage) error

err = o.checkSonamesMatch(existingSonameFiles, newSonameFiles)
if err != nil {
return fmt.Errorf("soname files differ, this can cause an ABI break. Existing soname files %s, New soname files %s: %w", strings.Join(existingSonameFiles, ","), strings.Join(newSonameFiles, ","), err)
return fmt.Errorf("soname files differ, this can cause an ABI breakage %w", err)
}

return nil
}

func (o *SoNameOptions) getSonameFiles(dir string) ([]string, error) {
reg := regexp.MustCompile(`\.so.(\d+\.)?(\d+\.)?(\*|\d+)`)
func (o *SoNameOptions) getSonameFiles(dir string) (map[string][]string, error) {
reg := regexp.MustCompile(`\.so\.(\d+\.)?(\d+\.)?(\*|\d+)`)

var fileList []string
sonameFiles := make(map[string][]string)
err := filepath.Walk(dir, func(path string, _ os.FileInfo, err error) error {
if err != nil {
return err
}
basePath := filepath.Base(path)
s := reg.FindString(basePath)
if s != "" {
fileList = append(fileList, basePath)
name := strings.Split(basePath, ".so")[0]
sonameFiles[name] = append(sonameFiles[name], s)
}

// also check for DT_SONAME
ef, err := elf.Open(filepath.Join(dir, basePath))
if err != nil {
return nil
return nil // Skipping files that can't be opened as ELF files
}
defer ef.Close()

sonames, err := ef.DynString(elf.DT_SONAME)
// most likely SONAME is not set on this object
if err != nil {
return nil
}

if len(sonames) > 0 {
fileList = append(fileList, sonames...)
if err == nil && len(sonames) > 0 {
name := strings.Split(sonames[0], ".so")[0]
sonameFiles[name] = append(sonameFiles[name], sonames[0])
}
return nil
})

return fileList, err
return sonameFiles, err
}

func (o *SoNameOptions) checkSonamesMatch(existingSonameFiles, newSonameFiles []string) error {
if len(existingSonameFiles) == 0 {
o.Logger.Printf("no existing soname files, skipping")
return nil
}

// regex to match version and optional qualifier
// \d+(\.\d+)* captures version numbers that may have multiple parts separated by dots
// ([a-zA-Z0-9-_]*) captures optional alphanumeric (including hyphens and underscores) qualifiers
r := regexp.MustCompile(`(\d+(\.\d+)*)([a-zA-Z0-9-_]*)`)

// first turn the existing soname files into a map so it is easier to match with
existingSonameMap := make(map[string]string)
for _, soname := range existingSonameFiles {
o.Logger.Printf("checking soname file %s", soname)
sonameParts := strings.Split(soname, ".so")

// Find the version and optional qualifier
matches := r.FindStringSubmatch(sonameParts[1])
if len(matches) > 0 {
version := matches[0] // The entire match, including optional qualifier
existingSonameMap[sonameParts[0]] = version
}
}

// now iterate over new soname files and compare with existing files
for _, soname := range newSonameFiles {
sonameParts := strings.Split(soname, ".so")
name := sonameParts[0]
versionStr := strings.TrimPrefix(sonameParts[1], ".")
existingVersionStr := existingSonameMap[name]

// skip if no matching file
if existingVersionStr == "" {
o.Logger.Printf("no existing soname version found for %s, skipping", name)
func (o *SoNameOptions) checkSonamesMatch(existingSonameFiles, newSonameFiles map[string][]string) error {
for name, newVersions := range newSonameFiles {
existingVersions, exists := existingSonameFiles[name]
if !exists {
// Continue if there are no existing versions to compare against, assuming no conflict.
continue
}

// turning the string version into proper version will give us major.minor.patch segments
existingVersion, err := versions.NewVersion(existingVersionStr)
if err != nil {
return fmt.Errorf("failed to parse existing version %s: %w", existingVersionStr, err)
}

matches := r.FindStringSubmatch(versionStr)
if len(matches) > 0 {
versionStr = matches[0] // The entire match, including optional qualifier
for _, newVer := range newVersions {
versionMatch := false
for _, existVer := range existingVersions {
compatible, err := areVersionsCompatible(newVer, existVer)
if err != nil {
return fmt.Errorf("error checking version compatibility for %s: %w", name, err)
}
if compatible {
versionMatch = true
break
}
}
if !versionMatch {
return fmt.Errorf("soname version mismatch for %s: existing versions %v, new version %s", name, existingVersions, newVer)
}
}
}
return nil
}

version, err := versions.NewVersion(versionStr)
if err != nil {
return fmt.Errorf("failed to parse new version %s: %w", existingVersionStr, err)
}
// Example helper function to decide if versions are compatible
func areVersionsCompatible(newVer, existVer string) (bool, error) {
// Check if either version string is the base "so", which we treat as a wildcard.
if existVer == "so" {
return true, nil
}
newMajor, err := parseMajorVersion(newVer)
if err != nil {
return false, fmt.Errorf("failed to parse new version: %w", err)
}
existMajor, err := parseMajorVersion(existVer)
if err != nil {
return false, fmt.Errorf("failed to parse existing version: %w", err)
}
return newMajor == existMajor, nil
}

// let's now compare the major segments as only major version increments indicate a break ABI compatibility
newVersionMajor := version.Segments()[0]
existingVersionMajor := existingVersion.Segments()[0]
if newVersionMajor > existingVersionMajor {
return fmt.Errorf("soname version check failed, %s has an existing version %s while new package contains a different version %s. This can cause ABI failures", name, existingVersion, version)
}
// Parses the major version part from a version string
func parseMajorVersion(ver string) (int, error) {
// Simplified version parsing logic here, assuming version string starts with "so."
parts := strings.Split(ver, ".")
if len(parts) < 2 {
return 0, fmt.Errorf("invalid version format")
}
return nil
major, err := strconv.Atoi(parts[1]) // Assuming "so.1.2.3" format
if err != nil {
return 0, err
}
return major, nil
}
118 changes: 84 additions & 34 deletions pkg/checks/so_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,73 +72,118 @@ func TestChecks_getSonameFiles(t *testing.T) {
func TestSoNameOptions_checkSonamesMatch(t *testing.T) {
tests := []struct {
name string
existingSonameFiles []string
newSonameFiles []string
wantErr assert.ErrorAssertionFunc
existingSonameFiles map[string][]string
newSonameFiles map[string][]string
wantErr bool
}{
{
name: "deleted", existingSonameFiles: []string{"foo.so", "bar.so"}, newSonameFiles: []string{"foo.so"},
wantErr: assert.NoError,
name: "deleted",
existingSonameFiles: map[string][]string{"foo": {"so"}, "bar": {"so"}},
newSonameFiles: map[string][]string{"foo": {"so"}},
wantErr: false,
},
{
name: "match", existingSonameFiles: []string{"foo.so", "bar.so"}, newSonameFiles: []string{"foo.so", "bar.so"},
wantErr: assert.NoError,
name: "match",
existingSonameFiles: map[string][]string{"foo": {"so"}, "bar": {"so"}},
newSonameFiles: map[string][]string{"foo": {"so"}, "bar": {"so"}},
wantErr: false,
},
{
name: "ignore", existingSonameFiles: []string{"foo.so"}, newSonameFiles: []string{"foo.so.1"},
wantErr: assert.NoError,
name: "ignore",
existingSonameFiles: map[string][]string{"foo": {"so"}},
newSonameFiles: map[string][]string{"foo": {"so.1"}},
wantErr: false,
},
{
name: "match", existingSonameFiles: []string{"foo.so.1"}, newSonameFiles: []string{"foo.so.1"},
wantErr: assert.NoError,
name: "match",
existingSonameFiles: map[string][]string{"foo": {"so.1"}},
newSonameFiles: map[string][]string{"foo": {"so.1"}},
wantErr: false,
},
{
name: "match_multiple", existingSonameFiles: []string{"foo.so.1", "bar.so.2"}, newSonameFiles: []string{"foo.so.1", "bar.so.2"},
wantErr: assert.NoError,
name: "match_multiple",
existingSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.2"}},
newSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.2"}},
wantErr: false,
},
{
name: "match_multiple_different_order", existingSonameFiles: []string{"bar.so.2", "foo.so.1"}, newSonameFiles: []string{"foo.so.1", "bar.so.2"},
wantErr: assert.NoError,
name: "match_multiple_different_order",
existingSonameFiles: map[string][]string{"bar": {"so.2"}, "foo": {"so.1"}},
newSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.2"}},
wantErr: false,
},
{
name: "single_fail", existingSonameFiles: []string{"foo.so.1"}, newSonameFiles: []string{"foo.so.2"},
wantErr: assert.Error,
name: "single_fail",
existingSonameFiles: map[string][]string{"foo": {"so.1"}},
newSonameFiles: map[string][]string{"foo": {"so.2"}},
wantErr: true,
},
{
name: "multi_fail", existingSonameFiles: []string{"foo.so.1", "bar.so.1"}, newSonameFiles: []string{"foo.so.1", "bar.so.2"},
wantErr: assert.Error,
name: "multi_fail",
existingSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.1"}},
newSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.2"}},
wantErr: true,
},
{
name: "skip_new", existingSonameFiles: []string{"foo.so.1", "bar.so.1"}, newSonameFiles: []string{"cheese.so.1"},
wantErr: assert.NoError,
name: "skip_new",
existingSonameFiles: map[string][]string{"foo": {"so.1"}, "bar": {"so.1"}},
newSonameFiles: map[string][]string{"cheese": {"so.1"}},
wantErr: false,
},
{
name: "abi_compatible", existingSonameFiles: []string{"foo.so.1.2"}, newSonameFiles: []string{"foo.so.1.3"},
wantErr: assert.NoError,
name: "abi_compatible",
existingSonameFiles: map[string][]string{"foo": {"so.1.2"}},
newSonameFiles: map[string][]string{"foo": {"so.1.3"}},
wantErr: false,
},
{
name: "no_existing", existingSonameFiles: []string{}, newSonameFiles: []string{"cheese.so.1"},
wantErr: assert.NoError,
name: "no_existing",
existingSonameFiles: map[string][]string{},
newSonameFiles: map[string][]string{"cheese": {"so.1"}},
wantErr: false,
},
{
name: "none_at_all", existingSonameFiles: []string{}, newSonameFiles: []string{},
wantErr: assert.NoError,
name: "none_at_all",
existingSonameFiles: map[string][]string{},
newSonameFiles: map[string][]string{},
wantErr: false,
},
{
name: "complex_chars_with_qualifier", existingSonameFiles: []string{"libstdc++.so.6.0.30-gdb.py"}, newSonameFiles: []string{"libstdc++.so.6.0.30-gdb.py"},
wantErr: assert.NoError,
name: "complex_chars_with_qualifier",
existingSonameFiles: map[string][]string{"libstdc++": {"so.6.0.30-gdb.py"}},
newSonameFiles: map[string][]string{"libstdc++": {"so.6.0.30-gdb.py"}},
wantErr: false,
},
{
name: "ignore_non_standard_version_suffix", existingSonameFiles: []string{"libgs.so.10.02.debug"}, newSonameFiles: []string{"libgs.so.10.02.debug"},
wantErr: assert.NoError,
name: "ignore_non_standard_version_suffix",
existingSonameFiles: map[string][]string{"libgs": {"so.10.02.debug"}},
newSonameFiles: map[string][]string{"libgs": {"so.10.02.debug"}},
wantErr: false,
},
{
name: "multiple_versions_handled",
existingSonameFiles: map[string][]string{"libkrb5": {"so.26", "so.3"}},
newSonameFiles: map[string][]string{"libkrb5": {"so.26", "so.3"}},
wantErr: false,
},
{
name: "multiple_versions_handled_new_version",
existingSonameFiles: map[string][]string{"libkrb5": {"so.26", "so.3"}},
newSonameFiles: map[string][]string{"libkrb5": {"so.27", "so.3"}},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o := SoNameOptions{
Logger: log.New(log.Writer(), "test: ", log.LstdFlags|log.Lmsgprefix),
}
tt.wantErr(t, o.checkSonamesMatch(tt.existingSonameFiles, tt.newSonameFiles), fmt.Sprintf("checkSonamesMatch(%v, %v)", tt.existingSonameFiles, tt.newSonameFiles))
err := o.checkSonamesMatch(tt.existingSonameFiles, tt.newSonameFiles)
if tt.wantErr {
assert.Error(t, err, fmt.Sprintf("checkSonamesMatch(%v, %v) should fail", tt.existingSonameFiles, tt.newSonameFiles))
} else {
assert.NoError(t, err, fmt.Sprintf("checkSonamesMatch(%v, %v)", tt.existingSonameFiles, tt.newSonameFiles))
}
})
}
}
Expand All @@ -152,13 +197,18 @@ func TestSoNameOptions_checkSonamesSubFolders(t *testing.T) {
t.Fatal(err)
}

err = os.WriteFile(filepath.Join(subDir, "bar.so.1.2.3"), []byte("test"), os.ModePerm)
filename := "bar.so.1.2.3"
err = os.WriteFile(filepath.Join(subDir, filename), []byte("test"), os.ModePerm)
if err != nil {
t.Fatal(err)
}

got, err := o.getSonameFiles(dir)
assert.NoError(t, err)

assert.Equal(t, "bar.so.1.2.3", got[0])
// Check if the map correctly identifies the base name 'bar' and includes the version 'so.1.2.3'
expected := map[string][]string{
"bar": {".so.1.2.3"},
}
assert.Equal(t, expected, got)
}
Loading