diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index 274344452f..c47e174737 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -48,7 +48,7 @@ func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error { // If it is a trusted push we would like to find the target entry which match the // tag provided in the function and then do an AddTarget later. - target := &client.Target{} + notaryTarget := &client.Target{} // Count the times of calling for handleTarget, // if it is called more that once, that should be considered an error in a trusted push. cnt := 0 @@ -65,12 +65,12 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn if dgst, err := digest.Parse(pushResult.Digest); err == nil { h, err := hex.DecodeString(dgst.Hex()) if err != nil { - target = nil + notaryTarget = nil return } - target.Name = pushResult.Tag - target.Hashes = data.Hashes{string(dgst.Algorithm()): h} - target.Length = int64(pushResult.Size) + notaryTarget.Name = pushResult.Tag + notaryTarget.Hashes = data.Hashes{string(dgst.Algorithm()): h} + notaryTarget.Length = int64(pushResult.Size) } } } @@ -99,7 +99,7 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn return errors.Errorf("internal error: only one call to handleTarget expected") } - if target == nil { + if notaryTarget == nil { return errors.Errorf("no targets found, provide a specific tag in order to sign it") } @@ -134,10 +134,10 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn return trust.NotaryError(repoInfo.Name.Name(), err) } _, _ = fmt.Fprintf(ioStreams.Out(), "Finished initializing %q\n", repoInfo.Name.Name()) - err = repo.AddTarget(target, data.CanonicalTargetsRole) + err = repo.AddTarget(notaryTarget, data.CanonicalTargetsRole) case nil: // already initialized and we have successfully downloaded the latest metadata - err = AddTargetToAllSignableRoles(repo, target) + err = trust.AddToAllSignableRoles(repo, notaryTarget) default: return trust.NotaryError(repoInfo.Name.Name(), err) } @@ -155,19 +155,6 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn return nil } -// AddTargetToAllSignableRoles attempts to add the image target to all the top level delegation roles we can -// (based on whether we have the signing key and whether the role's path allows -// us to). -// If there are no delegation roles, we add to the targets role. -func AddTargetToAllSignableRoles(repo client.Repository, target *client.Target) error { - signableRoles, err := trust.GetSignableRoles(repo, target) - if err != nil { - return err - } - - return repo.AddTarget(target, signableRoles...) -} - // trustedPull handles content trust pulling of an image func trustedPull(ctx context.Context, cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth, opts PullOptions) error { refs, err := getTrustedPullTargets(cli, imgRefAndAuth) diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go deleted file mode 100644 index eba38cbb97..0000000000 --- a/cli/command/image/trust_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package image - -import ( - "testing" - - "github.com/docker/cli/cli/trust" - registrytypes "github.com/docker/docker/api/types/registry" - "github.com/theupdateframework/notary/client" - "github.com/theupdateframework/notary/passphrase" - "github.com/theupdateframework/notary/trustpinning" - "gotest.tools/v3/assert" - "gotest.tools/v3/env" -) - -func TestENVTrustServer(t *testing.T) { - env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "https://notary-test.example.com:5000"}) - indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} - output, err := trust.Server(indexInfo) - expectedStr := "https://notary-test.example.com:5000" - if err != nil || output != expectedStr { - t.Fatalf("Expected server to be %s, got %s", expectedStr, output) - } -} - -func TestHTTPENVTrustServer(t *testing.T) { - env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "http://notary-test.example.com:5000"}) - indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} - _, err := trust.Server(indexInfo) - if err == nil { - t.Fatal("Expected error with invalid scheme") - } -} - -func TestOfficialTrustServer(t *testing.T) { - indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: true} - output, err := trust.Server(indexInfo) - if err != nil || output != trust.NotaryServer { - t.Fatalf("Expected server to be %s, got %s", trust.NotaryServer, output) - } -} - -func TestNonOfficialTrustServer(t *testing.T) { - indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: false} - output, err := trust.Server(indexInfo) - expectedStr := "https://" + indexInfo.Name - if err != nil || output != expectedStr { - t.Fatalf("Expected server to be %s, got %s", expectedStr, output) - } -} - -func TestAddTargetToAllSignableRolesError(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) - assert.NilError(t, err) - target := client.Target{} - err = AddTargetToAllSignableRoles(notaryRepo, &target) - assert.Error(t, err, "client is offline") -} diff --git a/cli/command/trust/helpers.go b/cli/command/trust/helpers.go index 3d2ba4acc3..0a9ef6671f 100644 --- a/cli/command/trust/helpers.go +++ b/cli/command/trust/helpers.go @@ -47,3 +47,9 @@ func getOrGenerateRootKeyAndInitRepo(notaryRepo client.Repository) error { } return notaryRepo.Initialize([]string{rootKey.ID()}, data.CanonicalSnapshotRole) } + +const testPass = "password" + +func testPassRetriever(string, string, bool, int) (string, bool, error) { + return testPass, false, nil +} diff --git a/cli/command/trust/helpers_test.go b/cli/command/trust/helpers_test.go deleted file mode 100644 index 9ce19259d1..0000000000 --- a/cli/command/trust/helpers_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package trust - -import ( - "testing" - - "github.com/theupdateframework/notary/client" - "github.com/theupdateframework/notary/passphrase" - "github.com/theupdateframework/notary/trustpinning" - "gotest.tools/v3/assert" -) - -func TestGetOrGenerateNotaryKeyAndInitRepo(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) - assert.NilError(t, err) - - err = getOrGenerateRootKeyAndInitRepo(notaryRepo) - assert.Error(t, err, "client is offline") -} diff --git a/cli/command/trust/key_generate_test.go b/cli/command/trust/key_generate_test.go index 74fc3fed68..1efd1d3121 100644 --- a/cli/command/trust/key_generate_test.go +++ b/cli/command/trust/key_generate_test.go @@ -11,7 +11,6 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/internal/test" "github.com/theupdateframework/notary" - "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/trustmanager" tufutils "github.com/theupdateframework/notary/tuf/utils" "gotest.tools/v3/assert" @@ -51,11 +50,9 @@ func TestGenerateKeySuccess(t *testing.T) { pubKeyCWD := t.TempDir() privKeyStorageDir := t.TempDir() - const testPass = "password" - cannedPasswordRetriever := passphrase.ConstantRetriever(testPass) // generate a single key keyName := "alice" - privKeyFileStore, err := trustmanager.NewKeyFileStore(privKeyStorageDir, cannedPasswordRetriever) + privKeyFileStore, err := trustmanager.NewKeyFileStore(privKeyStorageDir, testPassRetriever) assert.NilError(t, err) pubKeyPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore) diff --git a/cli/command/trust/key_load_test.go b/cli/command/trust/key_load_test.go index b2233f282a..2c004d3884 100644 --- a/cli/command/trust/key_load_test.go +++ b/cli/command/trust/key_load_test.go @@ -12,7 +12,6 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/internal/test" "github.com/theupdateframework/notary" - "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/storage" "github.com/theupdateframework/notary/trustmanager" tufutils "github.com/theupdateframework/notary/tuf/utils" @@ -122,8 +121,6 @@ func TestLoadKeyFromPath(t *testing.T) { keyStorageDir := t.TempDir() - const passwd = "password" - cannedPasswordRetriever := passphrase.ConstantRetriever(passwd) keyFileStore, err := storage.NewPrivateKeyFileStorage(keyStorageDir, notary.KeyExtension) assert.NilError(t, err) privKeyImporters := []trustmanager.Importer{keyFileStore} @@ -133,7 +130,7 @@ func TestLoadKeyFromPath(t *testing.T) { assert.NilError(t, err) // import the key to our keyStorageDir - assert.Check(t, loadPrivKeyBytesToStore(privKeyBytes, privKeyImporters, privKeyFilepath, "signer-name", cannedPasswordRetriever)) + assert.Check(t, loadPrivKeyBytesToStore(privKeyBytes, privKeyImporters, privKeyFilepath, "signer-name", testPassRetriever)) // check that the appropriate ~//private/.key file exists expectedImportKeyPath := filepath.Join(keyStorageDir, notary.PrivDir, keyID+"."+notary.KeyExtension) @@ -151,7 +148,7 @@ func TestLoadKeyFromPath(t *testing.T) { // assert encrypted header assert.Check(t, is.Equal("ENCRYPTED PRIVATE KEY", keyPEM.Type)) - decryptedKey, err := tufutils.ParsePKCS8ToTufKey(keyPEM.Bytes, []byte(passwd)) + decryptedKey, err := tufutils.ParsePKCS8ToTufKey(keyPEM.Bytes, []byte(testPass)) assert.NilError(t, err) fixturePEM, _ := pem.Decode(keyBytes) assert.Check(t, is.DeepEqual(fixturePEM.Bytes, decryptedKey.Private())) @@ -213,8 +210,6 @@ func TestLoadPubKeyFailure(t *testing.T) { assert.NilError(t, os.WriteFile(pubKeyFilepath, pubKeyFixture, notary.PrivNoExecPerms)) keyStorageDir := t.TempDir() - const passwd = "password" - cannedPasswordRetriever := passphrase.ConstantRetriever(passwd) keyFileStore, err := storage.NewPrivateKeyFileStorage(keyStorageDir, notary.KeyExtension) assert.NilError(t, err) privKeyImporters := []trustmanager.Importer{keyFileStore} @@ -223,7 +218,7 @@ func TestLoadPubKeyFailure(t *testing.T) { assert.NilError(t, err) // import the key to our keyStorageDir - it should fail - err = loadPrivKeyBytesToStore(pubKeyBytes, privKeyImporters, pubKeyFilepath, "signer-name", cannedPasswordRetriever) + err = loadPrivKeyBytesToStore(pubKeyBytes, privKeyImporters, pubKeyFilepath, "signer-name", testPassRetriever) expected := fmt.Sprintf("provided file %s is not a supported private key - to add a signer's public key use docker trust signer add", pubKeyFilepath) assert.Error(t, err, expected) } diff --git a/cli/command/trust/revoke_test.go b/cli/command/trust/revoke_test.go index 6e2ebd66bf..da1e48ecf8 100644 --- a/cli/command/trust/revoke_test.go +++ b/cli/command/trust/revoke_test.go @@ -9,8 +9,6 @@ import ( "github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test/notary" "github.com/theupdateframework/notary/client" - "github.com/theupdateframework/notary/passphrase" - "github.com/theupdateframework/notary/trustpinning" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/golden" @@ -151,14 +149,6 @@ func TestTrustRevokeCommand(t *testing.T) { } } -func TestGetSignableRolesForTargetAndRemoveError(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) - assert.NilError(t, err) - target := client.Target{} - err = getSignableRolesForTargetAndRemove(target, notaryRepo) - assert.Error(t, err, "client is offline") -} - func TestRevokeTrustPromptTermination(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) diff --git a/cli/command/trust/sign.go b/cli/command/trust/sign.go index fa2192bef9..bf974d0dc0 100644 --- a/cli/command/trust/sign.go +++ b/cli/command/trust/sign.go @@ -116,7 +116,7 @@ func signAndPublishToTarget(out io.Writer, imgRefAndAuth trust.ImageRefAndAuth, if err != nil { return err } - err = image.AddTargetToAllSignableRoles(notaryRepo, &target) + err = trust.AddToAllSignableRoles(notaryRepo, &target) if err == nil { prettyPrintExistingSignatureInfo(out, existingSigInfo) err = notaryRepo.Publish() diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index 7e8d0a08e7..e24ffbba73 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -14,7 +14,6 @@ import ( "github.com/theupdateframework/notary" "github.com/theupdateframework/notary/client" "github.com/theupdateframework/notary/client/changelist" - "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/trustpinning" "github.com/theupdateframework/notary/tuf/data" "gotest.tools/v3/assert" @@ -22,8 +21,6 @@ import ( "gotest.tools/v3/skip" ) -const passwd = "password" - func TestTrustSignCommandErrors(t *testing.T) { testCases := []struct { name string @@ -83,7 +80,7 @@ func TestTrustSignCommandOfflineErrors(t *testing.T) { } func TestGetOrGenerateNotaryKey(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) // repo is empty, try making a root key @@ -126,7 +123,7 @@ func TestGetOrGenerateNotaryKey(t *testing.T) { func TestAddStageSigners(t *testing.T) { skip.If(t, runtime.GOOS == "windows", "FIXME: not supported currently") - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) // stage targets/user @@ -207,7 +204,7 @@ func TestAddStageSigners(t *testing.T) { } func TestGetSignedManifestHashAndSize(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) _, _, err = getSignedManifestHashAndSize(notaryRepo, "test") assert.Error(t, err, "client is offline") @@ -229,7 +226,7 @@ func TestGetReleasedTargetHashAndSize(t *testing.T) { } func TestCreateTarget(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) _, err = createTarget(notaryRepo, "") assert.Error(t, err, "no tag specified") @@ -238,7 +235,7 @@ func TestCreateTarget(t *testing.T) { } func TestGetExistingSignatureInfoForReleasedTag(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) _, err = getExistingSignatureInfoForReleasedTag(notaryRepo, "test") assert.Error(t, err, "client is offline") @@ -267,7 +264,7 @@ func TestSignCommandChangeListIsCleanedOnError(t *testing.T) { err := cmd.Execute() assert.Assert(t, err != nil) - notaryRepo, err := client.NewFileCachedRepository(tmpDir, "docker.io/library/ubuntu", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(tmpDir, "docker.io/library/ubuntu", "https://localhost", nil, testPassRetriever, trustpinning.TrustPinConfig{}) assert.NilError(t, err) cl, err := notaryRepo.GetChangelist() assert.NilError(t, err) diff --git a/cli/trust/trust.go b/cli/trust/trust.go index bb7e597aa5..62bbd8f833 100644 --- a/cli/trust/trust.go +++ b/cli/trust/trust.go @@ -40,10 +40,11 @@ var ( ActionsPullOnly = []string{"pull"} // ActionsPushAndPull defines the actions for read-write interactions with a Notary Repository ActionsPushAndPull = []string{"pull", "push"} - // NotaryServer is the endpoint serving the Notary trust server - NotaryServer = "https://notary.docker.io" ) +// NotaryServer is the endpoint serving the Notary trust server +const NotaryServer = "https://notary.docker.io" + // GetTrustDirectory returns the base trust directory name func GetTrustDirectory() string { return filepath.Join(config.Dir(), "trust") @@ -238,6 +239,20 @@ func NotaryError(repoName string, err error) error { return err } +// AddToAllSignableRoles attempts to add the image target to all the top level +// delegation roles we can (based on whether we have the signing key and whether +// the role's path allows us to). +// +// If there are no delegation roles, we add to the targets role. +func AddToAllSignableRoles(repo client.Repository, target *client.Target) error { + signableRoles, err := GetSignableRoles(repo, target) + if err != nil { + return err + } + + return repo.AddTarget(target, signableRoles...) +} + // GetSignableRoles returns a list of roles for which we have valid signing // keys, given a notary repository and a target func GetSignableRoles(repo client.Repository, target *client.Target) ([]data.RoleName, error) { diff --git a/cli/trust/trust_test.go b/cli/trust/trust_test.go index 771b4291df..18531bf4c9 100644 --- a/cli/trust/trust_test.go +++ b/cli/trust/trust_test.go @@ -4,9 +4,9 @@ import ( "testing" "github.com/distribution/reference" + registrytypes "github.com/docker/docker/api/types/registry" "github.com/opencontainers/go-digest" "github.com/theupdateframework/notary/client" - "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/trustpinning" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -47,9 +47,42 @@ func TestGetDigest(t *testing.T) { } func TestGetSignableRolesError(t *testing.T) { - notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + notaryRepo, err := client.NewFileCachedRepository(t.TempDir(), "gun", "https://localhost", nil, nil, trustpinning.TrustPinConfig{}) assert.NilError(t, err) - target := client.Target{} - _, err = GetSignableRoles(notaryRepo, &target) - assert.Error(t, err, "client is offline") + _, err = GetSignableRoles(notaryRepo, &client.Target{}) + const expected = "client is offline" + assert.Error(t, err, expected) +} + +func TestENVTrustServer(t *testing.T) { + t.Setenv("DOCKER_CONTENT_TRUST_SERVER", "https://notary-test.example.com:5000") + indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} + output, err := Server(indexInfo) + const expected = "https://notary-test.example.com:5000" + assert.NilError(t, err) + assert.Equal(t, output, expected) +} + +func TestHTTPENVTrustServer(t *testing.T) { + t.Setenv("DOCKER_CONTENT_TRUST_SERVER", "http://notary-test.example.com:5000") + indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} + _, err := Server(indexInfo) + const expected = "valid https URL required for trust server" + assert.ErrorContains(t, err, expected, "Expected error with invalid scheme") +} + +func TestOfficialTrustServer(t *testing.T) { + indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: true} + output, err := Server(indexInfo) + const expected = NotaryServer + assert.NilError(t, err) + assert.Equal(t, output, expected) +} + +func TestNonOfficialTrustServer(t *testing.T) { + indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: false} + output, err := Server(indexInfo) + const expected = "https://testserver" + assert.NilError(t, err) + assert.Equal(t, output, expected) } diff --git a/internal/test/notary/client.go b/internal/test/notary/client.go index 3ee625fd3b..a948ad62cf 100644 --- a/internal/test/notary/client.go +++ b/internal/test/notary/client.go @@ -5,7 +5,6 @@ import ( "github.com/theupdateframework/notary/client" "github.com/theupdateframework/notary/client/changelist" "github.com/theupdateframework/notary/cryptoservice" - "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/storage" "github.com/theupdateframework/notary/trustmanager" "github.com/theupdateframework/notary/tuf/data" @@ -494,11 +493,17 @@ func (LoadedNotaryRepository) GetDelegationRoles() ([]data.Role, error) { return loadedDelegationRoles, nil } +const testPass = "password" + +func testPassRetriever(string, string, bool, int) (string, bool, error) { + return testPass, false, nil +} + // GetCryptoService is the getter for the repository's CryptoService func (l LoadedNotaryRepository) GetCryptoService() signed.CryptoService { if l.statefulCryptoService == nil { // give it an in-memory cryptoservice with a root key and targets key - l.statefulCryptoService = cryptoservice.NewCryptoService(trustmanager.NewKeyMemoryStore(passphrase.ConstantRetriever("password"))) + l.statefulCryptoService = cryptoservice.NewCryptoService(trustmanager.NewKeyMemoryStore(testPassRetriever)) l.statefulCryptoService.AddKey(data.CanonicalRootRole, l.GetGUN(), nil) l.statefulCryptoService.AddKey(data.CanonicalTargetsRole, l.GetGUN(), nil) }