diff --git a/pkg/config/remote/uptane/client_test.go b/pkg/config/remote/uptane/client_test.go index fa222265a099ae..f3c17e3d041584 100644 --- a/pkg/config/remote/uptane/client_test.go +++ b/pkg/config/remote/uptane/client_test.go @@ -297,11 +297,11 @@ func newTestRepository(version int, configTargets data.TargetFiles, directorTarg repos.directorTimestampVersion = 20 + version repos.directorTargetsVersion = 200 + version repos.directorSnapshotVersion = 2000 + version - repos.configRoot = generateRoot(repos.configRootKey, version, repos.configTimestampKey, repos.configTargetsKey, repos.configSnapshotKey) + repos.configRoot = generateRoot(repos.configRootKey, version, repos.configTimestampKey, repos.configTargetsKey, repos.configSnapshotKey, nil) repos.configTargets = generateTargets(repos.configTargetsKey, 100+version, configTargets) repos.configSnapshot = generateSnapshot(repos.configSnapshotKey, 1000+version, repos.configTargetsVersion) repos.configTimestamp = generateTimestamp(repos.configTimestampKey, 10+version, repos.configSnapshotVersion, repos.configSnapshot) - repos.directorRoot = generateRoot(repos.directorRootKey, version, repos.directorTimestampKey, repos.directorTargetsKey, repos.directorSnapshotKey) + repos.directorRoot = generateRoot(repos.directorRootKey, version, repos.directorTimestampKey, repos.directorTargetsKey, repos.directorSnapshotKey, nil) repos.directorTargets = generateTargets(repos.directorTargetsKey, 200+version, directorTargets) repos.directorSnapshot = generateSnapshot(repos.directorSnapshotKey, 2000+version, repos.directorTargetsVersion) repos.directorTimestamp = generateTimestamp(repos.directorTimestampKey, 20+version, repos.directorSnapshotVersion, repos.directorSnapshot) @@ -326,7 +326,7 @@ func (r testRepositories) toUpdate() *pbgo.LatestConfigsResponse { } } -func generateRoot(key keys.Signer, version int, timestampKey keys.Signer, targetsKey keys.Signer, snapshotKey keys.Signer) []byte { +func generateRoot(key keys.Signer, version int, timestampKey keys.Signer, targetsKey keys.Signer, snapshotKey keys.Signer, previousRootKey keys.Signer) []byte { root := data.NewRoot() root.Version = version root.Expires = time.Now().Add(1 * time.Hour) @@ -350,7 +350,13 @@ func generateRoot(key keys.Signer, version int, timestampKey keys.Signer, target KeyIDs: snapshotKey.PublicData().IDs(), Threshold: 1, } - signedRoot, _ := sign.Marshal(&root, key) + + rootSigners := []keys.Signer{key} + if previousRootKey != nil { + rootSigners = append(rootSigners, previousRootKey) + } + + signedRoot, _ := sign.Marshal(&root, rootSigners...) serializedRoot, _ := json.Marshal(signedRoot) return serializedRoot } diff --git a/pkg/config/remote/uptane/partial_client.go b/pkg/config/remote/uptane/partial_client.go index 0fc4be4133341e..dab9024bd9590d 100644 --- a/pkg/config/remote/uptane/partial_client.go +++ b/pkg/config/remote/uptane/partial_client.go @@ -8,6 +8,7 @@ package uptane import ( "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -52,6 +53,7 @@ type PartialState struct { } // PartialClient is a partial uptane client +// (see https://uptane.github.io/papers/uptane-standard.1.2.0.html#rfc.section.5.4.4.1) type PartialClient struct { sync.Mutex @@ -68,6 +70,7 @@ type PartialClient struct { } // NewPartialClient creates a new partial uptane client +// (see https://uptane.github.io/papers/uptane-standard.1.2.0.html#rfc.section.5.4.4.1) func NewPartialClient() (*PartialClient, error) { localStore := client.MemoryLocalStore() err := localStore.SetMeta("root.json", json.RawMessage(meta.RootsDirector().Last())) @@ -167,7 +170,15 @@ func (c *PartialClient) Update(response *pbgo.ClientGetConfigsResponse) error { } err = c.validateAndUpdateTargets(response.Targets.Raw) if err != nil { - return err + if (errors.Is(err, verify.ErrInvalid) || + errors.As(err, &verify.ErrRoleThreshold{})) { + return fmt.Errorf( + "updating targets: %w", + &ErrInvalid{err.Error()}, + ) + } + + return fmt.Errorf("updating target: error with unexpected type (%T): %w", err, err) } c.targetFiles = response.TargetFiles for _, target := range response.TargetFiles { @@ -217,6 +228,16 @@ func (c *PartialClient) TargetFile(path string) ([]byte, error) { return c.targetFile(path) } +// ErrInvalid represents the Uptane client rejecting invalid data +// (malformed or not signed properly) +type ErrInvalid struct { + msg string +} + +func (err *ErrInvalid) Error() string { + return err.msg +} + func (c *PartialClient) targetFile(path string) ([]byte, error) { var targetFile *pbgo.File for _, target := range c.targetFiles { @@ -229,7 +250,7 @@ func (c *PartialClient) targetFile(path string) ([]byte, error) { } targetMeta, hasMeta := c.targetMetas[path] if !hasMeta { - return nil, fmt.Errorf("target file meta %s not found", path) + return nil, &ErrInvalid{fmt.Sprintf("target file meta %s not found", path)} } if len(targetMeta.HashAlgorithms()) == 0 { return nil, fmt.Errorf("target file %s has no hash", path) diff --git a/pkg/config/remote/uptane/partial_client_test.go b/pkg/config/remote/uptane/partial_client_test.go index 4f66b2e58623f7..96c53be2976ac2 100644 --- a/pkg/config/remote/uptane/partial_client_test.go +++ b/pkg/config/remote/uptane/partial_client_test.go @@ -1,11 +1,13 @@ package uptane import ( + "regexp" "testing" "github.com/DataDog/datadog-agent/pkg/config" "github.com/DataDog/datadog-agent/pkg/proto/pbgo" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/theupdateframework/go-tuf/data" ) @@ -20,10 +22,10 @@ func (r testRepositories) toPartialUpdate() *pbgo.ClientGetConfigsResponse { func TestPartialClientVerifyValid(t *testing.T) { target1content, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } - testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) config.Datadog.Set("remote_configuration.director_root", testRepository.directorRoot) config.Datadog.Set("remote_configuration.config_root", testRepository.configRoot) @@ -34,7 +36,7 @@ func TestPartialClientVerifyValid(t *testing.T) { targets, err := client.Targets() assert.NoError(t, err) assert.Equal(t, targets1, targets) - targetFile, err := client.TargetFile("datadog/2/APM_SAMPLING/id/1") + targetFile, err := client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.NoError(t, err) assert.Equal(t, target1content, targetFile) } @@ -42,7 +44,7 @@ func TestPartialClientVerifyValid(t *testing.T) { func TestPartialClientVerifyValidMissingTargetFile(t *testing.T) { _, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{}) @@ -56,17 +58,17 @@ func TestPartialClientVerifyValidMissingTargetFile(t *testing.T) { targets, err := client.Targets() assert.NoError(t, err) assert.Equal(t, targets1, targets) - _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/1") + _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.Error(t, err) } func TestPartialClientVerifyValidRotation(t *testing.T) { target1content, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } - testRepository1 := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository1 := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) config.Datadog.Set("remote_configuration.director_root", testRepository1.directorRoot) config.Datadog.Set("remote_configuration.config_root", testRepository1.configRoot) client, err := NewPartialClient() @@ -75,16 +77,16 @@ func TestPartialClientVerifyValidRotation(t *testing.T) { err = client.Update(testRepository1.toPartialUpdate()) assert.NoError(t, err) - testRepository2 := newTestRepository(2, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository2 := newTestRepository(2, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) testRepository2.directorRootVersion = testRepository1.directorRootVersion + 1 - testRepository2.directorRoot = generateRoot(testRepository1.directorRootKey, testRepository2.directorRootVersion, testRepository2.directorTimestampKey, testRepository2.directorTargetsKey, testRepository2.directorSnapshotKey) + testRepository2.directorRoot = generateRoot(testRepository1.directorRootKey, testRepository2.directorRootVersion, testRepository2.directorTimestampKey, testRepository2.directorTargetsKey, testRepository2.directorSnapshotKey, nil) err = client.Update(testRepository2.toPartialUpdate()) assert.NoError(t, err) targets, err := client.Targets() assert.NoError(t, err) assert.Equal(t, targets1, targets) - targetFile, err := client.TargetFile("datadog/2/APM_SAMPLING/id/1") + targetFile, err := client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.NoError(t, err) assert.Equal(t, target1content, targetFile) } @@ -92,10 +94,10 @@ func TestPartialClientVerifyValidRotation(t *testing.T) { func TestPartialClientVerifyInvalidTargetFile(t *testing.T) { _, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } - testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: []byte(`fakecontent`)}}) + testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: []byte(`fakecontent`)}}) config.Datadog.Set("remote_configuration.director_root", testRepository.directorRoot) config.Datadog.Set("remote_configuration.config_root", testRepository.configRoot) @@ -105,17 +107,17 @@ func TestPartialClientVerifyInvalidTargetFile(t *testing.T) { assert.Error(t, err) _, err = client.Targets() assert.Error(t, err) - _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/1") + _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.Error(t, err) } func TestPartialClientVerifyInvalidTargets(t *testing.T) { target1content, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } - testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) config.Datadog.Set("remote_configuration.director_root", testRepository.directorRoot) config.Datadog.Set("remote_configuration.config_root", testRepository.configRoot) @@ -127,17 +129,17 @@ func TestPartialClientVerifyInvalidTargets(t *testing.T) { assert.Error(t, err) _, err = client.Targets() assert.Error(t, err) - _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/1") + _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.Error(t, err) } func TestPartialClientVerifyInvalidRotation(t *testing.T) { target1content, target1 := generateTarget() targets1 := data.TargetFiles{ - "datadog/2/APM_SAMPLING/id/1": target1, + "datadog/2/APM_SAMPLING/id/config": target1, } - testRepository1 := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository1 := newTestRepository(1, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) config.Datadog.Set("remote_configuration.director_root", testRepository1.directorRoot) config.Datadog.Set("remote_configuration.config_root", testRepository1.configRoot) client, err := NewPartialClient() @@ -146,12 +148,177 @@ func TestPartialClientVerifyInvalidRotation(t *testing.T) { err = client.Update(testRepository1.toPartialUpdate()) assert.NoError(t, err) - testRepository2 := newTestRepository(2, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/1", Raw: target1content}}) + testRepository2 := newTestRepository(2, nil, targets1, []*pbgo.File{{Path: "datadog/2/APM_SAMPLING/id/config", Raw: target1content}}) err = client.Update(testRepository2.toPartialUpdate()) + // TODO in the whole file: use "assert.ErrorAs" with specific error type assert.Error(t, err) _, err = client.Targets() assert.Error(t, err) - _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/1") + _, err = client.TargetFile("datadog/2/APM_SAMPLING/id/config") assert.Error(t, err) } + +func TestPartialClientRootKeyRotation(t *testing.T) { + require := require.New(t) + + _, targetFileMeta := generateTarget() + directorTargetMetadata := data.TargetFiles{ + "datadog/2/APM_SAMPLING/id/config": targetFileMeta, + } + + repository1 := newTestRepository(1, nil, directorTargetMetadata, nil) + config.Datadog.Set("remote_configuration.director_root", repository1.directorRoot) + config.Datadog.Set("remote_configuration.config_root", repository1.configRoot) + + client, err := NewPartialClient() + require.NoError(err) + + err = client.Update(repository1.toPartialUpdate()) + require.NoError(err) + + repository2 := newTestRepository(2, nil, directorTargetMetadata, nil) + repository2.directorRootVersion = repository1.directorRootVersion + 1 + repository2.directorRoot = generateRoot( + repository2.directorRootKey, + repository2.directorRootVersion, + repository2.directorTimestampKey, + repository2.directorTargetsKey, + repository2.directorSnapshotKey, + // new root must be signed by old root + repository1.directorRootKey, + ) + + err = client.Update(repository2.toPartialUpdate()) + require.NoError(err) + + root, err := client.getRoot() + require.NoError(err) + assert.Equal(t, root.Roles["root"].KeyIDs[0], repository2.directorRootKey.PublicData().IDs()[0]) +} + +// TestPartialClientRejectsUnsignedTarget tests that the partial uptane client +// does not accept targets which are not listed in the targets metadata file +func TestPartialClientRejectsUnsignedTarget(t *testing.T) { + require := require.New(t) + + files := []*pbgo.File{ + {Path: "datadog/2/APM_SAMPLING/id/config", Raw: []byte("mAlIcIoUs cOnTeNt!!")}, + } + // malicious target has simply be added without a signature + directorTargetMetadata := data.TargetFiles{} + + repository := newTestRepository(1, nil, directorTargetMetadata, files) + config.Datadog.Set("remote_configuration.director_root", repository.directorRoot) + config.Datadog.Set("remote_configuration.config_root", repository.configRoot) + + client, err := NewPartialClient() + require.NoError(err) + + err = client.Update(repository.toPartialUpdate()) + errInvalid := &ErrInvalid{} + require.ErrorAs(err, &errInvalid) +} + +// TestPartialClientRejectsInvalidSignature tests that the partial uptane client +// rejects target metadata with an invalid signature +func TestPartialClientRejectsInvalidSignature(t *testing.T) { + require := require.New(t) + + _, targetFileMeta := generateTarget() + directorTargetMetadata := data.TargetFiles{ + "datadog/2/APM_SAMPLING/id/config": targetFileMeta, + } + + repository := newTestRepository(1, nil, directorTargetMetadata, nil) + config.Datadog.Set("remote_configuration.director_root", repository.directorRoot) + config.Datadog.Set("remote_configuration.config_root", repository.configRoot) + + // changing the signature to make it invalid + repository.directorTargets = regexp.MustCompile(`"sig":"[a-f0-9]{6}`). + ReplaceAll(repository.directorTargets, []byte(`"sig":"abcdef`)) + + client, err := NewPartialClient() + require.NoError(err) + + err = client.Update(repository.toPartialUpdate()) + errInvalid := &ErrInvalid{} + require.ErrorAs(err, &errInvalid) +} + +func TestPartialClientRejectsRevokedTargetsKey(t *testing.T) { + require := require.New(t) + + _, targetFileMeta := generateTarget() + directorTargetMetadata := data.TargetFiles{ + "datadog/2/APM_SAMPLING/id/config": targetFileMeta, + } + + repository1 := newTestRepository(1, nil, directorTargetMetadata, nil) + config.Datadog.Set("remote_configuration.director_root", repository1.directorRoot) + config.Datadog.Set("remote_configuration.config_root", repository1.configRoot) + + client, err := NewPartialClient() + require.NoError(err) + + err = client.Update(repository1.toPartialUpdate()) + require.NoError(err) + + repository2 := newTestRepository(2, nil, directorTargetMetadata, nil) + repository2.directorRootVersion = repository1.directorRootVersion + 1 + repository2.directorRoot = generateRoot( + repository1.directorRootKey, + repository2.directorRootVersion, + repository2.directorTimestampKey, + repository2.directorTargetsKey, + repository2.directorSnapshotKey, + nil, + ) + + // revoked top-level targets metadata + repository2.directorTargets = repository1.directorTargets + + err = client.Update(repository2.toPartialUpdate()) + errInvalid := &ErrInvalid{} + require.ErrorAs(err, &errInvalid) +} + +func TestPartialClientRejectsRevokedRootKey(t *testing.T) { + require := require.New(t) + + _, targetFileMeta := generateTarget() + directorTargetMetadata := data.TargetFiles{ + "datadog/2/APM_SAMPLING/id/config": targetFileMeta, + } + + repository1 := newTestRepository(1, nil, directorTargetMetadata, nil) + config.Datadog.Set("remote_configuration.director_root", repository1.directorRoot) + config.Datadog.Set("remote_configuration.config_root", repository1.configRoot) + + client, err := NewPartialClient() + require.NoError(err) + + err = client.Update(repository1.toPartialUpdate()) + require.NoError(err) + + repository2 := newTestRepository(2, nil, directorTargetMetadata, nil) + repository2.directorRootVersion = repository1.directorRootVersion + 1 + repository2.directorRoot = generateRoot( + repository2.directorRootKey, + repository2.directorRootVersion, + repository2.directorTimestampKey, + repository2.directorTargetsKey, + repository2.directorSnapshotKey, + // new root must be signed by old root + repository1.directorRootKey, + ) + + err = client.Update(repository2.toPartialUpdate()) + require.NoError(err) + + // "root.json" from repository1 is only signed by root key version 1, + // which should be now revoked + err = client.Update(repository1.toPartialUpdate()) + errInvalid := &ErrInvalid{} + require.ErrorAs(err, &errInvalid) +}