adding rpm architecture of to our maps, so 32 bit and 64 bit rpms do …

…not fail has modified files check

Signed-off-by: Adam D. Cornett <[email protected]>

internal/policy/container/has_modified_files.go

@@ -231,17 +231,27 @@ func (p *HasModifiedFilesCheck) validate(ctx context.Context, layerIDs []string,
 				}
 
 				// Nope, nope, nope. File was modified without using RPM
-				logger.V(log.DBG).Info("found disallowed modification in layer", "file", modifiedFile)
+				logger.Info("found disallowed modification in layer", "file", modifiedFile)
 				disallowedModifications = true
 				continue
 			}
-			// Check that release contains the same arch and OS release
+
+			// Check that release contains the same arch, this is to ensure that a package did not get rebuilt with
+			// a different architecture
 			previousOsRelease := strings.Contains(previousPackage.Release, packageDist)
 			currentOsRelease := strings.Contains(currentPackage.Release, packageDist)
 
-			if (previousOsRelease && !currentOsRelease) || (previousPackage.Arch != currentPackage.Arch) {
-				// If either of these differ, that's a fail
-				return false, nil
+			if previousOsRelease && !currentOsRelease {
+				logger.Info("mismatch in OS release", "file", modifiedFile)
+				disallowedModifications = true
+				continue
+			}
+
+			// Check that the architectures for previous version and current version of a given package match
+			if previousPackage.Arch != currentPackage.Arch {
+				logger.Info("mismatch in package architecture", "file", modifiedFile)
+				disallowedModifications = true
+				continue
 			}
 
 			// This appears like an update. This is allowed.
@@ -274,7 +284,7 @@ func (p HasModifiedFilesCheck) Metadata() check.Metadata {
 func extractPackageNameVersionRelease(pkgList []*rpmdb.PackageInfo) map[string]packageMeta {
 	pkgNameList := make(map[string]packageMeta, len(pkgList))
 	for _, pkg := range pkgList {
-		pkgNameList[fmt.Sprintf("%s-%s-%s", pkg.Name, pkg.Version, pkg.Release)] = packageMeta{
+		pkgNameList[strings.Join([]string{pkg.Name, pkg.Version, pkg.Release, pkg.Arch}, "-")] = packageMeta{
 			Name:        pkg.Name,
 			Version:     pkg.Version,
 			Release:     pkg.Release,
@@ -393,19 +403,46 @@ func installedFileMapWithExclusions(ctx context.Context, pkglist []*rpmdb.Packag
 			return m, err
 		}
 
+		// converting directories to a map so we can filter them out quicker
+		pkgDirNamesMap := make(map[string]struct{})
+		for _, dir := range pkg.DirNames {
+			pkgDirNamesMap[dir] = struct{}{}
+		}
+
 		for _, file := range files {
+			if _, found := pkgDirNamesMap[file.Path]; found {
+				// The file is a directory. Skip it.
+				continue
+			}
+
 			if int32(file.Flags)&okFlags > 0 {
 				// It is one of the ok flags. Skip it.
 				continue
 			}
+
 			normalized := normalize(file.Path)
 			if pathIsExcluded(ctx, normalized) || directoryIsExcluded(ctx, normalized) || prefixAndSuffixIsExcluded(ctx, normalized) {
 				// It is either an explicitly excluded path or directory. Skip it.
 				continue
 			}
-			m[normalized] = fmt.Sprintf("%s-%s-%s", pkg.Name, pkg.Version, pkg.Release)
+
+			// checking to see if the file is already in the map.
+			// check to see if all attributes of the rpm match except architecture.
+			// this is to support cross architecture file ownership,
+			// the 2nd architecture we encounter, we can skip it.
+			if val, found := m[normalized]; found {
+				s := strings.Split(val, "-")
+				name, version, release, arch := s[0], s[1], s[2], s[3]
+
+				if name == pkg.Name && version == pkg.Version && release == pkg.Release && arch != pkg.Arch {
+					continue
+				}
+			}
+
+			m[normalized] = strings.Join([]string{pkg.Name, pkg.Version, pkg.Release, pkg.Arch}, "-")
 		}
 	}
+
 	return m, nil
 }
 

internal/policy/container/has_modified_files_test.go

@@ -243,6 +243,37 @@ var _ = Describe("HasModifiedFiles", func() {
 				Expect(ok).To(BeFalse())
 			})
 		})
+		When("the package architecture changes", func() {
+			var pkgs map[string]packageFilesRef
+			BeforeEach(func() {
+				pkgs = pkgRef
+
+				pkgSecondLayerPackageFiles := pkgs["secondlayer"].LayerPackageFiles
+				delete(pkgSecondLayerPackageFiles, "this")
+				pkgSecondLayerPackageFiles["this"] = "foo-1.0-1.d10"
+
+				pkgSecondLayerPackages := pkgs["secondlayer"].LayerPackages
+				delete(pkgSecondLayerPackages, "foo-1.0-1.d9")
+				pkgSecondLayerPackages["foo-1.0-1.d10"] = packageMeta{
+					Name:    "foo",
+					Version: "1.0",
+					Release: "1.d9",
+					Arch:    "differentarch",
+				}
+
+				pkgs["secondlayer"] = packageFilesRef{
+					LayerPackages:     pkgSecondLayerPackages,
+					LayerPackageFiles: pkgSecondLayerPackageFiles,
+					LayerFiles:        append(pkgs["secondlayer"].LayerFiles, "this"),
+					HasRPMDB:          true,
+				}
+			})
+			It("should fail because of different architectures dist", func() {
+				ok, err := hasModifiedFiles.validate(context.Background(), layers, pkgs, dist)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
 		When("release dist does not match installed OS", func() {
 			When("package is a net-new", func() {
 				When("a file is modified", func() {
@@ -321,11 +352,37 @@ var _ = Describe("HasModifiedFiles", func() {
 		var goodPkgList []*rpmdb.PackageInfo
 
 		BeforeEach(func() {
+			// goodPkgList represents three mock RPM's,
+			// the first is a basic one to cover the happy path.
+			// the second is need to test our filtering logic for files from another architecture.
+			// the third is to test our filtering logic for directories.
 			goodPkgList = []*rpmdb.PackageInfo{
 				{
 					BaseNames:  []string{basename},
 					DirIndexes: []int32{dirindex},
 					DirNames:   []string{dirname},
+					Name:       "foo",
+					Version:    "1.0.0",
+					Release:    "100",
+					Arch:       "x86_64",
+				},
+				{
+					BaseNames:  []string{basename},
+					DirIndexes: []int32{dirindex},
+					DirNames:   []string{dirname},
+					Name:       "foo",
+					Version:    "1.0.0",
+					Release:    "100",
+					Arch:       "i686",
+				},
+				{
+					BaseNames:  []string{""},
+					DirIndexes: []int32{dirindex},
+					DirNames:   []string{"/"},
+					Name:       "just-dirs",
+					Version:    "1.0.0",
+					Release:    "100",
+					Arch:       "x86_64",
 				},
 			}
 		})

test/containerfiles/container-fails-modifiedfiles-package.Dockerfile

@@ -11,7 +11,7 @@ LABEL name="preflight test image" \
       summary="testing the preflight tool" \
       description="test the preflight tool"
 
-RUN dnf remove -y tar
+RUN rm -f /bin/true
 
 USER preflightuser
 

test/containerfiles/container-fails-modifiedfiles-setuid.Dockerfile

@@ -0,0 +1,17 @@
+FROM registry.access.redhat.com/ubi8/ubi:latest
+
+RUN useradd preflightuser
+
+COPY --chown=preflightuser:preflightuser example-license.txt /licenses/
+
+LABEL name="preflight test image" \ 
+      vendor="preflight test vendor" \
+      version="1" \
+      release="1" \
+      summary="testing the preflight tool" \
+      description="test the preflight tool"
+
+RUN find / -xdev -perm -4000 -exec chmod ug-s {} +
+
+USER preflightuser
+

test/containerfiles/container-passes-multiple-arch-rpm.Dockerfile

@@ -0,0 +1,17 @@
+FROM registry.access.redhat.com/ubi8/ubi:8.7-1112
+
+RUN useradd preflightuser
+
+COPY --chown=preflightuser:preflightuser example-license.txt /licenses/
+
+LABEL name="preflight test image container-policy" \
+      vendor="preflight test vendor" \
+      version="1" \
+      release="1" \
+      summary="testing the preflight tool" \
+      description="test the preflight tool"
+
+RUN dnf update glibc -y && dnf install glibc.i686 -y
+
+USER preflightuser
+