From df1d8d0e9353528708a87609b932ae6b833c7ec0 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:57:43 +0200 Subject: 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(-) (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 0e3d8fad9..cefe76da7 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)...) } } -- cgit v1.2.3-54-g00ecf From 4f68075306efb9381de3c5ea5762a3f843137b56 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 19:33:00 +0200 Subject: 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(-) (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index cefe76da7..61bf03727 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...) } } -- cgit v1.2.3-54-g00ecf From cbdbb025a3f6e6e5417cdade032075d679842056 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 21:39:14 +0200 Subject: 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 (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 61bf03727..381ea5deb 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 62950131d..352be781c 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 000000000..1f2f585c8 --- /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 +} -- cgit v1.2.3-54-g00ecf From 7723a1ea654624b5cfcedc6d94e947169967c183 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Wed, 24 Aug 2022 22:09:58 +0200 Subject: 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(-) (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index 381ea5deb..c58ddff06 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 3a31b9338..0dc46eac3 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 6186d4cbd..2813b126d 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 +} -- cgit v1.2.3-54-g00ecf