From df1d8d0e9353528708a87609b932ae6b833c7ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:57:43 +0200 Subject: [PATCH 01/24] Remove commented out code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can always recover it from git, but it seems to serve no purpose anyway. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 0e3d8fad97..cefe76da72 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -142,16 +142,12 @@ func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistri Transport: transport, Type: trustTypeDescription(repoval[0].Type), } - // TODO - keyarr is not used and I don't know its intent; commenting out for now for someone to fix later - // keyarr := []string{} uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - // keyarr = append(keyarr, repoele.KeyPath) uids = append(uids, trust.GetGPGIdFromKeyPath(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - // keyarr = append(keyarr, string(repoele.KeyData)) uids = append(uids, trust.GetGPGIdFromKeyData(repoele.KeyData)...) } } From 1d2def8d062588c46d066d54bba4284f72fec334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 22:34:23 +0200 Subject: [PATCH 02/24] Remove an unused trust.ShowOutput type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 663a1b5e2d..28dac1ab62 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -53,14 +53,6 @@ type RegistryNamespace struct { SigStoreStaging string `json:"sigstore-staging"` // For writing only. } -// ShowOutput keep the fields for image trust show command -type ShowOutput struct { - Repo string - Trusttype string - GPGid string - Sigstore string -} - // systemRegistriesDirPath is the path to registries.d. const systemRegistriesDirPath = "/etc/containers/registries.d" From 5be00f2270d4da30b0bffb15fca47f0325a963e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 21:06:29 +0200 Subject: [PATCH 03/24] Reorganize pkg/trust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the existing code into policy.go and registries.go, depending on which files it concerns. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/config.go | 12 -- pkg/trust/policy.go | 125 +++++++++++++++++++++ pkg/trust/registries.go | 121 ++++++++++++++++++++ pkg/trust/trust.go | 241 ++-------------------------------------- 4 files changed, 255 insertions(+), 244 deletions(-) delete mode 100644 pkg/trust/config.go create mode 100644 pkg/trust/policy.go create mode 100644 pkg/trust/registries.go diff --git a/pkg/trust/config.go b/pkg/trust/config.go deleted file mode 100644 index 6186d4cbd1..0000000000 --- a/pkg/trust/config.go +++ /dev/null @@ -1,12 +0,0 @@ -package trust - -// Policy describes a basic trust policy configuration -type Policy struct { - Transport string `json:"transport"` - Name string `json:"name,omitempty"` - RepoName string `json:"repo_name,omitempty"` - Keys []string `json:"keys,omitempty"` - SignatureStore string `json:"sigstore,omitempty"` - Type string `json:"type"` - GPGId string `json:"gpg_id,omitempty"` -} diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go new file mode 100644 index 0000000000..9c44f7da25 --- /dev/null +++ b/pkg/trust/policy.go @@ -0,0 +1,125 @@ +package trust + +import ( + "bufio" + "bytes" + "encoding/base64" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/containers/image/v5/types" + "github.com/sirupsen/logrus" +) + +// PolicyContent struct for policy.json file +type PolicyContent struct { + Default []RepoContent `json:"default"` + Transports TransportsContent `json:"transports,omitempty"` +} + +// RepoContent struct used under each repo +type RepoContent struct { + Type string `json:"type"` + KeyType string `json:"keyType,omitempty"` + KeyPath string `json:"keyPath,omitempty"` + KeyData string `json:"keyData,omitempty"` + SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` +} + +// RepoMap map repo name to policycontent for each repo +type RepoMap map[string][]RepoContent + +// TransportsContent struct for content under "transports" +type TransportsContent map[string]RepoMap + +// DefaultPolicyPath returns a path to the default policy of the system. +func DefaultPolicyPath(sys *types.SystemContext) string { + systemDefaultPolicyPath := "/etc/containers/policy.json" + if sys != nil { + if sys.SignaturePolicyPath != "" { + return sys.SignaturePolicyPath + } + if sys.RootForImplicitAbsolutePaths != "" { + return filepath.Join(sys.RootForImplicitAbsolutePaths, systemDefaultPolicyPath) + } + } + return systemDefaultPolicyPath +} + +// CreateTmpFile creates a temp file under dir and writes the content into it +func CreateTmpFile(dir, pattern string, content []byte) (string, error) { + tmpfile, err := ioutil.TempFile(dir, pattern) + if err != nil { + return "", err + } + defer tmpfile.Close() + + if _, err := tmpfile.Write(content); err != nil { + return "", err + } + return tmpfile.Name(), nil +} + +// GetGPGIdFromKeyPath return user keyring from key path +func GetGPGIdFromKeyPath(path string) []string { + cmd := exec.Command("gpg2", "--with-colons", path) + results, err := cmd.Output() + if err != nil { + logrus.Errorf("Getting key identity: %s", err) + return nil + } + return parseUids(results) +} + +// GetGPGIdFromKeyData return user keyring from keydata +func GetGPGIdFromKeyData(key string) []string { + decodeKey, err := base64.StdEncoding.DecodeString(key) + if err != nil { + logrus.Errorf("%s, error decoding key data", err) + return nil + } + tmpfileName, err := CreateTmpFile("", "", decodeKey) + if err != nil { + logrus.Errorf("Creating key date temp file %s", err) + } + defer os.Remove(tmpfileName) + return GetGPGIdFromKeyPath(tmpfileName) +} + +func parseUids(colonDelimitKeys []byte) []string { + var parseduids []string + scanner := bufio.NewScanner(bytes.NewReader(colonDelimitKeys)) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "uid:") || strings.HasPrefix(line, "pub:") { + uid := strings.Split(line, ":")[9] + if uid == "" { + continue + } + parseduid := uid + if strings.Contains(uid, "<") && strings.Contains(uid, ">") { + parseduid = strings.SplitN(strings.SplitAfterN(uid, "<", 2)[1], ">", 2)[0] + } + parseduids = append(parseduids, parseduid) + } + } + return parseduids +} + +// GetPolicy parse policy.json into PolicyContent struct +func GetPolicy(policyPath string) (PolicyContent, error) { + var policyContentStruct PolicyContent + policyContent, err := ioutil.ReadFile(policyPath) + if err != nil { + return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) + } + if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { + return policyContentStruct, fmt.Errorf("could not parse trust policies from %s: %w", policyPath, err) + } + return policyContentStruct, nil +} diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go new file mode 100644 index 0000000000..ba6ffe281d --- /dev/null +++ b/pkg/trust/registries.go @@ -0,0 +1,121 @@ +package trust + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/containers/image/v5/types" + "github.com/docker/docker/pkg/homedir" + "github.com/ghodss/yaml" +) + +// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. +// NOTE: Keep this in sync with docs/registries.d.md! +type RegistryConfiguration struct { + DefaultDocker *RegistryNamespace `json:"default-docker"` + // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), + Docker map[string]RegistryNamespace `json:"docker"` +} + +// RegistryNamespace defines lookaside locations for a single namespace. +type RegistryNamespace struct { + SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. + SigStoreStaging string `json:"sigstore-staging"` // For writing only. +} + +// systemRegistriesDirPath is the path to registries.d. +const systemRegistriesDirPath = "/etc/containers/registries.d" + +// userRegistriesDir is the path to the per user registries.d. +var userRegistriesDir = filepath.FromSlash(".config/containers/registries.d") + +// RegistriesDirPath returns a path to registries.d +func RegistriesDirPath(sys *types.SystemContext) string { + if sys != nil && sys.RegistriesDirPath != "" { + return sys.RegistriesDirPath + } + userRegistriesDirPath := filepath.Join(homedir.Get(), userRegistriesDir) + if _, err := os.Stat(userRegistriesDirPath); err == nil { + return userRegistriesDirPath + } + if sys != nil && sys.RootForImplicitAbsolutePaths != "" { + return filepath.Join(sys.RootForImplicitAbsolutePaths, systemRegistriesDirPath) + } + + return systemRegistriesDirPath +} + +// LoadAndMergeConfig loads configuration files in dirPath +func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { + mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} + dockerDefaultMergedFrom := "" + nsMergedFrom := map[string]string{} + + dir, err := os.Open(dirPath) + if err != nil { + if os.IsNotExist(err) { + return &mergedConfig, nil + } + return nil, err + } + configNames, err := dir.Readdirnames(0) + if err != nil { + return nil, err + } + for _, configName := range configNames { + if !strings.HasSuffix(configName, ".yaml") { + continue + } + configPath := filepath.Join(dirPath, configName) + configBytes, err := ioutil.ReadFile(configPath) + if err != nil { + return nil, err + } + var config RegistryConfiguration + err = yaml.Unmarshal(configBytes, &config) + if err != nil { + return nil, fmt.Errorf("error parsing %s: %w", configPath, err) + } + if config.DefaultDocker != nil { + if mergedConfig.DefaultDocker != nil { + return nil, fmt.Errorf(`error parsing signature storage configuration: "default-docker" defined both in "%s" and "%s"`, + dockerDefaultMergedFrom, configPath) + } + mergedConfig.DefaultDocker = config.DefaultDocker + dockerDefaultMergedFrom = configPath + } + for nsName, nsConfig := range config.Docker { // includes config.Docker == nil + if _, ok := mergedConfig.Docker[nsName]; ok { + return nil, fmt.Errorf(`error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`, + nsName, nsMergedFrom[nsName], configPath) + } + mergedConfig.Docker[nsName] = nsConfig + nsMergedFrom[nsName] = configPath + } + } + return &mergedConfig, nil +} + +// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file +func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { + searchKey := key + if !strings.Contains(searchKey, "/") { + val, exists := registryConfigs.Docker[searchKey] + if exists { + return &val + } + } + for range strings.Split(key, "/") { + val, exists := registryConfigs.Docker[searchKey] + if exists { + return &val + } + if strings.Contains(searchKey, "/") { + searchKey = searchKey[:strings.LastIndex(searchKey, "/")] + } + } + return registryConfigs.DefaultDocker +} diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 28dac1ab62..6186d4cbd1 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -1,235 +1,12 @@ package trust -import ( - "bufio" - "bytes" - "encoding/base64" - "encoding/json" - "fmt" - "io/ioutil" - "os" - "os/exec" - "path/filepath" - "strings" - - "github.com/containers/image/v5/types" - "github.com/docker/docker/pkg/homedir" - "github.com/ghodss/yaml" - "github.com/sirupsen/logrus" -) - -// PolicyContent struct for policy.json file -type PolicyContent struct { - Default []RepoContent `json:"default"` - Transports TransportsContent `json:"transports,omitempty"` -} - -// RepoContent struct used under each repo -type RepoContent struct { - Type string `json:"type"` - KeyType string `json:"keyType,omitempty"` - KeyPath string `json:"keyPath,omitempty"` - KeyData string `json:"keyData,omitempty"` - SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` -} - -// RepoMap map repo name to policycontent for each repo -type RepoMap map[string][]RepoContent - -// TransportsContent struct for content under "transports" -type TransportsContent map[string]RepoMap - -// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. -// NOTE: Keep this in sync with docs/registries.d.md! -type RegistryConfiguration struct { - DefaultDocker *RegistryNamespace `json:"default-docker"` - // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), - Docker map[string]RegistryNamespace `json:"docker"` -} - -// RegistryNamespace defines lookaside locations for a single namespace. -type RegistryNamespace struct { - SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. - SigStoreStaging string `json:"sigstore-staging"` // For writing only. -} - -// systemRegistriesDirPath is the path to registries.d. -const systemRegistriesDirPath = "/etc/containers/registries.d" - -// userRegistriesDir is the path to the per user registries.d. -var userRegistriesDir = filepath.FromSlash(".config/containers/registries.d") - -// DefaultPolicyPath returns a path to the default policy of the system. -func DefaultPolicyPath(sys *types.SystemContext) string { - systemDefaultPolicyPath := "/etc/containers/policy.json" - if sys != nil { - if sys.SignaturePolicyPath != "" { - return sys.SignaturePolicyPath - } - if sys.RootForImplicitAbsolutePaths != "" { - return filepath.Join(sys.RootForImplicitAbsolutePaths, systemDefaultPolicyPath) - } - } - return systemDefaultPolicyPath -} - -// RegistriesDirPath returns a path to registries.d -func RegistriesDirPath(sys *types.SystemContext) string { - if sys != nil && sys.RegistriesDirPath != "" { - return sys.RegistriesDirPath - } - userRegistriesDirPath := filepath.Join(homedir.Get(), userRegistriesDir) - if _, err := os.Stat(userRegistriesDirPath); err == nil { - return userRegistriesDirPath - } - if sys != nil && sys.RootForImplicitAbsolutePaths != "" { - return filepath.Join(sys.RootForImplicitAbsolutePaths, systemRegistriesDirPath) - } - - return systemRegistriesDirPath -} - -// LoadAndMergeConfig loads configuration files in dirPath -func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { - mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} - dockerDefaultMergedFrom := "" - nsMergedFrom := map[string]string{} - - dir, err := os.Open(dirPath) - if err != nil { - if os.IsNotExist(err) { - return &mergedConfig, nil - } - return nil, err - } - configNames, err := dir.Readdirnames(0) - if err != nil { - return nil, err - } - for _, configName := range configNames { - if !strings.HasSuffix(configName, ".yaml") { - continue - } - configPath := filepath.Join(dirPath, configName) - configBytes, err := ioutil.ReadFile(configPath) - if err != nil { - return nil, err - } - var config RegistryConfiguration - err = yaml.Unmarshal(configBytes, &config) - if err != nil { - return nil, fmt.Errorf("error parsing %s: %w", configPath, err) - } - if config.DefaultDocker != nil { - if mergedConfig.DefaultDocker != nil { - return nil, fmt.Errorf(`error parsing signature storage configuration: "default-docker" defined both in "%s" and "%s"`, - dockerDefaultMergedFrom, configPath) - } - mergedConfig.DefaultDocker = config.DefaultDocker - dockerDefaultMergedFrom = configPath - } - for nsName, nsConfig := range config.Docker { // includes config.Docker == nil - if _, ok := mergedConfig.Docker[nsName]; ok { - return nil, fmt.Errorf(`error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`, - nsName, nsMergedFrom[nsName], configPath) - } - mergedConfig.Docker[nsName] = nsConfig - nsMergedFrom[nsName] = configPath - } - } - return &mergedConfig, nil -} - -// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file -func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { - searchKey := key - if !strings.Contains(searchKey, "/") { - val, exists := registryConfigs.Docker[searchKey] - if exists { - return &val - } - } - for range strings.Split(key, "/") { - val, exists := registryConfigs.Docker[searchKey] - if exists { - return &val - } - if strings.Contains(searchKey, "/") { - searchKey = searchKey[:strings.LastIndex(searchKey, "/")] - } - } - return registryConfigs.DefaultDocker -} - -// CreateTmpFile creates a temp file under dir and writes the content into it -func CreateTmpFile(dir, pattern string, content []byte) (string, error) { - tmpfile, err := ioutil.TempFile(dir, pattern) - if err != nil { - return "", err - } - defer tmpfile.Close() - - if _, err := tmpfile.Write(content); err != nil { - return "", err - } - return tmpfile.Name(), nil -} - -// GetGPGIdFromKeyPath return user keyring from key path -func GetGPGIdFromKeyPath(path string) []string { - cmd := exec.Command("gpg2", "--with-colons", path) - results, err := cmd.Output() - if err != nil { - logrus.Errorf("Getting key identity: %s", err) - return nil - } - return parseUids(results) -} - -// GetGPGIdFromKeyData return user keyring from keydata -func GetGPGIdFromKeyData(key string) []string { - decodeKey, err := base64.StdEncoding.DecodeString(key) - if err != nil { - logrus.Errorf("%s, error decoding key data", err) - return nil - } - tmpfileName, err := CreateTmpFile("", "", decodeKey) - if err != nil { - logrus.Errorf("Creating key date temp file %s", err) - } - defer os.Remove(tmpfileName) - return GetGPGIdFromKeyPath(tmpfileName) -} - -func parseUids(colonDelimitKeys []byte) []string { - var parseduids []string - scanner := bufio.NewScanner(bytes.NewReader(colonDelimitKeys)) - for scanner.Scan() { - line := scanner.Text() - if strings.HasPrefix(line, "uid:") || strings.HasPrefix(line, "pub:") { - uid := strings.Split(line, ":")[9] - if uid == "" { - continue - } - parseduid := uid - if strings.Contains(uid, "<") && strings.Contains(uid, ">") { - parseduid = strings.SplitN(strings.SplitAfterN(uid, "<", 2)[1], ">", 2)[0] - } - parseduids = append(parseduids, parseduid) - } - } - return parseduids -} - -// GetPolicy parse policy.json into PolicyContent struct -func GetPolicy(policyPath string) (PolicyContent, error) { - var policyContentStruct PolicyContent - policyContent, err := ioutil.ReadFile(policyPath) - if err != nil { - return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) - } - if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { - return policyContentStruct, fmt.Errorf("could not parse trust policies from %s: %w", policyPath, err) - } - return policyContentStruct, nil +// Policy describes a basic trust policy configuration +type Policy struct { + Transport string `json:"transport"` + Name string `json:"name,omitempty"` + RepoName string `json:"repo_name,omitempty"` + Keys []string `json:"keys,omitempty"` + SignatureStore string `json:"sigstore,omitempty"` + Type string `json:"type"` + GPGId string `json:"gpg_id,omitempty"` } From 4c5366ee0306777d164dfa6ce02c97e29849d9f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 21:08:41 +0200 Subject: [PATCH 04/24] Make trust.CreateTempFile private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing uses it outside the package. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 9c44f7da25..62950131de 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -51,8 +51,8 @@ func DefaultPolicyPath(sys *types.SystemContext) string { return systemDefaultPolicyPath } -// CreateTmpFile creates a temp file under dir and writes the content into it -func CreateTmpFile(dir, pattern string, content []byte) (string, error) { +// createTmpFile creates a temp file under dir and writes the content into it +func createTmpFile(dir, pattern string, content []byte) (string, error) { tmpfile, err := ioutil.TempFile(dir, pattern) if err != nil { return "", err @@ -83,7 +83,7 @@ func GetGPGIdFromKeyData(key string) []string { logrus.Errorf("%s, error decoding key data", err) return nil } - tmpfileName, err := CreateTmpFile("", "", decodeKey) + tmpfileName, err := createTmpFile("", "", decodeKey) if err != nil { logrus.Errorf("Creating key date temp file %s", err) } From 4f68075306efb9381de3c5ea5762a3f843137b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:33:00 +0200 Subject: [PATCH 05/24] Add a variable for scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only process the incoming args[] (which is a single-element array for some reason) once, and use a semantic variable name for the value we care about. Should not change behavior, the only caller already supposedly ensures that len(args) == 1. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index cefe76da72..61bf037274 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -46,6 +46,11 @@ func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options ent } func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options entities.SetTrustOptions) error { + if len(args) != 1 { + return fmt.Errorf("SetTrust called with unexpected %d args", len(args)) + } + scope := args[0] + var ( policyContentStruct trust.PolicyContent newReposContent []trust.RepoContent @@ -81,7 +86,7 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } else { newReposContent = append(newReposContent, trust.RepoContent{Type: trustType}) } - if args[0] == "default" { + if scope == "default" { policyContentStruct.Default = newReposContent } else { if len(policyContentStruct.Default) == 0 { @@ -89,9 +94,9 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } registryExists := false for transport, transportval := range policyContentStruct.Transports { - _, registryExists = transportval[args[0]] + _, registryExists = transportval[scope] if registryExists { - policyContentStruct.Transports[transport][args[0]] = newReposContent + policyContentStruct.Transports[transport][scope] = newReposContent break } } @@ -102,7 +107,7 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti if policyContentStruct.Transports["docker"] == nil { policyContentStruct.Transports["docker"] = make(map[string][]trust.RepoContent) } - policyContentStruct.Transports["docker"][args[0]] = append(policyContentStruct.Transports["docker"][args[0]], newReposContent...) + policyContentStruct.Transports["docker"][scope] = append(policyContentStruct.Transports["docker"][scope], newReposContent...) } } From cbdbb025a3f6e6e5417cdade032075d679842056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 21:39:14 +0200 Subject: [PATCH 06/24] Move most of imageEngine.SetTrust to pkg/trust.AddPolicyEntries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Also, actually add a basic smoke test of the core functionality. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 68 +++----------------------------- pkg/trust/policy.go | 74 +++++++++++++++++++++++++++++++++++ pkg/trust/policy_test.go | 71 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 63 deletions(-) create mode 100644 pkg/trust/policy_test.go diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 61bf037274..381ea5debf 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -2,11 +2,8 @@ package abi import ( "context" - "encoding/json" - "errors" "fmt" "io/ioutil" - "os" "strings" "github.com/containers/podman/v4/pkg/domain/entities" @@ -51,71 +48,16 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti } scope := args[0] - var ( - policyContentStruct trust.PolicyContent - newReposContent []trust.RepoContent - ) - trustType := options.Type - if trustType == "accept" { - trustType = "insecureAcceptAnything" - } - - pubkeysfile := options.PubKeysFile - if len(pubkeysfile) == 0 && trustType == "signedBy" { - return errors.New("at least one public key must be defined for type 'signedBy'") - } - policyPath := trust.DefaultPolicyPath(ir.Libpod.SystemContext()) if len(options.PolicyPath) > 0 { policyPath = options.PolicyPath } - _, err := os.Stat(policyPath) - if !os.IsNotExist(err) { - policyContent, err := ioutil.ReadFile(policyPath) - if err != nil { - return err - } - if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { - return errors.New("could not read trust policies") - } - } - if len(pubkeysfile) != 0 { - for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, trust.RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) - } - } else { - newReposContent = append(newReposContent, trust.RepoContent{Type: trustType}) - } - if scope == "default" { - policyContentStruct.Default = newReposContent - } else { - if len(policyContentStruct.Default) == 0 { - return errors.New("default trust policy must be set") - } - registryExists := false - for transport, transportval := range policyContentStruct.Transports { - _, registryExists = transportval[scope] - if registryExists { - policyContentStruct.Transports[transport][scope] = newReposContent - break - } - } - if !registryExists { - if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]trust.RepoMap) - } - if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]trust.RepoContent) - } - policyContentStruct.Transports["docker"][scope] = append(policyContentStruct.Transports["docker"][scope], newReposContent...) - } - } - data, err := json.MarshalIndent(policyContentStruct, "", " ") - if err != nil { - return fmt.Errorf("error setting trust policy: %w", err) - } - return ioutil.WriteFile(policyPath, data, 0644) + return trust.AddPolicyEntries(policyPath, trust.AddPolicyEntriesInput{ + Scope: scope, + Type: options.Type, + PubKeyFiles: options.PubKeysFile, + }) } func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.Policy, error) { diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 62950131de..352be781c7 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -5,6 +5,7 @@ import ( "bytes" "encoding/base64" "encoding/json" + "errors" "fmt" "io/ioutil" "os" @@ -123,3 +124,76 @@ func GetPolicy(policyPath string) (PolicyContent, error) { } return policyContentStruct, nil } + +// AddPolicyEntriesInput collects some parameters to AddPolicyEntries, +// primarily so that the callers use named values instead of just strings in a sequence. +type AddPolicyEntriesInput struct { + Scope string // "default" or a docker/atomic scope name + Type string + PubKeyFiles []string // For signature enforcement types, paths to public keys files (where the image needs to be signed by at least one key from _each_ of the files). File format depends on Type. +} + +// AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. +func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { + var ( + policyContentStruct PolicyContent + newReposContent []RepoContent + ) + trustType := input.Type + if trustType == "accept" { + trustType = "insecureAcceptAnything" + } + + pubkeysfile := input.PubKeyFiles + if len(pubkeysfile) == 0 && trustType == "signedBy" { + return errors.New("at least one public key must be defined for type 'signedBy'") + } + + _, err := os.Stat(policyPath) + if !os.IsNotExist(err) { + policyContent, err := ioutil.ReadFile(policyPath) + if err != nil { + return err + } + if err := json.Unmarshal(policyContent, &policyContentStruct); err != nil { + return errors.New("could not read trust policies") + } + } + if len(pubkeysfile) != 0 { + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + } + } else { + newReposContent = append(newReposContent, RepoContent{Type: trustType}) + } + if input.Scope == "default" { + policyContentStruct.Default = newReposContent + } else { + if len(policyContentStruct.Default) == 0 { + return errors.New("default trust policy must be set") + } + registryExists := false + for transport, transportval := range policyContentStruct.Transports { + _, registryExists = transportval[input.Scope] + if registryExists { + policyContentStruct.Transports[transport][input.Scope] = newReposContent + break + } + } + if !registryExists { + if policyContentStruct.Transports == nil { + policyContentStruct.Transports = make(map[string]RepoMap) + } + if policyContentStruct.Transports["docker"] == nil { + policyContentStruct.Transports["docker"] = make(map[string][]RepoContent) + } + policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) + } + } + + data, err := json.MarshalIndent(policyContentStruct, "", " ") + if err != nil { + return fmt.Errorf("error setting trust policy: %w", err) + } + return ioutil.WriteFile(policyPath, data, 0644) +} diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go new file mode 100644 index 0000000000..1f2f585c8c --- /dev/null +++ b/pkg/trust/policy_test.go @@ -0,0 +1,71 @@ +package trust + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/containers/image/v5/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAddPolicyEntries(t *testing.T) { + tempDir := t.TempDir() + policyPath := filepath.Join(tempDir, "policy.json") + + minimalPolicy := &signature.Policy{ + Default: []signature.PolicyRequirement{ + signature.NewPRInsecureAcceptAnything(), + }, + } + minimalPolicyJSON, err := json.Marshal(minimalPolicy) + require.NoError(t, err) + err = os.WriteFile(policyPath, minimalPolicyJSON, 0600) + require.NoError(t, err) + + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "default", + Type: "reject", + }) + assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/accepted", + Type: "accept", + }) + assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/multi-signed", + Type: "signedBy", + PubKeyFiles: []string{"/1.pub", "/2.pub"}, + }) + assert.NoError(t, err) + + // Test that the outcome is consumable, and compare it with the expected values. + parsedPolicy, err := signature.NewPolicyFromFile(policyPath) + require.NoError(t, err) + assert.Equal(t, &signature.Policy{ + Default: signature.PolicyRequirements{ + signature.NewPRReject(), + }, + Transports: map[string]signature.PolicyTransportScopes{ + "docker": { + "quay.io/accepted": { + signature.NewPRInsecureAcceptAnything(), + }, + "quay.io/multi-signed": { + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + }, + }, parsedPolicy) +} + +// xNewPRSignedByKeyPath is a wrapper for NewPRSignedByKeyPath which must not fail. +func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSignedByKeyPath(signature.SBKeyTypeGPGKeys, keyPath, signedIdentity) + require.NoError(t, err) + return pr +} From e2d1bdd1d8c10617818e5805330c54523580b647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:39:11 +0200 Subject: [PATCH 07/24] Improve validation of data in ImageEngine.SetTrust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Also reject public keys with types that don't use them - Reject unknown trust types - And add unit tests Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 18 +++++++++++++++--- pkg/trust/policy_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 352be781c7..df4f49ff19 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -143,10 +143,22 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if trustType == "accept" { trustType = "insecureAcceptAnything" } - pubkeysfile := input.PubKeyFiles - if len(pubkeysfile) == 0 && trustType == "signedBy" { - return errors.New("at least one public key must be defined for type 'signedBy'") + + // The error messages in validation failures use input.Type instead of trustType to match the user’s input. + switch trustType { + case "insecureAcceptAnything", "reject": + if len(pubkeysfile) != 0 { + return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) + } + + case "signedBy": + if len(pubkeysfile) == 0 { + return errors.New("at least one public key must be defined for type 'signedBy'") + } + + default: + return fmt.Errorf("unknown trust type %q", input.Type) } _, err := os.Stat(policyPath) diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index 1f2f585c8c..c4781335fb 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -25,6 +25,38 @@ func TestAddPolicyEntries(t *testing.T) { err = os.WriteFile(policyPath, minimalPolicyJSON, 0600) require.NoError(t, err) + // Invalid input: + for _, invalid := range []AddPolicyEntriesInput{ + { + Scope: "default", + Type: "accept", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "insecureAcceptAnything", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "reject", + PubKeyFiles: []string{"/does-not-make-sense"}, + }, + { + Scope: "default", + Type: "signedBy", + PubKeyFiles: []string{}, // A key is missing + }, + { + Scope: "default", + Type: "this-is-unknown", + PubKeyFiles: []string{}, + }, + } { + err := AddPolicyEntries(policyPath, invalid) + assert.Error(t, err, "%#v", invalid) + } + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ Scope: "default", Type: "reject", From 9828bc44534d6527d44351470d5f943281b7dfba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:42:08 +0200 Subject: [PATCH 08/24] Create new policy entries together with validating input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That way, we don't have to switch over trustType twice. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index df4f49ff19..77e02a05c0 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -151,11 +151,15 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if len(pubkeysfile) != 0 { return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) } + newReposContent = append(newReposContent, RepoContent{Type: trustType}) case "signedBy": if len(pubkeysfile) == 0 { return errors.New("at least one public key must be defined for type 'signedBy'") } + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + } default: return fmt.Errorf("unknown trust type %q", input.Type) @@ -171,13 +175,6 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { return errors.New("could not read trust policies") } } - if len(pubkeysfile) != 0 { - for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) - } - } else { - newReposContent = append(newReposContent, RepoContent{Type: trustType}) - } if input.Scope == "default" { policyContentStruct.Default = newReposContent } else { From ff3f574fc0db5e442adfac54b86af7c462595ffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:48:26 +0200 Subject: [PATCH 09/24] Add support for sigstoreSigned in (podman image trust set) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOTE: This does not edit the use-sigstore-attachments value in registries.d, similarly to how (podman image trust set) didn't set the lookaside paths for simple signing. Signed-off-by: Miloslav Trmač --- cmd/podman/images/trust_set.go | 4 ++-- docs/source/markdown/podman-image-trust.1.md | 9 +++++--- pkg/trust/policy.go | 8 +++++++ pkg/trust/policy_test.go | 22 ++++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/cmd/podman/images/trust_set.go b/cmd/podman/images/trust_set.go index 832e9f7247..e7339f0b18 100644 --- a/cmd/podman/images/trust_set.go +++ b/cmd/podman/images/trust_set.go @@ -53,7 +53,7 @@ File(s) must exist before using this command`) } func setTrust(cmd *cobra.Command, args []string) error { - validTrustTypes := []string{"accept", "insecureAcceptAnything", "reject", "signedBy"} + validTrustTypes := []string{"accept", "insecureAcceptAnything", "reject", "signedBy", "sigstoreSigned"} valid, err := isValidImageURI(args[0]) if err != nil || !valid { @@ -61,7 +61,7 @@ func setTrust(cmd *cobra.Command, args []string) error { } if !util.StringInSlice(setOptions.Type, validTrustTypes) { - return fmt.Errorf("invalid choice: %s (choose from 'accept', 'reject', 'signedBy')", setOptions.Type) + return fmt.Errorf("invalid choice: %s (choose from 'accept', 'reject', 'signedBy', 'sigstoreSigned')", setOptions.Type) } return registry.ImageEngine().SetTrust(registry.Context(), args, setOptions) } diff --git a/docs/source/markdown/podman-image-trust.1.md b/docs/source/markdown/podman-image-trust.1.md index 4e80bdcf5f..2a7da82cc2 100644 --- a/docs/source/markdown/podman-image-trust.1.md +++ b/docs/source/markdown/podman-image-trust.1.md @@ -32,7 +32,8 @@ Trust **type** provides a way to: Allowlist ("accept") or Denylist ("reject") registries or -Require signature (“signedBy”). +Require a simple signing signature (“signedBy”), +Require a sigstore signature ("sigstoreSigned"). Trust may be updated using the command **podman image trust set** for an existing trust scope. @@ -45,12 +46,14 @@ Trust may be updated using the command **podman image trust set** for an existin #### **--pubkeysfile**, **-f**=*KEY1* A path to an exported public key on the local system. Key paths will be referenced in policy.json. Any path to a file may be used but locating the file in **/etc/pki/containers** is recommended. Options may be used multiple times to - require an image be signed by multiple keys. The **--pubkeysfile** option is required for the **signedBy** type. + require an image be signed by multiple keys. The **--pubkeysfile** option is required for the **signedBy** and **sigstoreSigned** types. #### **--type**, **-t**=*value* The trust type for this policy entry. Accepted values: - **signedBy** (default): Require signatures with corresponding list of + **signedBy** (default): Require simple signing signatures with corresponding list of + public keys + **sigstoreSigned**: Require sigstore signatures with corresponding list of public keys **accept**: do not require any signatures for this registry scope diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 77e02a05c0..3a31b93386 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -161,6 +161,14 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) } + case "sigstoreSigned": + if len(pubkeysfile) == 0 { + return errors.New("at least one public key must be defined for type 'sigstoreSigned'") + } + for _, filepath := range pubkeysfile { + newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyPath: filepath}) + } + default: return fmt.Errorf("unknown trust type %q", input.Type) } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index c4781335fb..c2c2d93beb 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -47,6 +47,11 @@ func TestAddPolicyEntries(t *testing.T) { Type: "signedBy", PubKeyFiles: []string{}, // A key is missing }, + { + Scope: "default", + Type: "sigstoreSigned", + PubKeyFiles: []string{}, // A key is missing + }, { Scope: "default", Type: "this-is-unknown", @@ -73,6 +78,12 @@ func TestAddPolicyEntries(t *testing.T) { PubKeyFiles: []string{"/1.pub", "/2.pub"}, }) assert.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + PubKeyFiles: []string{"/1.pub", "/2.pub"}, + }) + assert.NoError(t, err) // Test that the outcome is consumable, and compare it with the expected values. parsedPolicy, err := signature.NewPolicyFromFile(policyPath) @@ -90,6 +101,10 @@ func TestAddPolicyEntries(t *testing.T) { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "quay.io/sigstore-signed": { + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, }, }, }, parsedPolicy) @@ -101,3 +116,10 @@ func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signatur require.NoError(t, err) return pr } + +// xNewPRSigstoreSignedKeyPath is a wrapper for NewPRSigstoreSignedKeyPath which must not fail. +func xNewPRSigstoreSignedKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) + require.NoError(t, err) + return pr +} From 7723a1ea654624b5cfcedc6d94e947169967c183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 22:09:58 +0200 Subject: [PATCH 10/24] Move most of ImageEngine.ShowTrust into pkg/trust.PolicyDescription MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/domain/infra/abi/trust.go | 68 +---------------------------------- pkg/trust/policy.go | 10 ++++++ pkg/trust/trust.go | 68 +++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 67 deletions(-) diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 381ea5debf..c58ddff062 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -4,11 +4,9 @@ import ( "context" "fmt" "io/ioutil" - "strings" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/trust" - "github.com/sirupsen/logrus" ) func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options entities.ShowTrustOptions) (*entities.ShowTrustReport, error) { @@ -31,11 +29,7 @@ func (ir *ImageEngine) ShowTrust(ctx context.Context, args []string, options ent if len(options.RegistryPath) > 0 { report.SystemRegistriesDirPath = options.RegistryPath } - policyContentStruct, err := trust.GetPolicy(policyPath) - if err != nil { - return nil, fmt.Errorf("could not read trust policies: %w", err) - } - report.Policies, err = getPolicyShowOutput(policyContentStruct, report.SystemRegistriesDirPath) + report.Policies, err = trust.PolicyDescription(policyPath, report.SystemRegistriesDirPath) if err != nil { return nil, fmt.Errorf("could not show trust policies: %w", err) } @@ -59,63 +53,3 @@ func (ir *ImageEngine) SetTrust(ctx context.Context, args []string, options enti PubKeyFiles: options.PubKeysFile, }) } - -func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistriesDirPath string) ([]*trust.Policy, error) { - var output []*trust.Policy - - registryConfigs, err := trust.LoadAndMergeConfig(systemRegistriesDirPath) - if err != nil { - return nil, err - } - - if len(policyContentStruct.Default) > 0 { - defaultPolicyStruct := trust.Policy{ - Transport: "all", - Name: "* (default)", - RepoName: "default", - Type: trustTypeDescription(policyContentStruct.Default[0].Type), - } - output = append(output, &defaultPolicyStruct) - } - for transport, transval := range policyContentStruct.Transports { - if transport == "docker" { - transport = "repository" - } - - for repo, repoval := range transval { - tempTrustShowOutput := trust.Policy{ - Name: repo, - RepoName: repo, - Transport: transport, - Type: trustTypeDescription(repoval[0].Type), - } - uids := []string{} - for _, repoele := range repoval { - if len(repoele.KeyPath) > 0 { - uids = append(uids, trust.GetGPGIdFromKeyPath(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, trust.GetGPGIdFromKeyData(repoele.KeyData)...) - } - } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - - registryNamespace := trust.HaveMatchRegistry(repo, registryConfigs) - if registryNamespace != nil { - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore - } - output = append(output, &tempTrustShowOutput) - } - } - return output, nil -} - -var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} - -func trustTypeDescription(trustType string) string { - trustDescription, exist := typeDescription[trustType] - if !exist { - logrus.Warnf("Invalid trust type %s", trustType) - } - return trustDescription -} diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 3a31b93386..0dc46eac39 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -125,6 +125,16 @@ func GetPolicy(policyPath string) (PolicyContent, error) { return policyContentStruct, nil } +var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} + +func trustTypeDescription(trustType string) string { + trustDescription, exist := typeDescription[trustType] + if !exist { + logrus.Warnf("Invalid trust type %s", trustType) + } + return trustDescription +} + // AddPolicyEntriesInput collects some parameters to AddPolicyEntries, // primarily so that the callers use named values instead of just strings in a sequence. type AddPolicyEntriesInput struct { diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 6186d4cbd1..2813b126d2 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -1,5 +1,10 @@ package trust +import ( + "fmt" + "strings" +) + // Policy describes a basic trust policy configuration type Policy struct { Transport string `json:"transport"` @@ -10,3 +15,66 @@ type Policy struct { Type string `json:"type"` GPGId string `json:"gpg_id,omitempty"` } + +// PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. +func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { + policyContentStruct, err := GetPolicy(policyPath) + if err != nil { + return nil, fmt.Errorf("could not read trust policies: %w", err) + } + res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath) + if err != nil { + return nil, fmt.Errorf("could not show trust policies: %w", err) + } + return res, nil +} + +func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirPath string) ([]*Policy, error) { + var output []*Policy + + registryConfigs, err := LoadAndMergeConfig(systemRegistriesDirPath) + if err != nil { + return nil, err + } + + if len(policyContentStruct.Default) > 0 { + defaultPolicyStruct := Policy{ + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: trustTypeDescription(policyContentStruct.Default[0].Type), + } + output = append(output, &defaultPolicyStruct) + } + for transport, transval := range policyContentStruct.Transports { + if transport == "docker" { + transport = "repository" + } + + for repo, repoval := range transval { + tempTrustShowOutput := Policy{ + Name: repo, + RepoName: repo, + Transport: transport, + Type: trustTypeDescription(repoval[0].Type), + } + uids := []string{} + for _, repoele := range repoval { + if len(repoele.KeyPath) > 0 { + uids = append(uids, GetGPGIdFromKeyPath(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, GetGPGIdFromKeyData(repoele.KeyData)...) + } + } + tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + + registryNamespace := HaveMatchRegistry(repo, registryConfigs) + if registryNamespace != nil { + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } + output = append(output, &tempTrustShowOutput) + } + } + return output, nil +} From 35fa8c16a2e921ac7c45d6df3fa09a0ec6fdbbdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 22:36:38 +0200 Subject: [PATCH 11/24] Make most of pkg/trust package-private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now have only a few entrypoints that are called externally, so make the rest private. This will make it more obvious that we are not breaking any external users. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 51 +++++++++++++++++++++-------------------- pkg/trust/registries.go | 24 +++++++++---------- pkg/trust/trust.go | 12 +++++----- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 0dc46eac39..d2b904b07e 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -17,14 +17,15 @@ import ( "github.com/sirupsen/logrus" ) -// PolicyContent struct for policy.json file -type PolicyContent struct { - Default []RepoContent `json:"default"` - Transports TransportsContent `json:"transports,omitempty"` +// policyContent is the overall structure of a policy.json file (= c/image/v5/signature.Policy) +type policyContent struct { + Default []repoContent `json:"default"` + Transports transportsContent `json:"transports,omitempty"` } -// RepoContent struct used under each repo -type RepoContent struct { +// repoContent is a single policy requirement (one of possibly several for a scope), representing all of the individual alternatives in a single merged struct +// (= c/image/v5/signature.{PolicyRequirement,pr*}) +type repoContent struct { Type string `json:"type"` KeyType string `json:"keyType,omitempty"` KeyPath string `json:"keyPath,omitempty"` @@ -32,11 +33,11 @@ type RepoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } -// RepoMap map repo name to policycontent for each repo -type RepoMap map[string][]RepoContent +// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type repoMap map[string][]repoContent -// TransportsContent struct for content under "transports" -type TransportsContent map[string]RepoMap +// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) +type transportsContent map[string]repoMap // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { @@ -66,8 +67,8 @@ func createTmpFile(dir, pattern string, content []byte) (string, error) { return tmpfile.Name(), nil } -// GetGPGIdFromKeyPath return user keyring from key path -func GetGPGIdFromKeyPath(path string) []string { +// getGPGIdFromKeyPath returns GPG key IDs of keys stored at the provided path. +func getGPGIdFromKeyPath(path string) []string { cmd := exec.Command("gpg2", "--with-colons", path) results, err := cmd.Output() if err != nil { @@ -77,8 +78,8 @@ func GetGPGIdFromKeyPath(path string) []string { return parseUids(results) } -// GetGPGIdFromKeyData return user keyring from keydata -func GetGPGIdFromKeyData(key string) []string { +// getGPGIdFromKeyData returns GPG key IDs of keys in the provided keyring. +func getGPGIdFromKeyData(key string) []string { decodeKey, err := base64.StdEncoding.DecodeString(key) if err != nil { logrus.Errorf("%s, error decoding key data", err) @@ -89,7 +90,7 @@ func GetGPGIdFromKeyData(key string) []string { logrus.Errorf("Creating key date temp file %s", err) } defer os.Remove(tmpfileName) - return GetGPGIdFromKeyPath(tmpfileName) + return getGPGIdFromKeyPath(tmpfileName) } func parseUids(colonDelimitKeys []byte) []string { @@ -112,9 +113,9 @@ func parseUids(colonDelimitKeys []byte) []string { return parseduids } -// GetPolicy parse policy.json into PolicyContent struct -func GetPolicy(policyPath string) (PolicyContent, error) { - var policyContentStruct PolicyContent +// getPolicy parses policy.json into policyContent. +func getPolicy(policyPath string) (policyContent, error) { + var policyContentStruct policyContent policyContent, err := ioutil.ReadFile(policyPath) if err != nil { return policyContentStruct, fmt.Errorf("unable to read policy file: %w", err) @@ -146,8 +147,8 @@ type AddPolicyEntriesInput struct { // AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { var ( - policyContentStruct PolicyContent - newReposContent []RepoContent + policyContentStruct policyContent + newReposContent []repoContent ) trustType := input.Type if trustType == "accept" { @@ -161,14 +162,14 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { if len(pubkeysfile) != 0 { return fmt.Errorf("%d public keys unexpectedly provided for trust type %v", len(pubkeysfile), input.Type) } - newReposContent = append(newReposContent, RepoContent{Type: trustType}) + newReposContent = append(newReposContent, repoContent{Type: trustType}) case "signedBy": if len(pubkeysfile) == 0 { return errors.New("at least one public key must be defined for type 'signedBy'") } for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) + newReposContent = append(newReposContent, repoContent{Type: trustType, KeyType: "GPGKeys", KeyPath: filepath}) } case "sigstoreSigned": @@ -176,7 +177,7 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { return errors.New("at least one public key must be defined for type 'sigstoreSigned'") } for _, filepath := range pubkeysfile { - newReposContent = append(newReposContent, RepoContent{Type: trustType, KeyPath: filepath}) + newReposContent = append(newReposContent, repoContent{Type: trustType, KeyPath: filepath}) } default: @@ -209,10 +210,10 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { } if !registryExists { if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]RepoMap) + policyContentStruct.Transports = make(map[string]repoMap) } if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]RepoContent) + policyContentStruct.Transports["docker"] = make(map[string][]repoContent) } policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) } diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index ba6ffe281d..da2e7eb426 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -12,16 +12,16 @@ import ( "github.com/ghodss/yaml" ) -// RegistryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. +// registryConfiguration is one of the files in registriesDirPath configuring lookaside locations, or the result of merging them all. // NOTE: Keep this in sync with docs/registries.d.md! -type RegistryConfiguration struct { - DefaultDocker *RegistryNamespace `json:"default-docker"` +type registryConfiguration struct { + DefaultDocker *registryNamespace `json:"default-docker"` // The key is a namespace, using fully-expanded Docker reference format or parent namespaces (per dockerReference.PolicyConfiguration*), - Docker map[string]RegistryNamespace `json:"docker"` + Docker map[string]registryNamespace `json:"docker"` } -// RegistryNamespace defines lookaside locations for a single namespace. -type RegistryNamespace struct { +// registryNamespace defines lookaside locations for a single namespace. +type registryNamespace struct { SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. SigStoreStaging string `json:"sigstore-staging"` // For writing only. } @@ -48,9 +48,9 @@ func RegistriesDirPath(sys *types.SystemContext) string { return systemRegistriesDirPath } -// LoadAndMergeConfig loads configuration files in dirPath -func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { - mergedConfig := RegistryConfiguration{Docker: map[string]RegistryNamespace{}} +// loadAndMergeConfig loads registries.d configuration files in dirPath +func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { + mergedConfig := registryConfiguration{Docker: map[string]registryNamespace{}} dockerDefaultMergedFrom := "" nsMergedFrom := map[string]string{} @@ -74,7 +74,7 @@ func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { if err != nil { return nil, err } - var config RegistryConfiguration + var config registryConfiguration err = yaml.Unmarshal(configBytes, &config) if err != nil { return nil, fmt.Errorf("error parsing %s: %w", configPath, err) @@ -99,8 +99,8 @@ func LoadAndMergeConfig(dirPath string) (*RegistryConfiguration, error) { return &mergedConfig, nil } -// HaveMatchRegistry checks if trust settings for the registry have been configured in yaml file -func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *RegistryNamespace { +// haveMatchRegistry returns configuration from registryConfigs that is configured for key. +func haveMatchRegistry(key string, registryConfigs *registryConfiguration) *registryNamespace { searchKey := key if !strings.Contains(searchKey, "/") { val, exists := registryConfigs.Docker[searchKey] diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 2813b126d2..606e4ed934 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -18,7 +18,7 @@ type Policy struct { // PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { - policyContentStruct, err := GetPolicy(policyPath) + policyContentStruct, err := getPolicy(policyPath) if err != nil { return nil, fmt.Errorf("could not read trust policies: %w", err) } @@ -29,10 +29,10 @@ func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) return res, nil } -func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirPath string) ([]*Policy, error) { +func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string) ([]*Policy, error) { var output []*Policy - registryConfigs, err := LoadAndMergeConfig(systemRegistriesDirPath) + registryConfigs, err := loadAndMergeConfig(systemRegistriesDirPath) if err != nil { return nil, err } @@ -61,15 +61,15 @@ func getPolicyShowOutput(policyContentStruct PolicyContent, systemRegistriesDirP uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - uids = append(uids, GetGPGIdFromKeyPath(repoele.KeyPath)...) + uids = append(uids, getGPGIdFromKeyPath(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - uids = append(uids, GetGPGIdFromKeyData(repoele.KeyData)...) + uids = append(uids, getGPGIdFromKeyData(repoele.KeyData)...) } } tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - registryNamespace := HaveMatchRegistry(repo, registryConfigs) + registryNamespace := haveMatchRegistry(repo, registryConfigs) if registryNamespace != nil { tempTrustShowOutput.SignatureStore = registryNamespace.SigStore } From 4b2bd1036b4952a35a526202c8965cd3b32162ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 22:56:14 +0200 Subject: [PATCH 12/24] Make the output of (podman image trust show) deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sort map keys instead of iterating in the Go-imposed random order. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 606e4ed934..e93b4cd9d1 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -2,6 +2,7 @@ package trust import ( "fmt" + "sort" "strings" ) @@ -46,12 +47,26 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP } output = append(output, &defaultPolicyStruct) } - for transport, transval := range policyContentStruct.Transports { + // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. + transports := []string{} + for t := range policyContentStruct.Transports { + transports = append(transports, t) + } + sort.Strings(transports) + for _, transport := range transports { + transval := policyContentStruct.Transports[transport] if transport == "docker" { transport = "repository" } - for repo, repoval := range transval { + // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. + scopes := []string{} + for s := range transval { + scopes = append(scopes, s) + } + sort.Strings(scopes) + for _, repo := range scopes { + repoval := transval[repo] tempTrustShowOutput := Policy{ Name: repo, RepoName: repo, From 4df1e2524b9a8b3ff2d3768ac7fe54e98a966886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 22:56:54 +0200 Subject: [PATCH 13/24] Add a unit test for trust.PolicyDescription MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add at least a basic unit test for the various entry types. So that we don't have to actually deal with GPG keys and /usr/bin/gpg*, parametrize the code with a gpgIDReader , and pass a fake one in the unit test. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 8 ++- pkg/trust/testdata/default.yaml | 25 +++++++++ pkg/trust/testdata/redhat.yaml | 3 ++ pkg/trust/trust.go | 13 +++-- pkg/trust/trust_test.go | 92 +++++++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 pkg/trust/testdata/default.yaml create mode 100644 pkg/trust/testdata/redhat.yaml create mode 100644 pkg/trust/trust_test.go diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index d2b904b07e..7f32e2afc8 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -53,6 +53,10 @@ func DefaultPolicyPath(sys *types.SystemContext) string { return systemDefaultPolicyPath } +// gpgIDReader returns GPG key IDs of keys stored at the provided path. +// It exists only for tests, production code should always use getGPGIdFromKeyPath. +type gpgIDReader func(string) []string + // createTmpFile creates a temp file under dir and writes the content into it func createTmpFile(dir, pattern string, content []byte) (string, error) { tmpfile, err := ioutil.TempFile(dir, pattern) @@ -79,7 +83,7 @@ func getGPGIdFromKeyPath(path string) []string { } // getGPGIdFromKeyData returns GPG key IDs of keys in the provided keyring. -func getGPGIdFromKeyData(key string) []string { +func getGPGIdFromKeyData(idReader gpgIDReader, key string) []string { decodeKey, err := base64.StdEncoding.DecodeString(key) if err != nil { logrus.Errorf("%s, error decoding key data", err) @@ -90,7 +94,7 @@ func getGPGIdFromKeyData(key string) []string { logrus.Errorf("Creating key date temp file %s", err) } defer os.Remove(tmpfileName) - return getGPGIdFromKeyPath(tmpfileName) + return idReader(tmpfileName) } func parseUids(colonDelimitKeys []byte) []string { diff --git a/pkg/trust/testdata/default.yaml b/pkg/trust/testdata/default.yaml new file mode 100644 index 0000000000..31bcd35ef6 --- /dev/null +++ b/pkg/trust/testdata/default.yaml @@ -0,0 +1,25 @@ +# This is a default registries.d configuration file. You may +# add to this file or create additional files in registries.d/. +# +# lookaside: indicates a location that is read and write +# lookaside-staging: indicates a location that is only for write +# +# lookaside and lookaside-staging take a value of the following: +# lookaside: {schema}://location +# +# For reading signatures, schema may be http, https, or file. +# For writing signatures, schema may only be file. + +# This is the default signature write location for docker registries. +default-docker: +# lookaside: file:///var/lib/containers/sigstore + lookaside-staging: file:///var/lib/containers/sigstore + +# The 'docker' indicator here is the start of the configuration +# for docker registries. +# +# docker: +# +# privateregistry.com: +# lookaside: http://privateregistry.com/sigstore/ +# lookaside-staging: /mnt/nfs/privateregistry/sigstore diff --git a/pkg/trust/testdata/redhat.yaml b/pkg/trust/testdata/redhat.yaml new file mode 100644 index 0000000000..35f2c611c8 --- /dev/null +++ b/pkg/trust/testdata/redhat.yaml @@ -0,0 +1,3 @@ +docker: + registry.redhat.io: + sigstore: https://registry.redhat.io/containers/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index e93b4cd9d1..9dd6878f97 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -19,18 +19,23 @@ type Policy struct { // PolicyDescription returns an user-focused description of the policy in policyPath and registries.d data from registriesDirPath. func PolicyDescription(policyPath, registriesDirPath string) ([]*Policy, error) { + return policyDescriptionWithGPGIDReader(policyPath, registriesDirPath, getGPGIdFromKeyPath) +} + +// policyDescriptionWithGPGIDReader is PolicyDescription with a gpgIDReader parameter. It exists only to make testing easier. +func policyDescriptionWithGPGIDReader(policyPath, registriesDirPath string, idReader gpgIDReader) ([]*Policy, error) { policyContentStruct, err := getPolicy(policyPath) if err != nil { return nil, fmt.Errorf("could not read trust policies: %w", err) } - res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath) + res, err := getPolicyShowOutput(policyContentStruct, registriesDirPath, idReader) if err != nil { return nil, fmt.Errorf("could not show trust policies: %w", err) } return res, nil } -func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string) ([]*Policy, error) { +func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirPath string, idReader gpgIDReader) ([]*Policy, error) { var output []*Policy registryConfigs, err := loadAndMergeConfig(systemRegistriesDirPath) @@ -76,10 +81,10 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - uids = append(uids, getGPGIdFromKeyPath(repoele.KeyPath)...) + uids = append(uids, idReader(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(repoele.KeyData)...) + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } } tempTrustShowOutput.GPGId = strings.Join(uids, ", ") diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go new file mode 100644 index 0000000000..fc906572dc --- /dev/null +++ b/pkg/trust/trust_test.go @@ -0,0 +1,92 @@ +package trust + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/containers/image/v5/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPolicyDescription(t *testing.T) { + tempDir := t.TempDir() + policyPath := filepath.Join(tempDir, "policy.json") + + // Override getGPGIdFromKeyPath because we don't want to bother with (and spend the unit-test time on) generating valid GPG keys, and running the real GPG binary. + // Instead of reading the files at all, just expect file names like /id1,id2,...,idN.pub + idReader := func(keyPath string) []string { + require.True(t, strings.HasPrefix(keyPath, "/")) + require.True(t, strings.HasSuffix(keyPath, ".pub")) + return strings.Split(keyPath[1:len(keyPath)-4], ",") + } + + for _, c := range []struct { + policy *signature.Policy + expected []*Policy + }{ + { + &signature.Policy{ + Default: signature.PolicyRequirements{ + signature.NewPRReject(), + }, + Transports: map[string]signature.PolicyTransportScopes{ + "docker": { + "quay.io/accepted": { + signature.NewPRInsecureAcceptAnything(), + }, + "registry.redhat.io": { + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + "quay.io/multi-signed": { + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + }, + }, + []*Policy{ + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "reject", + }, + { + Transport: "repository", + Name: "quay.io/accepted", + RepoName: "quay.io/accepted", + Type: "accept", + }, + { + Transport: "repository", + Name: "quay.io/multi-signed", + RepoName: "quay.io/multi-signed", + Type: "signed", + SignatureStore: "", + GPGId: "1, 2, 3", + }, + { + Transport: "repository", + Name: "registry.redhat.io", + RepoName: "registry.redhat.io", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + }, + }, + } { + policyJSON, err := json.Marshal(c.policy) + require.NoError(t, err) + err = os.WriteFile(policyPath, policyJSON, 0600) + require.NoError(t, err) + + res, err := policyDescriptionWithGPGIDReader(policyPath, "./testdata", idReader) + require.NoError(t, err) + assert.Equal(t, c.expected, res) + } +} From d4c5217280a420aa28e9f9d116989f419dc427a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 19:56:37 +0200 Subject: [PATCH 14/24] Recognize the new lookaside names for simple signing sigstore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 6 ++++-- pkg/trust/testdata/quay.io.yaml | 3 +++ pkg/trust/trust.go | 6 +++++- pkg/trust/trust_test.go | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 pkg/trust/testdata/quay.io.yaml diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index da2e7eb426..23de8b1e33 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -22,8 +22,10 @@ type registryConfiguration struct { // registryNamespace defines lookaside locations for a single namespace. type registryNamespace struct { - SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. - SigStoreStaging string `json:"sigstore-staging"` // For writing only. + Lookaside string `json:"lookaside"` // For reading, and if LookasideStaging is not present, for writing. + LookasideStaging string `json:"lookaside-staging"` // For writing only. + SigStore string `json:"sigstore"` // For reading, and if SigStoreStaging is not present, for writing. + SigStoreStaging string `json:"sigstore-staging"` // For writing only. } // systemRegistriesDirPath is the path to registries.d. diff --git a/pkg/trust/testdata/quay.io.yaml b/pkg/trust/testdata/quay.io.yaml new file mode 100644 index 0000000000..80071596d2 --- /dev/null +++ b/pkg/trust/testdata/quay.io.yaml @@ -0,0 +1,3 @@ +docker: + quay.io/multi-signed: + lookaside: https://quay.example.com/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 9dd6878f97..aaddcf93ed 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -91,7 +91,11 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP registryNamespace := haveMatchRegistry(repo, registryConfigs) if registryNamespace != nil { - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + if registryNamespace.Lookaside != "" { + tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } } output = append(output, &tempTrustShowOutput) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index fc906572dc..3ee49cc472 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -66,7 +66,7 @@ func TestPolicyDescription(t *testing.T) { Name: "quay.io/multi-signed", RepoName: "quay.io/multi-signed", Type: "signed", - SignatureStore: "", + SignatureStore: "https://quay.example.com/sigstore", GPGId: "1, 2, 3", }, { From 51064acc49127daf1e945b19fe859bcc67d840ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:07:18 +0200 Subject: [PATCH 15/24] Split descriptionsOfPolicyRequirements out of getPolicyShowOutput MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will evetually allow us to use it for the default scope as well, which currently uses a simplified version. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 52 ++++++++++++---------- pkg/trust/trust_test.go | 95 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 23 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index aaddcf93ed..2d6f1fb878 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -72,33 +72,39 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP sort.Strings(scopes) for _, repo := range scopes { repoval := transval[repo] - tempTrustShowOutput := Policy{ + template := Policy{ + Transport: transport, Name: repo, RepoName: repo, - Transport: transport, - Type: trustTypeDescription(repoval[0].Type), } - uids := []string{} - for _, repoele := range repoval { - if len(repoele.KeyPath) > 0 { - uids = append(uids, idReader(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) - } - } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") - - registryNamespace := haveMatchRegistry(repo, registryConfigs) - if registryNamespace != nil { - if registryNamespace.Lookaside != "" { - tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside - } else { // incl. registryNamespace.SigStore == "" - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore - } - } - output = append(output, &tempTrustShowOutput) + output = append(output, descriptionsOfPolicyRequirements(repoval, template, registryConfigs, repo, idReader)...) } } return output, nil } + +// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. +func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { + tempTrustShowOutput := template + tempTrustShowOutput.Type = trustTypeDescription(reqs[0].Type) + uids := []string{} + for _, repoele := range reqs { + if len(repoele.KeyPath) > 0 { + uids = append(uids, idReader(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + } + } + tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + + registryNamespace := haveMatchRegistry(scope, registryConfigs) + if registryNamespace != nil { + if registryNamespace.Lookaside != "" { + tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + } + } + return []*Policy{&tempTrustShowOutput} +} diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index 3ee49cc472..ef2d10061f 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -90,3 +90,98 @@ func TestPolicyDescription(t *testing.T) { assert.Equal(t, c.expected, res) } } + +func TestDescriptionsOfPolicyRequirements(t *testing.T) { + // Override getGPGIdFromKeyPath because we don't want to bother with (and spend the unit-test time on) generating valid GPG keys, and running the real GPG binary. + // Instead of reading the files at all, just expect file names like /id1,id2,...,idN.pub + idReader := func(keyPath string) []string { + require.True(t, strings.HasPrefix(keyPath, "/")) + require.True(t, strings.HasSuffix(keyPath, ".pub")) + return strings.Split(keyPath[1:len(keyPath)-4], ",") + } + + template := Policy{ + Transport: "transport", + Name: "name", + RepoName: "repoName", + } + registryConfigs, err := loadAndMergeConfig("./testdata") + require.NoError(t, err) + + for _, c := range []struct { + scope string + reqs signature.PolicyRequirements + expected []*Policy + }{ + { + "", + signature.PolicyRequirements{ + signature.NewPRReject(), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "reject", + }, + }, + }, + { + "quay.io/accepted", + signature.PolicyRequirements{ + signature.NewPRInsecureAcceptAnything(), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "accept", + }, + }, + }, + { + "registry.redhat.io", + signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + }, + }, + { + "quay.io/multi-signed", + signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "1, 2, 3", + }, + }, + }, + } { + reqsJSON, err := json.Marshal(c.reqs) + require.NoError(t, err) + var parsedRegs []repoContent + err = json.Unmarshal(reqsJSON, &parsedRegs) + require.NoError(t, err) + + res := descriptionsOfPolicyRequirements(parsedRegs, template, registryConfigs, c.scope, idReader) + assert.Equal(t, c.expected, res) + } +} From 1a97c4d9fa8d991625c2a6b5d9f353ef9cd5f6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:07:54 +0200 Subject: [PATCH 16/24] Rename tempTrustShowOutput to entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that it is the primary return value of a small function, the long name only makes reading harder. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 2d6f1fb878..dd42626485 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -85,8 +85,8 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP // descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { - tempTrustShowOutput := template - tempTrustShowOutput.Type = trustTypeDescription(reqs[0].Type) + entry := template + entry.Type = trustTypeDescription(reqs[0].Type) uids := []string{} for _, repoele := range reqs { if len(repoele.KeyPath) > 0 { @@ -96,15 +96,15 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } } - tempTrustShowOutput.GPGId = strings.Join(uids, ", ") + entry.GPGId = strings.Join(uids, ", ") registryNamespace := haveMatchRegistry(scope, registryConfigs) if registryNamespace != nil { if registryNamespace.Lookaside != "" { - tempTrustShowOutput.SignatureStore = registryNamespace.Lookaside + entry.SignatureStore = registryNamespace.Lookaside } else { // incl. registryNamespace.SigStore == "" - tempTrustShowOutput.SignatureStore = registryNamespace.SigStore + entry.SignatureStore = registryNamespace.SigStore } } - return []*Policy{&tempTrustShowOutput} + return []*Policy{&entry} } From b15afce551a521b6224cf0a0c5a29beb89556e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:27:44 +0200 Subject: [PATCH 17/24] Rename haveMatchRegistry to registriesDConfigurationForScope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just so that we don't have a boolean-named function returning a struct. Also reorder the parameters to have the container first, and the lookup key second. Shoud not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 18 +++++++++--------- pkg/trust/trust.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index 23de8b1e33..e179b61ac0 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -101,22 +101,22 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { return &mergedConfig, nil } -// haveMatchRegistry returns configuration from registryConfigs that is configured for key. -func haveMatchRegistry(key string, registryConfigs *registryConfiguration) *registryNamespace { - searchKey := key - if !strings.Contains(searchKey, "/") { - val, exists := registryConfigs.Docker[searchKey] +// registriesDConfigurationForScope returns registries.d configuration for the provided scope. +func registriesDConfigurationForScope(registryConfigs *registryConfiguration, scope string) *registryNamespace { + searchScope := scope + if !strings.Contains(searchScope, "/") { + val, exists := registryConfigs.Docker[searchScope] if exists { return &val } } - for range strings.Split(key, "/") { - val, exists := registryConfigs.Docker[searchKey] + for range strings.Split(scope, "/") { + val, exists := registryConfigs.Docker[searchScope] if exists { return &val } - if strings.Contains(searchKey, "/") { - searchKey = searchKey[:strings.LastIndex(searchKey, "/")] + if strings.Contains(searchScope, "/") { + searchScope = searchScope[:strings.LastIndex(searchScope, "/")] } } return registryConfigs.DefaultDocker diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index dd42626485..a9ce99dd33 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -98,7 +98,7 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } entry.GPGId = strings.Join(uids, ", ") - registryNamespace := haveMatchRegistry(scope, registryConfigs) + registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) if registryNamespace != nil { if registryNamespace.Lookaside != "" { entry.SignatureStore = registryNamespace.Lookaside From 2f6c145e86027da7ecf352331db70f5e688701b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:28:14 +0200 Subject: [PATCH 18/24] Use the full descriptionsOfPolicyRequirements for the default scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... instead of taking a shortcut, e.g. not listing any keys if they are required. Signed-off-by: Miloslav Trmač --- pkg/trust/registries.go | 27 +++++++++++++++------------ pkg/trust/trust.go | 7 +++---- pkg/trust/trust_test.go | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/pkg/trust/registries.go b/pkg/trust/registries.go index e179b61ac0..0adc38232c 100644 --- a/pkg/trust/registries.go +++ b/pkg/trust/registries.go @@ -102,21 +102,24 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) { } // registriesDConfigurationForScope returns registries.d configuration for the provided scope. +// scope can be "" to return only the global default configuration entry. func registriesDConfigurationForScope(registryConfigs *registryConfiguration, scope string) *registryNamespace { searchScope := scope - if !strings.Contains(searchScope, "/") { - val, exists := registryConfigs.Docker[searchScope] - if exists { - return &val - } - } - for range strings.Split(scope, "/") { - val, exists := registryConfigs.Docker[searchScope] - if exists { - return &val + if searchScope != "" { + if !strings.Contains(searchScope, "/") { + val, exists := registryConfigs.Docker[searchScope] + if exists { + return &val + } } - if strings.Contains(searchScope, "/") { - searchScope = searchScope[:strings.LastIndex(searchScope, "/")] + for range strings.Split(scope, "/") { + val, exists := registryConfigs.Docker[searchScope] + if exists { + return &val + } + if strings.Contains(searchScope, "/") { + searchScope = searchScope[:strings.LastIndex(searchScope, "/")] + } } } return registryConfigs.DefaultDocker diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index a9ce99dd33..7412fab206 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -44,13 +44,12 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP } if len(policyContentStruct.Default) > 0 { - defaultPolicyStruct := Policy{ + template := Policy{ Transport: "all", Name: "* (default)", RepoName: "default", - Type: trustTypeDescription(policyContentStruct.Default[0].Type), } - output = append(output, &defaultPolicyStruct) + output = append(output, descriptionsOfPolicyRequirements(policyContentStruct.Default, template, registryConfigs, "", idReader)...) } // FIXME: This should use x/exp/maps.Keys after we update to Go 1.18. transports := []string{} @@ -83,7 +82,7 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP return output, nil } -// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope in registryConfigs. +// descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope (which may be "") in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { entry := template entry.Type = trustTypeDescription(reqs[0].Type) diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index ef2d10061f..d04e9f2114 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -79,6 +79,24 @@ func TestPolicyDescription(t *testing.T) { }, }, }, + { + &signature.Policy{ + Default: signature.PolicyRequirements{ + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + }, + []*Policy{ + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "signed", + SignatureStore: "", + GPGId: "1, 2, 3", + }, + }, + }, } { policyJSON, err := json.Marshal(c.policy) require.NoError(t, err) From bba306788aba723d8555281eb07edd90a5890e64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:36:14 +0200 Subject: [PATCH 19/24] Reorganize descriptionsOfPolicyRequirements a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do the registries.d lookup once, separately from building an entry, so that we can share it across entries. Also prepare a separate res to allow adding multiple entries. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 7412fab206..7b1b798ca3 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -84,6 +84,18 @@ func getPolicyShowOutput(policyContentStruct policyContent, systemRegistriesDirP // descriptionsOfPolicyRequirements turns reqs into user-readable policy entries, with Transport/Name/Reponame coming from template, potentially looking up scope (which may be "") in registryConfigs. func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, registryConfigs *registryConfiguration, scope string, idReader gpgIDReader) []*Policy { + res := []*Policy{} + + var lookasidePath string + registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) + if registryNamespace != nil { + if registryNamespace.Lookaside != "" { + lookasidePath = registryNamespace.Lookaside + } else { // incl. registryNamespace.SigStore == "" + lookasidePath = registryNamespace.SigStore + } + } + entry := template entry.Type = trustTypeDescription(reqs[0].Type) uids := []string{} @@ -96,14 +108,9 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } } entry.GPGId = strings.Join(uids, ", ") + entry.SignatureStore = lookasidePath - registryNamespace := registriesDConfigurationForScope(registryConfigs, scope) - if registryNamespace != nil { - if registryNamespace.Lookaside != "" { - entry.SignatureStore = registryNamespace.Lookaside - } else { // incl. registryNamespace.SigStore == "" - entry.SignatureStore = registryNamespace.SigStore - } - } - return []*Policy{&entry} + res = append(res, &entry) + + return res } From b36a1d1b79d7579738430adfd0696c324c3dacc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:45:57 +0200 Subject: [PATCH 20/24] BREAKING CHANGE: Change how (podman image trust show) represents multiple requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently - the output uses the first entry's type, even if the requirements are different (notably signedBy + sigstoreSIgned) - all public keys IDs are collected to a single line, even if some of them are interchangeable, and some are required (e.g. two signedBy requirements could require an image to be signed by (redhatProd OR redhatBeta) AND (vendor1 OR vendor2) So, stop collapsing the requirements, and return a separate entry for each one. Multiple GPG IDs on a single line used to mean AND or OR, now they always mean AND. Signed-off-by: Miloslav Trmač --- pkg/trust/trust.go | 14 ++++---- pkg/trust/trust_test.go | 80 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 7b1b798ca3..5f292083f9 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -96,21 +96,21 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis } } - entry := template - entry.Type = trustTypeDescription(reqs[0].Type) - uids := []string{} for _, repoele := range reqs { + entry := template + entry.Type = trustTypeDescription(repoele.Type) + + uids := []string{} if len(repoele.KeyPath) > 0 { uids = append(uids, idReader(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } + entry.GPGId = strings.Join(uids, ", ") + entry.SignatureStore = lookasidePath + res = append(res, &entry) } - entry.GPGId = strings.Join(uids, ", ") - entry.SignatureStore = lookasidePath - - res = append(res, &entry) return res } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index d04e9f2114..edafeb5c1e 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -67,7 +67,15 @@ func TestPolicyDescription(t *testing.T) { RepoName: "quay.io/multi-signed", Type: "signed", SignatureStore: "https://quay.example.com/sigstore", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "repository", + Name: "quay.io/multi-signed", + RepoName: "quay.io/multi-signed", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "2, 3", }, { Transport: "repository", @@ -93,7 +101,15 @@ func TestPolicyDescription(t *testing.T) { RepoName: "default", Type: "signed", SignatureStore: "", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "all", + Name: "* (default)", + RepoName: "default", + Type: "signed", + SignatureStore: "", + GPGId: "2, 3", }, }, }, @@ -188,7 +204,65 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { RepoName: "repoName", Type: "signed", SignatureStore: "https://quay.example.com/sigstore", - GPGId: "1, 2, 3", + GPGId: "1", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://quay.example.com/sigstore", + GPGId: "2, 3", + }, + }, + }, + { // Multiple kinds of requirements are represented individually. + "registry.redhat.io", + signature.PolicyRequirements{ + signature.NewPRReject(), + signature.NewPRInsecureAcceptAnything(), + xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + Type: "reject", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + Type: "accept", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "1", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "2, 3", }, }, }, From 752eceaecc979627e998bee2dba8ee9ce47aa5cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 24 Aug 2022 20:51:13 +0200 Subject: [PATCH 21/24] Support (image trust show) for sigstoreSigned entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sigstoreSigned does not have GPG IDs, so we add N/A in that column. NOTE: this does not show the use-sigstore-attachments value from registries.d. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 2 +- pkg/trust/trust.go | 24 ++++++++++------ pkg/trust/trust_test.go | 62 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 7f32e2afc8..085f0076a1 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -130,7 +130,7 @@ func getPolicy(policyPath string) (policyContent, error) { return policyContentStruct, nil } -var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "reject": "reject"} +var typeDescription = map[string]string{"insecureAcceptAnything": "accept", "signedBy": "signed", "sigstoreSigned": "sigstoreSigned", "reject": "reject"} func trustTypeDescription(trustType string) string { trustDescription, exist := typeDescription[trustType] diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 5f292083f9..a27ce5a850 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -100,15 +100,23 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis entry := template entry.Type = trustTypeDescription(repoele.Type) - uids := []string{} - if len(repoele.KeyPath) > 0 { - uids = append(uids, idReader(repoele.KeyPath)...) - } - if len(repoele.KeyData) > 0 { - uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + var gpgIDString string + switch repoele.Type { + case "signedBy": + uids := []string{} + if len(repoele.KeyPath) > 0 { + uids = append(uids, idReader(repoele.KeyPath)...) + } + if len(repoele.KeyData) > 0 { + uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) + } + gpgIDString = strings.Join(uids, ", ") + + case "sigstoreSigned": + gpgIDString = "N/A" // We could potentially return key fingerprints here, but they would not be _GPG_ fingerprints. } - entry.GPGId = strings.Join(uids, ", ") - entry.SignatureStore = lookasidePath + entry.GPGId = gpgIDString + entry.SignatureStore = lookasidePath // We do this even for sigstoreSigned and things like type: reject, to show that the sigstore is being read. res = append(res, &entry) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index edafeb5c1e..58394e77b4 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -45,6 +45,10 @@ func TestPolicyDescription(t *testing.T) { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "quay.io/sigstore-signed": { + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, }, }, }, @@ -77,6 +81,22 @@ func TestPolicyDescription(t *testing.T) { SignatureStore: "https://quay.example.com/sigstore", GPGId: "2, 3", }, + { + Transport: "repository", + Name: "quay.io/sigstore-signed", + RepoName: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + { + Transport: "repository", + Name: "quay.io/sigstore-signed", + RepoName: "quay.io/sigstore-signed", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, { Transport: "repository", Name: "registry.redhat.io", @@ -215,6 +235,30 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { GPGId: "2, 3", }, }, + }, { + "quay.io/sigstore-signed", + signature.PolicyRequirements{ + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "", + GPGId: "N/A", + }, + }, }, { // Multiple kinds of requirements are represented individually. "registry.redhat.io", @@ -224,6 +268,8 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSigstoreSignedKeyPath(t, "/2.pub", signature.NewPRMMatchRepoDigestOrExact()), }, []*Policy{ { @@ -264,6 +310,22 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { SignatureStore: "https://registry.redhat.io/containers/sigstore", GPGId: "2, 3", }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "N/A", + }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "sigstoreSigned", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "N/A", + }, }, }, } { From a7e88c8dacd56d266173d3fa04c66eb1a964b332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 25 Aug 2022 01:02:13 +0200 Subject: [PATCH 22/24] Add support for showing keyPaths in (podman image trust show) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 1 + pkg/trust/policy_test.go | 7 +++++++ pkg/trust/testdata/redhat.yaml | 2 ++ pkg/trust/trust.go | 3 +++ pkg/trust/trust_test.go | 35 ++++++++++++++++++++++++++++++++++ 5 files changed, 48 insertions(+) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index 085f0076a1..a41982c13c 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -29,6 +29,7 @@ type repoContent struct { Type string `json:"type"` KeyType string `json:"keyType,omitempty"` KeyPath string `json:"keyPath,omitempty"` + KeyPaths []string `json:"keyPaths,omitempty"` KeyData string `json:"keyData,omitempty"` SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index c2c2d93beb..0f97217224 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -117,6 +117,13 @@ func xNewPRSignedByKeyPath(t *testing.T, keyPath string, signedIdentity signatur return pr } +// xNewPRSignedByKeyPaths is a wrapper for NewPRSignedByKeyPaths which must not fail. +func xNewPRSignedByKeyPaths(t *testing.T, keyPaths []string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { + pr, err := signature.NewPRSignedByKeyPaths(signature.SBKeyTypeGPGKeys, keyPaths, signedIdentity) + require.NoError(t, err) + return pr +} + // xNewPRSigstoreSignedKeyPath is a wrapper for NewPRSigstoreSignedKeyPath which must not fail. func xNewPRSigstoreSignedKeyPath(t *testing.T, keyPath string, signedIdentity signature.PolicyReferenceMatch) signature.PolicyRequirement { pr, err := signature.NewPRSigstoreSignedKeyPath(keyPath, signedIdentity) diff --git a/pkg/trust/testdata/redhat.yaml b/pkg/trust/testdata/redhat.yaml index 35f2c611c8..8e40a41749 100644 --- a/pkg/trust/testdata/redhat.yaml +++ b/pkg/trust/testdata/redhat.yaml @@ -1,3 +1,5 @@ docker: registry.redhat.io: sigstore: https://registry.redhat.io/containers/sigstore + registry.access.redhat.com: + sigstore: https://registry.redhat.io/containers/sigstore diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index a27ce5a850..07d144bc11 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -107,6 +107,9 @@ func descriptionsOfPolicyRequirements(reqs []repoContent, template Policy, regis if len(repoele.KeyPath) > 0 { uids = append(uids, idReader(repoele.KeyPath)...) } + for _, path := range repoele.KeyPaths { + uids = append(uids, idReader(path)...) + } if len(repoele.KeyData) > 0 { uids = append(uids, getGPGIdFromKeyData(idReader, repoele.KeyData)...) } diff --git a/pkg/trust/trust_test.go b/pkg/trust/trust_test.go index 58394e77b4..22b780fc99 100644 --- a/pkg/trust/trust_test.go +++ b/pkg/trust/trust_test.go @@ -41,6 +41,9 @@ func TestPolicyDescription(t *testing.T) { "registry.redhat.io": { xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), }, + "registry.access.redhat.com": { + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), + }, "quay.io/multi-signed": { xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), @@ -98,6 +101,13 @@ func TestPolicyDescription(t *testing.T) { GPGId: "N/A", }, { + Transport: "repository", + Name: "registry.access.redhat.com", + RepoName: "registry.access.redhat.com", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, { Transport: "repository", Name: "registry.redhat.io", RepoName: "registry.redhat.io", @@ -211,6 +221,22 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { }, }, }, + { + "registry.access.redhat.com", + signature.PolicyRequirements{ + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), + }, + []*Policy{ + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, + }, + }, { "quay.io/multi-signed", signature.PolicyRequirements{ @@ -266,6 +292,7 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { signature.NewPRReject(), signature.NewPRInsecureAcceptAnything(), xNewPRSignedByKeyPath(t, "/redhat.pub", signature.NewPRMMatchRepoDigestOrExact()), + xNewPRSignedByKeyPaths(t, []string{"/redhat.pub", "/redhat-beta.pub"}, signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyPath(t, "/2,3.pub", signature.NewPRMMatchRepoDigestOrExact()), xNewPRSigstoreSignedKeyPath(t, "/1.pub", signature.NewPRMMatchRepoDigestOrExact()), @@ -294,6 +321,14 @@ func TestDescriptionsOfPolicyRequirements(t *testing.T) { SignatureStore: "https://registry.redhat.io/containers/sigstore", GPGId: "redhat", }, + { + Transport: "transport", + Name: "name", + RepoName: "repoName", + Type: "signed", + SignatureStore: "https://registry.redhat.io/containers/sigstore", + GPGId: "redhat, redhat-beta", + }, { Transport: "transport", Name: "name", From ad0c785f8e0846112b301872ffc8a7ea276c82a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 25 Aug 2022 01:11:07 +0200 Subject: [PATCH 23/24] Reorganize the types in policy.go a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to go from top to bottom. Should not change behavior. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index a41982c13c..daa7698b2d 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -23,6 +23,12 @@ type policyContent struct { Transports transportsContent `json:"transports,omitempty"` } +// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) +type transportsContent map[string]repoMap + +// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type repoMap map[string][]repoContent + // repoContent is a single policy requirement (one of possibly several for a scope), representing all of the individual alternatives in a single merged struct // (= c/image/v5/signature.{PolicyRequirement,pr*}) type repoContent struct { @@ -34,12 +40,6 @@ type repoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } -// repoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) -type repoMap map[string][]repoContent - -// transportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports) -type transportsContent map[string]repoMap - // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { systemDefaultPolicyPath := "/etc/containers/policy.json" From 61fe95bb4fa7b6f83cd2a013877664db90cd773b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 25 Aug 2022 01:25:08 +0200 Subject: [PATCH 24/24] Preserve all unknown PolicyRequirement fields on (podman image trust set) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are unmarshaling and re-marshaling JSON, which can _silently_ drop data with the Go design decision.data. Try harder, by using json.RawMessage at least for the data we care about. Alternatively, this could use json.Decoder.DisallowUnknownFields. Signed-off-by: Miloslav Trmač --- pkg/trust/policy.go | 30 ++++++++++++++----- pkg/trust/policy_test.go | 64 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/pkg/trust/policy.go b/pkg/trust/policy.go index daa7698b2d..326fe17aff 100644 --- a/pkg/trust/policy.go +++ b/pkg/trust/policy.go @@ -40,6 +40,18 @@ type repoContent struct { SignedIdentity json.RawMessage `json:"signedIdentity,omitempty"` } +// genericPolicyContent is the overall structure of a policy.json file (= c/image/v5/signature.Policy), using generic data for individual requirements. +type genericPolicyContent struct { + Default json.RawMessage `json:"default"` + Transports genericTransportsContent `json:"transports,omitempty"` +} + +// genericTransportsContent contains policies for individual transports (= c/image/v5/signature.Policy.Transports), using generic data for individual requirements. +type genericTransportsContent map[string]genericRepoMap + +// genericRepoMap maps a scope name to requirements that apply to that scope (= c/image/v5/signature.PolicyTransportScopes) +type genericRepoMap map[string]json.RawMessage + // DefaultPolicyPath returns a path to the default policy of the system. func DefaultPolicyPath(sys *types.SystemContext) string { systemDefaultPolicyPath := "/etc/containers/policy.json" @@ -152,7 +164,7 @@ type AddPolicyEntriesInput struct { // AddPolicyEntries adds one or more policy entries necessary to implement AddPolicyEntriesInput. func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { var ( - policyContentStruct policyContent + policyContentStruct genericPolicyContent newReposContent []repoContent ) trustType := input.Type @@ -188,8 +200,12 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { default: return fmt.Errorf("unknown trust type %q", input.Type) } + newReposJSON, err := json.Marshal(newReposContent) + if err != nil { + return err + } - _, err := os.Stat(policyPath) + _, err = os.Stat(policyPath) if !os.IsNotExist(err) { policyContent, err := ioutil.ReadFile(policyPath) if err != nil { @@ -200,7 +216,7 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { } } if input.Scope == "default" { - policyContentStruct.Default = newReposContent + policyContentStruct.Default = json.RawMessage(newReposJSON) } else { if len(policyContentStruct.Default) == 0 { return errors.New("default trust policy must be set") @@ -209,18 +225,18 @@ func AddPolicyEntries(policyPath string, input AddPolicyEntriesInput) error { for transport, transportval := range policyContentStruct.Transports { _, registryExists = transportval[input.Scope] if registryExists { - policyContentStruct.Transports[transport][input.Scope] = newReposContent + policyContentStruct.Transports[transport][input.Scope] = json.RawMessage(newReposJSON) break } } if !registryExists { if policyContentStruct.Transports == nil { - policyContentStruct.Transports = make(map[string]repoMap) + policyContentStruct.Transports = make(map[string]genericRepoMap) } if policyContentStruct.Transports["docker"] == nil { - policyContentStruct.Transports["docker"] = make(map[string][]repoContent) + policyContentStruct.Transports["docker"] = make(map[string]json.RawMessage) } - policyContentStruct.Transports["docker"][input.Scope] = append(policyContentStruct.Transports["docker"][input.Scope], newReposContent...) + policyContentStruct.Transports["docker"][input.Scope] = json.RawMessage(newReposJSON) } } diff --git a/pkg/trust/policy_test.go b/pkg/trust/policy_test.go index 0f97217224..3952b72c39 100644 --- a/pkg/trust/policy_test.go +++ b/pkg/trust/policy_test.go @@ -108,6 +108,70 @@ func TestAddPolicyEntries(t *testing.T) { }, }, }, parsedPolicy) + + // Test that completely unknown JSON is preserved + jsonWithUnknownData := `{ + "default": [ + { + "type": "this is unknown", + "unknown field": "should be preserved" + } + ], + "transports": + { + "docker-daemon": + { + "": [{ + "type":"this is unknown 2", + "unknown field 2": "should be preserved 2" + }] + } + } +}` + err = os.WriteFile(policyPath, []byte(jsonWithUnknownData), 0600) + require.NoError(t, err) + err = AddPolicyEntries(policyPath, AddPolicyEntriesInput{ + Scope: "quay.io/innocuous", + Type: "signedBy", + PubKeyFiles: []string{"/1.pub"}, + }) + require.NoError(t, err) + updatedJSONWithUnknownData, err := os.ReadFile(policyPath) + require.NoError(t, err) + // Decode updatedJSONWithUnknownData so that this test does not depend on details of the encoding. + // To reduce noise in the constants below: + type a = []interface{} + type m = map[string]interface{} + var parsedUpdatedJSON m + err = json.Unmarshal(updatedJSONWithUnknownData, &parsedUpdatedJSON) + require.NoError(t, err) + assert.Equal(t, m{ + "default": a{ + m{ + "type": "this is unknown", + "unknown field": "should be preserved", + }, + }, + "transports": m{ + "docker-daemon": m{ + "": a{ + m{ + "type": "this is unknown 2", + "unknown field 2": "should be preserved 2", + }, + }, + }, + "docker": m{ + "quay.io/innocuous": a{ + m{ + "type": "signedBy", + "keyType": "GPGKeys", + "keyPath": "/1.pub", + }, + }, + }, + }, + }, parsedUpdatedJSON) } // xNewPRSignedByKeyPath is a wrapper for NewPRSignedByKeyPath which must not fail.