Skip to content

Commit

Permalink
check soname: fix edge case where a package can contain multiple shar…
Browse files Browse the repository at this point in the history 
…ed object versions.

```
libkrb5 has an existing version 3.0.0 while new package contains a different version 26.0.0.  This can cause ABI failures
```

ref build logs: [link](https://github.com/wolfi-dev/os/actions/runs/8690170824/job/23829609093?pr=16886)

Signed-off-by: James Rawlings <[email protected]>
  • Loading branch information
@rawlingsj
rawlingsj committed Apr 16, 2024
1 parent c2cfab7 commit 0893a1c
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 105 deletions.
133 changes: 62 additions & 71 deletions pkg/checks/so_name.go
View file
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
View file
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)
}

0 comments on commit 0893a1c

Please sign in to comment.