diff --git a/internal/policy/policy.go b/internal/policy/policy.go index f55f9a033..69eb65da2 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -11,8 +11,6 @@ import ( "github.com/gittuf/gittuf/internal/gitinterface" "github.com/gittuf/gittuf/internal/rsl" - "github.com/gittuf/gittuf/internal/signerverifier" - "github.com/gittuf/gittuf/internal/signerverifier/dsse" "github.com/gittuf/gittuf/internal/tuf" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" @@ -282,6 +280,17 @@ func (s *State) PublicKeys() (map[string]*tuf.Key, error) { // FindAuthorizedSigningKeyIDs traverses the policy metadata to identify the // keys trusted to sign for the specified role. +// +// Deprecated: diamond delegations are legal in policy. So, role A and role B +// can both independently delegate to role C, and they *don't* need to specify +// the same set of keys / threshold. So, when signing role C, we actually can't +// determine if the keys being used to sign it are valid. It depends strictly on +// how role C is reached, whether via role A or role B. In turn, that depends on +// the exact namespace being verified. In TUF, this issue is known as +// "promiscuous delegations". See: +// https://github.com/theupdateframework/specification/issues/19, +// https://github.com/theupdateframework/specification/issues/214, and +// https://github.com/theupdateframework/python-tuf/issues/660. func (s *State) FindAuthorizedSigningKeyIDs(ctx context.Context, roleName string) ([]string, error) { if err := s.Verify(ctx); err != nil { return nil, err @@ -314,6 +323,8 @@ func (s *State) FindAuthorizedSigningKeyIDs(ctx context.Context, roleName string // FindPublicKeysForPath identifies the trusted keys for the path. If the path // protected in gittuf policy, the trusted keys are returned. +// +// Deprecated: use FindVerifiersForPath. func (s *State) FindPublicKeysForPath(ctx context.Context, path string) ([]*tuf.Key, error) { if err := s.Verify(ctx); err != nil { return nil, err @@ -365,11 +376,17 @@ func (s *State) FindPublicKeysForPath(ctx context.Context, path string) ([]*tuf. } } +// FindVerifiersForPath identifies the trusted set of verifiers for the +// specified path. While walking the delegation graph for the path, signatures +// for delegated metadata files are verified using the verifier context. func (s *State) FindVerifiersForPath(ctx context.Context, path string) ([]*Verifier, error) { - if err := s.Verify(ctx); err != nil { - return nil, err + if !s.HasTargetsRole(TargetsRoleName) { + // No policies exist + return nil, ErrMetadataNotFound } + // This envelope is verified when state is loaded, as this is + // the start for all delegation graph searches targetsMetadata, err := s.GetTargetsMetadata(TargetsRoleName) if err != nil { return nil, err @@ -413,6 +430,11 @@ func (s *State) FindVerifiersForPath(ctx context.Context, path string) ([]*Verif verifiers = append(verifiers, verifier) if s.HasTargetsRole(delegation.Name) { + env := s.DelegationEnvelopes[delegation.Name] + if err := verifier.Verify(ctx, nil, env); err != nil { + return nil, err + } + delegatedMetadata, err := s.GetTargetsMetadata(delegation.Name) if err != nil { return nil, err @@ -436,20 +458,11 @@ func (s *State) FindVerifiersForPath(ctx context.Context, path string) ([]*Verif } } -// Verify performs a self-contained verification of all the metadata in the -// State starting from the Root. Any metadata that is unreachable in the -// delegations graph returns an error. +// Verify verifies the signatures of the Root role and the top level Targets +// role if it exists. func (s *State) Verify(ctx context.Context) error { - rootVerifiers := []sslibdsse.Verifier{} - for _, k := range s.RootPublicKeys { - sv, err := signerverifier.NewSignerVerifierFromTUFKey(k) - if err != nil { - return err - } - - rootVerifiers = append(rootVerifiers, sv) - } - if err := dsse.VerifyEnvelope(ctx, s.RootEnvelope, rootVerifiers, len(rootVerifiers)); err != nil { + rootVerifier := s.getRootVerifier() + if err := rootVerifier.Verify(ctx, nil, s.RootEnvelope); err != nil { return err } @@ -457,120 +470,12 @@ func (s *State) Verify(ctx context.Context) error { return nil } - rootMetadata := &tuf.RootMetadata{} - rootContents, err := s.RootEnvelope.DecodeB64Payload() + targetsVerifier, err := s.getTargetsVerifier() if err != nil { return err } - if err := json.Unmarshal(rootContents, rootMetadata); err != nil { - return err - } - - targetsVerifiers := []sslibdsse.Verifier{} - for _, keyID := range rootMetadata.Roles[TargetsRoleName].KeyIDs { - key := rootMetadata.Keys[keyID] - sv, err := signerverifier.NewSignerVerifierFromTUFKey(key) - if err != nil { - return err - } - targetsVerifiers = append(targetsVerifiers, sv) - } - if err := dsse.VerifyEnvelope(ctx, s.TargetsEnvelope, targetsVerifiers, rootMetadata.Roles[TargetsRoleName].Threshold); err != nil { - return err - } - - if len(s.DelegationEnvelopes) == 0 { - return nil - } - - delegationEnvelopes := map[string]*sslibdsse.Envelope{} - for k, v := range s.DelegationEnvelopes { - delegationEnvelopes[k] = v - } - - targetsMetadata := &tuf.TargetsMetadata{} - targetsContents, err := s.TargetsEnvelope.DecodeB64Payload() - if err != nil { - return err - } - if err := json.Unmarshal(targetsContents, targetsMetadata); err != nil { - return err - } - - if err := targetsMetadata.Validate(); err != nil { - return err - } - - // Note: If targetsMetadata.Delegations == nil while delegationEnvelopes is - // not empty, we probably want to error out. This should panic. - delegationKeys := targetsMetadata.Delegations.Keys - delegationsQueue := targetsMetadata.Delegations.Roles - - // We can likely process top level targets and all delegated envelopes in - // the loop below by combining the two but this separated model seems easier - // to reason about. Else, we define a custom starting delegation from root - // to targets in the queue and start this loop from there. - - for { - if len(delegationsQueue) == 0 { - break - } - - delegation := delegationsQueue[0] - delegationsQueue = delegationsQueue[1:] - - delegationEnvelope, ok := delegationEnvelopes[delegation.Name] - if !ok { - // Delegation does not have an envelope to verify - continue - } - delete(delegationEnvelopes, delegation.Name) - - delegationVerifiers := make([]sslibdsse.Verifier, 0, len(delegation.KeyIDs)) - for _, keyID := range delegation.KeyIDs { - key := delegationKeys[keyID] - sv, err := signerverifier.NewSignerVerifierFromTUFKey(key) - if err != nil { - return err - } - - delegationVerifiers = append(delegationVerifiers, sv) - } - - if err := dsse.VerifyEnvelope(ctx, delegationEnvelope, delegationVerifiers, delegation.Threshold); err != nil { - return err - } - - delegationMetadata := &tuf.TargetsMetadata{} - delegationContents, err := delegationEnvelope.DecodeB64Payload() - if err != nil { - return err - } - if err := json.Unmarshal(delegationContents, delegationMetadata); err != nil { - return err - } - - if err := delegationMetadata.Validate(); err != nil { - return err - } - - if delegationMetadata.Delegations == nil { - continue - } - - for keyID, key := range delegationMetadata.Delegations.Keys { - delegationKeys[keyID] = key - } - - delegationsQueue = append(delegationsQueue, delegationMetadata.Delegations.Roles...) - } - - if len(delegationEnvelopes) != 0 { - return ErrDanglingDelegationMetadata - } - - return nil + return targetsVerifier.Verify(ctx, nil, s.TargetsEnvelope) } // Commit verifies and writes the State to the policy namespace. It also creates @@ -724,6 +629,44 @@ func (s *State) HasTargetsRole(roleName string) bool { return ok } +func (s *State) getRootVerifier() *Verifier { + // TODO: validate against the root metadata itself? + // This eventually goes back to how the very first root is bootstrapped + // See: https://github.com/gittuf/gittuf/issues/117 + return &Verifier{ + keys: s.RootPublicKeys, + threshold: len(s.RootPublicKeys), + } +} + +func (s *State) getTargetsVerifier() (*Verifier, error) { + rootMetadata, err := s.GetRootMetadata() + if err != nil { + return nil, err + } + + verifier := &Verifier{keys: make([]*tuf.Key, 0, len(rootMetadata.Roles[TargetsRoleName].KeyIDs))} + for _, keyID := range rootMetadata.Roles[TargetsRoleName].KeyIDs { + verifier.keys = append(verifier.keys, rootMetadata.Keys[keyID]) + } + verifier.threshold = rootMetadata.Roles[TargetsRoleName].Threshold + + return verifier, nil +} + +// findDelegationEntry finds the delegation entry for some role in the parent +// role. +// +// Deprecated: diamond delegations are legal in policy. So, role A and role B +// can both independently delegate to role C, and they *don't* need to specify +// the same set of keys / threshold. So, when signing role C, we actually can't +// determine if the keys being used to sign it are valid. It depends strictly on +// how role C is reached, whether via role A or role B. In turn, that depends on +// the exact namespace being verified. In TUF, this issue is known as +// "promiscuous delegations". See: +// https://github.com/theupdateframework/specification/issues/19, +// https://github.com/theupdateframework/specification/issues/214, and +// https://github.com/theupdateframework/python-tuf/issues/660. func (s *State) findDelegationEntry(roleName string) (*tuf.Delegation, error) { topLevelTargetsMetadata, err := s.GetTargetsMetadata(TargetsRoleName) if err != nil { @@ -760,7 +703,6 @@ func (s *State) findDelegationEntry(roleName string) (*tuf.Delegation, error) { } if s.HasTargetsRole(delegation.Name) { - // TODO: clarifying terminating / reachability delegationsQueue = append(delegationsQueue, delegationTargetsMetadata[delegation.Name].Delegations.Roles...) } } diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 193b6521c..1b74e8da4 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -143,23 +143,33 @@ func TestStateKeys(t *testing.T) { } func TestStateVerify(t *testing.T) { - state := createTestStateWithOnlyRoot(t) + t.Run("only root", func(t *testing.T) { + state := createTestStateWithOnlyRoot(t) - if err := state.Verify(context.Background()); err != nil { - t.Error(err) - } + if err := state.Verify(testCtx); err != nil { + t.Error(err) + } - rootKeys := []*tuf.Key{} - copy(rootKeys, state.RootPublicKeys) - state.RootPublicKeys = []*tuf.Key{} + rootKeys := []*tuf.Key{} + copy(rootKeys, state.RootPublicKeys) + state.RootPublicKeys = []*tuf.Key{} - err := state.Verify(context.Background()) - assert.NotNil(t, err) + err := state.Verify(context.Background()) + assert.NotNil(t, err) - state.RootPublicKeys = rootKeys - state.RootEnvelope.Signatures = []sslibdsse.Signature{} - err = state.Verify(context.Background()) - assert.NotNil(t, err) + state.RootPublicKeys = rootKeys + state.RootEnvelope.Signatures = []sslibdsse.Signature{} + err = state.Verify(context.Background()) + assert.NotNil(t, err) + }) + + t.Run("with policy", func(t *testing.T) { + state := createTestStateWithPolicy(t) + + if err := state.Verify(testCtx); err != nil { + t.Error(err) + } + }) } func TestStateCommit(t *testing.T) { @@ -195,48 +205,58 @@ func TestStateGetRootMetadata(t *testing.T) { } func TestStateFindVerifiersForPath(t *testing.T) { - state := createTestStateWithPolicy(t) + t.Run("with policy", func(t *testing.T) { + state := createTestStateWithPolicy(t) - gpgKey, err := gpg.LoadGPGKeyFromBytes(gpgPubKeyBytes) - if err != nil { - t.Fatal(err) - } + gpgKey, err := gpg.LoadGPGKeyFromBytes(gpgPubKeyBytes) + if err != nil { + t.Fatal(err) + } - tests := map[string]struct { - path string - verifiers []*Verifier - }{ - "verifiers for refs/heads/main": { - path: "git:refs/heads/main", - verifiers: []*Verifier{{ - name: "protect-main", - keys: []*tuf.Key{gpgKey}, - threshold: 1, - }}, - }, - "verifiers for files": { - path: "file:1", - verifiers: []*Verifier{{ - name: "protect-files-1-and-2", - keys: []*tuf.Key{gpgKey}, - threshold: 1, - }}, - }, - "verifiers for unprotected branch": { - path: "git:refs/heads/unprotected", - verifiers: []*Verifier{}, - }, - "verifiers for unprotected files": { - path: "file:unprotected", - verifiers: []*Verifier{}, - }, - } + tests := map[string]struct { + path string + verifiers []*Verifier + }{ + "verifiers for refs/heads/main": { + path: "git:refs/heads/main", + verifiers: []*Verifier{{ + name: "protect-main", + keys: []*tuf.Key{gpgKey}, + threshold: 1, + }}, + }, + "verifiers for files": { + path: "file:1", + verifiers: []*Verifier{{ + name: "protect-files-1-and-2", + keys: []*tuf.Key{gpgKey}, + threshold: 1, + }}, + }, + "verifiers for unprotected branch": { + path: "git:refs/heads/unprotected", + verifiers: []*Verifier{}, + }, + "verifiers for unprotected files": { + path: "file:unprotected", + verifiers: []*Verifier{}, + }, + } - for name, test := range tests { - verifiers, err := state.FindVerifiersForPath(context.Background(), test.path) - assert.Nil(t, err, fmt.Sprintf("unexpected error in test '%s'", name)) - assert.Equal(t, test.verifiers, verifiers, fmt.Sprintf("policy verifiers for path '%s' don't match expected verifiers in test '%s'", test.path, name)) - } + for name, test := range tests { + verifiers, err := state.FindVerifiersForPath(context.Background(), test.path) + assert.Nil(t, err, fmt.Sprintf("unexpected error in test '%s'", name)) + assert.Equal(t, test.verifiers, verifiers, fmt.Sprintf("policy verifiers for path '%s' don't match expected verifiers in test '%s'", test.path, name)) + } + }) + + t.Run("without policy", func(t *testing.T) { + state := createTestStateWithOnlyRoot(t) + + verifiers, err := state.FindVerifiersForPath(context.Background(), "test-path") + assert.Nil(t, verifiers) + assert.ErrorIs(t, err, ErrMetadataNotFound) + }) } func TestStateFindPublicKeysForPath(t *testing.T) { diff --git a/internal/policy/verify.go b/internal/policy/verify.go index f9a06618f..5c39c08ec 100644 --- a/internal/policy/verify.go +++ b/internal/policy/verify.go @@ -759,20 +759,22 @@ func (v *Verifier) Threshold() int { // Verify is used to check for a threshold of signatures using the verifier. The // threshold of signatures may be met using a combination of at most one Git -// signature and in-toto attestation signatures. -func (v *Verifier) Verify(ctx context.Context, gitObject object.Object, attestation *sslibdsse.Envelope) error { +// signature and signatures embedded in a DSSE envelope. Verify does not inspect +// the envelope's payload, but instead only verifies the signatures. The caller +// must ensure the validity of the envelope's contents. +func (v *Verifier) Verify(ctx context.Context, gitObject object.Object, env *sslibdsse.Envelope) error { if v.threshold < 1 { return ErrInvalidVerifier } - if gitObject == nil && attestation == nil { + if gitObject == nil && env == nil { return ErrVerifierConditionsUnmet } - if attestation != nil { - // combining the attestation and the git object we still do not have - // sufficient signatures - if (1 + len(attestation.Signatures)) < v.threshold { + if env != nil { + if (1 + len(env.Signatures)) < v.threshold { + // Combining the attestation and the git object we still do not have + // sufficient signatures return ErrVerifierConditionsUnmet } } @@ -780,50 +782,53 @@ func (v *Verifier) Verify(ctx context.Context, gitObject object.Object, attestat var keyIDUsed string gitObjectVerified := false - // First, verify the gitObject's signature - switch o := gitObject.(type) { - case *object.Commit: - for _, key := range v.keys { - err := gitinterface.VerifyCommitSignature(ctx, o, key) - if err == nil { - // Signature verification succeeded - keyIDUsed = key.KeyID - gitObjectVerified = true - break - } - if errors.Is(err, gitinterface.ErrUnknownSigningMethod) { - continue - } - if !errors.Is(err, gitinterface.ErrIncorrectVerificationKey) { - return err - } - } - case *object.Tag: - for _, key := range v.keys { - err := gitinterface.VerifyTagSignature(ctx, o, key) - if err == nil { - // Signature verification succeeded - keyIDUsed = key.KeyID - gitObjectVerified = true - break - } - if errors.Is(err, gitinterface.ErrUnknownSigningMethod) { - continue + // First, verify the gitObject's signature if one is presented + if gitObject != nil { + switch o := gitObject.(type) { + case *object.Commit: + for _, key := range v.keys { + err := gitinterface.VerifyCommitSignature(ctx, o, key) + if err == nil { + // Signature verification succeeded + keyIDUsed = key.KeyID + gitObjectVerified = true + break + } + if errors.Is(err, gitinterface.ErrUnknownSigningMethod) { + continue + } + if !errors.Is(err, gitinterface.ErrIncorrectVerificationKey) { + return err + } } - if !errors.Is(err, gitinterface.ErrIncorrectVerificationKey) { - return err + case *object.Tag: + for _, key := range v.keys { + err := gitinterface.VerifyTagSignature(ctx, o, key) + if err == nil { + // Signature verification succeeded + keyIDUsed = key.KeyID + gitObjectVerified = true + break + } + if errors.Is(err, gitinterface.ErrUnknownSigningMethod) { + continue + } + if !errors.Is(err, gitinterface.ErrIncorrectVerificationKey) { + return err + } } + default: + return ErrUnknownObjectType } - default: - return ErrUnknownObjectType } - // If threshold is 1, we can return + // If threshold is 1 and the Git signature is verified, we can return if v.threshold == 1 && gitObjectVerified { return nil } - // Second, verify signatures on the attestation + // Second, verify signatures on the attestation, subtracting the threshold + // by 1 to account for a verified git signature envelopeThreshold := v.threshold if gitObjectVerified { envelopeThreshold-- @@ -832,6 +837,8 @@ func (v *Verifier) Verify(ctx context.Context, gitObject object.Object, attestat verifiers := make([]sslibdsse.Verifier, 0, len(v.keys)-1) for _, key := range v.keys { if key.KeyID == keyIDUsed { + // Do not create a DSSE verifier for the key used to verify the Git + // signature continue } @@ -842,7 +849,7 @@ func (v *Verifier) Verify(ctx context.Context, gitObject object.Object, attestat verifiers = append(verifiers, verifier) } - if err := dsse.VerifyEnvelope(ctx, attestation, verifiers, envelopeThreshold); err != nil { + if err := dsse.VerifyEnvelope(ctx, env, verifiers, envelopeThreshold); err != nil { return ErrVerifierConditionsUnmet } diff --git a/internal/repository/targets.go b/internal/repository/targets.go index 87196586f..a99a918b3 100644 --- a/internal/repository/targets.go +++ b/internal/repository/targets.go @@ -20,10 +20,6 @@ func (r *Repository) InitializeTargets(ctx context.Context, targetsKeyBytes []by if err != nil { return err } - keyID, err := sv.KeyID() - if err != nil { - return err - } state, err := policy.LoadCurrentState(ctx, r.r) if err != nil { @@ -33,15 +29,10 @@ func (r *Repository) InitializeTargets(ctx context.Context, targetsKeyBytes []by return ErrCannotReinitialize } - // Verify if targetsKeyBytes is authorized to sign for the role - authorizedKeyIDs, err := state.FindAuthorizedSigningKeyIDs(ctx, targetsRoleName) - if err != nil { - return err - } - - if !isKeyAuthorized(authorizedKeyIDs, keyID) { - return ErrUnauthorizedKey - } + // TODO: verify is role can be signed using the presented key. This requires + // the user to pass in the delegating role as well as we do not want to + // assume which role is the delegating role (diamond delegations are legal). + // See: https://github.com/gittuf/gittuf/issues/246. targetsMetadata := policy.InitializeTargetsMetadata() @@ -76,10 +67,6 @@ func (r *Repository) AddDelegation(ctx context.Context, signingKeyBytes []byte, if err != nil { return err } - keyID, err := sv.KeyID() - if err != nil { - return err - } state, err := policy.LoadCurrentState(ctx, r.r) if err != nil { @@ -89,13 +76,10 @@ func (r *Repository) AddDelegation(ctx context.Context, signingKeyBytes []byte, return policy.ErrMetadataNotFound } - authorizedKeyIDsForRole, err := state.FindAuthorizedSigningKeyIDs(ctx, targetsRoleName) - if err != nil { - return err - } - if !isKeyAuthorized(authorizedKeyIDsForRole, keyID) { - return ErrUnauthorizedKey - } + // TODO: verify is role can be signed using the presented key. This requires + // the user to pass in the delegating role as well as we do not want to + // assume which role is the delegating role (diamond delegations are legal). + // See: https://github.com/gittuf/gittuf/issues/246. authorizedKeys := []*tuf.Key{} for _, kb := range authorizedKeysBytes { @@ -147,10 +131,6 @@ func (r *Repository) RemoveDelegation(ctx context.Context, signingKeyBytes []byt if err != nil { return err } - keyID, err := sv.KeyID() - if err != nil { - return err - } state, err := policy.LoadCurrentState(ctx, r.r) if err != nil { @@ -160,13 +140,10 @@ func (r *Repository) RemoveDelegation(ctx context.Context, signingKeyBytes []byt return policy.ErrMetadataNotFound } - authorizedKeyIDsForRole, err := state.FindAuthorizedSigningKeyIDs(ctx, targetsRoleName) - if err != nil { - return err - } - if !isKeyAuthorized(authorizedKeyIDsForRole, keyID) { - return ErrUnauthorizedKey - } + // TODO: verify is role can be signed using the presented key. This requires + // the user to pass in the delegating role as well as we do not want to + // assume which role is the delegating role (diamond delegations are legal). + // See: https://github.com/gittuf/gittuf/issues/246. targetsMetadata, err := state.GetTargetsMetadata(targetsRoleName) if err != nil { @@ -208,10 +185,6 @@ func (r *Repository) AddKeyToTargets(ctx context.Context, signingKeyBytes []byte if err != nil { return err } - keyID, err := sv.KeyID() - if err != nil { - return err - } state, err := policy.LoadCurrentState(ctx, r.r) if err != nil { @@ -221,13 +194,10 @@ func (r *Repository) AddKeyToTargets(ctx context.Context, signingKeyBytes []byte return policy.ErrMetadataNotFound } - authorizedKeyIDsForRole, err := state.FindAuthorizedSigningKeyIDs(ctx, targetsRoleName) - if err != nil { - return err - } - if !isKeyAuthorized(authorizedKeyIDsForRole, keyID) { - return ErrUnauthorizedKey - } + // TODO: verify is role can be signed using the presented key. This requires + // the user to pass in the delegating role as well as we do not want to + // assume which role is the delegating role (diamond delegations are legal). + // See: https://github.com/gittuf/gittuf/issues/246. authorizedKeys := []*tuf.Key{} keyIDs := ""