Enable GPG checking when installing/updating packages. (#216)

Currently, GPG signature checking is disabled when installing/updating
packages. This is less than ideal when downloading packages from
external sources such as PMC (packages.microsoft.com).

This change enables GPG checking by default for both the base image's
repo files and the repo files provided by the user. However, the user
can disable the GPG checks for their provided repo files by setting
`gpgcheck` and `repo_gpgcheck` to `0`.

GPG checking remains disabled for local directories since there is no
way for the user to provide a GPG public key. Also, local directories
are likely to contain a user's custom built packages anyway, which are
unlikely to be signed.

Author: cwize1

docs/imagecustomizer/api/cli.md

@@ -101,13 +101,27 @@ Can be one of:
 
   The RPMs may either be in the directory itself or any subdirectories.
 
+  GPG signature checking is disabled for local directories.
+  If you wish to enable GPG signature checking, then use a repo file instead and set the
+  `gpgkey` field within the repo file.
+
 - `*.repo` file path: A path to a RPM repo definition file.
 
   The file name extension must be `.repo`.
 
-  Note: This file is not installed in the image during customization.
-  If that is also needed, then use `AdditionalFiles` to place the repo file within
+  If the repo file's `baseurl` or `gpgkey` fields contain a `file://` URL, then the
+  host's directories pointed to by the URL will be bind mounted into the chroot
+  environment and the URL will be replaced with the chroot equivalent URL.
+
+  GPG signature checking is enabled by default.
+  If you wish to disable GPG checking, then set both `gpgcheck` and `repo_gpgcheck` to
+  `0` in the repo file.
+
+  The repo file will only be used during image customization and will not be added to
   the image.
+  If you want to add the repo file to the image, then use use
+  [additionalFiles](../api/configuration/os.md#additionalfiles-additionalfile) to place
+  the repo file under the `/etc/yum.repos.d` directory.
 
 This option can be specified multiple times.
 

toolkit/tools/internal/safemount/safemount.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 	"time"
 
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/logger"
@@ -16,9 +17,9 @@ import (
 )
 
 type Mount struct {
-	target     string
-	isMounted  bool
-	dirCreated bool
+	target           string
+	isMounted        bool
+	fileOrDirCreated bool
 }
 
 // Creates a new system mount.
@@ -47,13 +48,37 @@ func (m *Mount) newMountHelper(source, target, fstype string, flags uintptr, dat
 		source, target, fstype, flags, data)
 
 	if makeAndDeleteDir {
-		// Create the mount target directory.
-		err = os.MkdirAll(target, os.ModePerm)
+		sourceInfo, err := os.Stat(source)
 		if err != nil {
-			return fmt.Errorf("failed to create mount directory (%s):\n%w", target, err)
+			return fmt.Errorf("failed to stat mount source (%s):\n%w", source, err)
 		}
 
-		m.dirCreated = true
+		sourceIsFile := (sourceInfo.Mode() & os.ModeType) == 0
+		isBindMount := (flags & unix.MS_BIND) == unix.MS_BIND
+		if !sourceIsFile || !isBindMount {
+			// Create the mount target directory.
+			err = os.MkdirAll(target, os.ModePerm)
+			if err != nil {
+				return fmt.Errorf("failed to create mount directory (%s):\n%w", target, err)
+			}
+		} else {
+			// Mount is a file bind mount.
+			// So, create a placeholder file instead of a placeholder directory.
+
+			// Create parent directory.
+			err = os.MkdirAll(filepath.Dir(target), os.ModePerm)
+			if err != nil {
+				return fmt.Errorf("failed to create mount target parent directory (%s):\n%w", target, err)
+			}
+
+			// Create placeholder file.
+			err = os.WriteFile(target, []byte(nil), os.ModePerm)
+			if err != nil {
+				return fmt.Errorf("failed to create mount target file (%s):\n%w", target, err)
+			}
+		}
+
+		m.fileOrDirCreated = true
 	}
 
 	// Create the mount.
@@ -116,7 +141,7 @@ func (m *Mount) close(async bool) error {
 		m.isMounted = false
 	}
 
-	if m.dirCreated {
+	if m.fileOrDirCreated {
 		logger.Log.Debugf("Deleting directory (%s)", m.target)
 
 		// Note: Do not use `RemoveAll` here in case the unmount silently failed.
@@ -126,7 +151,7 @@ func (m *Mount) close(async bool) error {
 			return fmt.Errorf("failed to delete mount directory (%s):\n%w", m.target, err)
 		}
 
-		m.dirCreated = false
+		m.fileOrDirCreated = false
 	}
 
 	return nil

toolkit/tools/pkg/imagecustomizerlib/customizepackages.go

@@ -106,7 +106,7 @@ func addRemoveAndUpdatePackages(buildDir string, baseConfigPath string, config *
 
 func refreshTdnfMetadata(imageChroot *safechroot.Chroot) error {
 	tdnfArgs := []string{
-		"-v", "check-update", "--refresh", "--nogpgcheck", "--assumeyes",
+		"-v", "check-update", "--refresh", "--assumeyes",
 		"--setopt", fmt.Sprintf("reposdir=%s", rpmsMountParentDirInChroot),
 	}
 
@@ -168,7 +168,7 @@ func updateAllPackages(imageChroot *safechroot.Chroot) error {
 	logger.Log.Infof("Updating base image packages")
 
 	tdnfUpdateArgs := []string{
-		"-v", "update", "--nogpgcheck", "--assumeyes", "--cacheonly",
+		"-v", "update", "--assumeyes", "--cacheonly",
 		"--setopt", fmt.Sprintf("reposdir=%s", rpmsMountParentDirInChroot),
 	}
 
@@ -189,7 +189,7 @@ func installOrUpdatePackages(action string, allPackagesToAdd []string, imageChro
 	// Note: When using `--repofromdir`, tdnf will not use any default repos and will only use the last
 	// `--repofromdir` specified.
 	tdnfInstallArgs := []string{
-		"-v", action, "--nogpgcheck", "--assumeyes", "--cacheonly",
+		"-v", action, "--assumeyes", "--cacheonly",
 		"--setopt", fmt.Sprintf("reposdir=%s", rpmsMountParentDirInChroot),
 	}
 

toolkit/tools/pkg/imagecustomizerlib/customizepackages_test.go

@@ -132,12 +132,22 @@ func copyRpms(sourceDir string, targetDir string, excludePrefixes []string) erro
 	return nil
 }
 
-func TestCustomizeImagePackagesAddOfflineLocalRepo(t *testing.T) {
-	testTmpDir := filepath.Join(tmpDir, "TestCustomizeImagePackagesAddOfflineLocalRepo")
+func TestCustomizeImagePackagesAddOfflineLocalRepoWithGpgKey(t *testing.T) {
+	testCustomizeImagePackagesAddOfflineLocalRepoHelper(t, "TestCustomizeImagePackagesAddOfflineLocalRepoWithGpgKey",
+		true)
+}
+
+func TestCustomizeImagePackagesAddOfflineLocalRepoNoGpgKey(t *testing.T) {
+	testCustomizeImagePackagesAddOfflineLocalRepoHelper(t, "TestCustomizeImagePackagesAddOfflineLocalRepoNoGpgKey",
+		false)
+}
+
+func testCustomizeImagePackagesAddOfflineLocalRepoHelper(t *testing.T, testName string, withGpgKey bool) {
+	testTmpDir := filepath.Join(tmpDir, testName)
 
 	baseImage := checkSkipForCustomizeImage(t, baseImageTypeCoreEfi, baseImageVersionDefault)
 
-	downloadedRpmsRepoFile := getDownloadedRpmsRepoFile(t, "2.0")
+	downloadedRpmsRepoFile := getDownloadedRpmsRepoFile(t, "2.0", withGpgKey)
 	rpmSources := []string{downloadedRpmsRepoFile}
 
 	buildDir := filepath.Join(testTmpDir, "build")

toolkit/tools/pkg/imagecustomizerlib/main_test.go

@@ -140,8 +140,14 @@ func getDownloadedRpmsDir(t *testing.T, azureLinuxVersion string) string {
 	return downloadedRpmsDir
 }
 
-func getDownloadedRpmsRepoFile(t *testing.T, azureLinuxVersion string) string {
+func getDownloadedRpmsRepoFile(t *testing.T, azureLinuxVersion string, withGpgKey bool) string {
 	dir := getDownloadedRpmsDir(t, azureLinuxVersion)
-	repoFile := filepath.Join(dir, "../", fmt.Sprintf("rpms-%s.repo", azureLinuxVersion))
+
+	suffix := "nokey"
+	if withGpgKey {
+		suffix = "withkey"
+	}
+
+	repoFile := filepath.Join(dir, "../", fmt.Sprintf("rpms-%s-%s.repo", azureLinuxVersion, suffix))
 	return repoFile
 }

toolkit/tools/pkg/imagecustomizerlib/rpmsourcesmounts.go

@@ -147,13 +147,13 @@ func (m *rpmSourcesMounts) createRepoFromDirectory(rpmSource string, allReposCon
 	rpmSourceName := path.Base(rpmSource)
 
 	// Mount the directory.
-	mountTargetDirectoryInChroot, err := m.mountRpmsDirectory(rpmSourceName, rpmSource, imageChroot)
+	mountTargetDirectoryInChroot, err := m.mountRpmsSource("baseurl", rpmSourceName, rpmSource, imageChroot)
 	if err != nil {
 		return err
 	}
 
 	// Add local repo config.
-	err = appendLocalRepo(allReposConfig, mountTargetDirectoryInChroot)
+	err = appendLocalRepo(allReposConfig, mountTargetDirectoryInChroot, rpmSource)
 	if err != nil {
 		return fmt.Errorf("failed to append local repo config:\n%w", err)
 	}
@@ -181,25 +181,52 @@ func (m *rpmSourcesMounts) createRepoFromRepoConfig(rpmSource string, isHostConf
 		}
 
 		if isHostConfig {
-			baseUrlKey, err := repoConfig.GetKey("baseurl")
+			_, err = repoConfig.GetKey("baseurl")
 			if err != nil {
 				return fmt.Errorf("invalid repo config (%s):\n%w", rpmSource, err)
 			}
 
-			// Check if the repo points to a local directory.
-			baseurl := baseUrlKey.String()
-			filePath, hasFilePrefix := strings.CutPrefix(baseurl, "file://")
-			if hasFilePrefix {
-				// Mount the directory in the chroot.
-				rpmSourceName := path.Base(baseurl)
-				mountTargetDirectoryInChroot, err := m.mountRpmsDirectory(rpmSourceName, filePath, imageChroot)
+			gpgCheckKey, _ := repoConfig.GetKey("gpgcheck")
+			repoGpgCheckKey, _ := repoConfig.GetKey("repo_gpgcheck")
+			switch {
+			case gpgCheckKey != nil && gpgCheckKey.String() != "1":
+				logger.Log.Infof("GPG signature checking disabled for RPM repo (%s)", repoConfig.Name())
+
+			case repoGpgCheckKey != nil && repoGpgCheckKey.String() != "1":
+				logger.Log.Infof("GPG signature checking disabled for RPM repo metadata (%s)", repoConfig.Name())
+			}
+
+			for _, field := range []string{"baseurl", "gpgkey"} {
+				fieldKey, err := repoConfig.GetKey(field)
 				if err != nil {
-					return fmt.Errorf("failed mount repo config local directory (%s):\n%w", rpmSource, err)
+					// Field doesn't exist.
+					continue
 				}
 
-				// Change the baseurl to point to the bind mount directory.
-				newBaseurl := fmt.Sprintf("file://%s", mountTargetDirectoryInChroot)
-				repoConfig.Key("baseurl").SetValue(newBaseurl)
+				fieldValue := fieldKey.String()
+				values := strings.Fields(fieldValue)
+
+				newValues := []string(nil)
+				for _, value := range values {
+					// Check if the value points to a local file/directory.
+					filePath, hasFilePrefix := strings.CutPrefix(value, "file://")
+					if hasFilePrefix {
+						// Bind mount the file/directory in the chroot.
+						rpmSourceName := path.Base(value)
+						mountTargetDirectoryInChroot, err := m.mountRpmsSource(field, rpmSourceName, filePath,
+							imageChroot)
+						if err != nil {
+							return fmt.Errorf("failed mount repo config local file/directory (%s):\n%w", filePath, err)
+						}
+
+						// Change the value to point to the bind mount file/directory.
+						newValue := fmt.Sprintf("file://%s", mountTargetDirectoryInChroot)
+						newValues = append(newValues, newValue)
+					}
+				}
+
+				newFieldValue := strings.Join(newValues, " ")
+				repoConfig.Key(field).SetValue(newFieldValue)
 			}
 		}
 
@@ -213,18 +240,18 @@ func (m *rpmSourcesMounts) createRepoFromRepoConfig(rpmSource string, isHostConf
 	return nil
 }
 
-func (m *rpmSourcesMounts) mountRpmsDirectory(rpmSourceName string, rpmsDirectory string,
+func (m *rpmSourcesMounts) mountRpmsSource(fieldName string, sourceName string, sourcePath string,
 	imageChroot *safechroot.Chroot,
 ) (string, error) {
 	i := len(m.mounts)
-	targetName := fmt.Sprintf("%02d%s", i, rpmSourceName)
+	targetName := fmt.Sprintf("%02d-%s-%s", i, fieldName, sourceName)
 	mountTargetDirectoryInChroot := path.Join(rpmsMountParentDirInChroot, targetName)
 	mountTargetDirectory := path.Join(imageChroot.RootDir(), mountTargetDirectoryInChroot)
 
 	// Create a read-only bind mount.
-	mount, err := safemount.NewMount(rpmsDirectory, mountTargetDirectory, "", unix.MS_BIND|unix.MS_RDONLY, "", true)
+	mount, err := safemount.NewMount(sourcePath, mountTargetDirectory, "", unix.MS_BIND|unix.MS_RDONLY, "", true)
 	if err != nil {
-		return "", fmt.Errorf("failed to mount RPM source directory from (%s) to (%s):\n%w", rpmsDirectory, mountTargetDirectory, err)
+		return "", fmt.Errorf("failed to mount RPM source from (%s) to (%s):\n%w", sourcePath, mountTargetDirectory, err)
 	}
 
 	m.mounts = append(m.mounts, mount)
@@ -310,7 +337,7 @@ func getRpmSourceFileType(rpmSourcePath string) (string, error) {
 }
 
 // Add a local directory containing RPMs to the allrepos.repo file.
-func appendLocalRepo(iniFile *ini.File, mountTargetDirectoryInChroot string) error {
+func appendLocalRepo(iniFile *ini.File, mountTargetDirectoryInChroot string, rpmSource string) error {
 	repoName := filepath.Base(mountTargetDirectoryInChroot)
 	iniSection, err := iniFile.NewSection(repoName)
 	if err != nil {
@@ -334,6 +361,22 @@ func appendLocalRepo(iniFile *ini.File, mountTargetDirectoryInChroot string) err
 		return err
 	}
 
+	// Disable GPG checks for local directories.
+	// There is no API to specify the GPG public key to use to verify the packages in the local directories.
+	// Also, local directories are likely to contain a user's custom built packages, which are very unlikely to be
+	// signed. If a user does sign their own packages, then they can pass in a .repo file instead and set the 'gpgkey'
+	// field within the .repo file.
+	_, err = iniSection.NewKey("gpgcheck", "0")
+	if err != nil {
+		return err
+	}
+
+	_, err = iniSection.NewKey("repo_gpgcheck", "0")
+	if err != nil {
+		return err
+	}
+
+	logger.Log.Infof("GPG signature checking disabled for RPM repo (%s)", rpmSource)
 	return nil
 }
 

toolkit/tools/pkg/imagecustomizerlib/testdata/testrpms/download-test-rpms.sh

@@ -37,7 +37,8 @@ set -x
 
 DOWNLOADER_RPMS_DIRS="$SCRIPT_DIR/downloadedrpms"
 OUT_DIR="$DOWNLOADER_RPMS_DIRS/$IMAGE_VERSION"
-REPO_FILE="$DOWNLOADER_RPMS_DIRS/rpms-$IMAGE_VERSION.repo"
+REPO_WITH_KEY_FILE="$DOWNLOADER_RPMS_DIRS/rpms-$IMAGE_VERSION-withkey.repo"
+REPO_NO_KEY_FILE="$DOWNLOADER_RPMS_DIRS/rpms-$IMAGE_VERSION-nokey.repo"
 
 mkdir -p "$OUT_DIR"
 
@@ -50,14 +51,32 @@ docker build \
 # Extract the RPM files.
 docker run \
   --rm \
-   -v $OUT_DIR:/outdir:z \
+   -v $OUT_DIR:/rpmsdir:z \
    "$CONTAINER_TAG" \
-   cp -r /downloadedrpms/. "/outdir"
+   cp -r /downloadedrpms/. "/rpmsdir"
 
-# Create repo file.
-cat << EOF > "$REPO_FILE"
+docker run \
+  --rm \
+   -v $OUT_DIR:/rpmsdir:z \
+   "$CONTAINER_TAG" \
+   cp -r /etc/pki/rpm-gpg/. "/rpmsdir"
+
+# Create repo files.
+cat << EOF > "$REPO_WITH_KEY_FILE"
+[localrpms]
+name=Local RPMs repo
+baseurl=file://$OUT_DIR
+enabled=1
+gpgcheck=1
+repo_gpgcheck=0
+gpgkey=file://$OUT_DIR/MICROSOFT-RPM-GPG-KEY
+EOF
+
+cat << EOF > "$REPO_NO_KEY_FILE"
 [localrpms]
 name=Local RPMs repo
 baseurl=file://$OUT_DIR
 enabled=1
+gpgcheck=0
+repo_gpgcheck=0
 EOF

toolkit/tools/pkg/imagecustomizerlib/testdata/testrpms/downloader/Dockerfile

@@ -5,7 +5,7 @@ RUN tdnf update -y && \
    tdnf install -y dnf dnf-plugins-core createrepo_c
 
 # Download the RPMs needed by the following tests:
-# - TestCustomizeImageAddPackage
+# - TestCustomizeImagePackagesAddOfflineLocalRepo
 RUN dnf download -y --resolve --alldeps --destdir /downloadedrpms \
     jq \
     golang