From 7fe11bd10226efbb768e188a0686630c22928841 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Mon, 11 Apr 2016 17:26:08 -0700 Subject: [PATCH 1/3] Add key ID verification to Verify Signed-off-by: Riyaz Faizullabhoy --- tuf/signed/errors.go | 7 +++++++ tuf/signed/verify.go | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/tuf/signed/errors.go b/tuf/signed/errors.go index 2c5174bef9..1591ae4f2b 100644 --- a/tuf/signed/errors.go +++ b/tuf/signed/errors.go @@ -51,6 +51,13 @@ func (e ErrInvalidKeyType) Error() string { return "key type is not valid for signature" } +// ErrInvalidKeyID indicates the specified key ID was incorrect for its associated data +type ErrInvalidKeyID struct{} + +func (e ErrInvalidKeyID) Error() string { + return "key ID is not valid for key content" +} + // ErrInvalidKeyLength indicates that while we may support the cipher, the provided // key length is not specifically supported, i.e. we support RSA, but not 1024 bit keys type ErrInvalidKeyLength struct { diff --git a/tuf/signed/verify.go b/tuf/signed/verify.go index 7e34e2599f..0805bba65a 100644 --- a/tuf/signed/verify.go +++ b/tuf/signed/verify.go @@ -125,6 +125,10 @@ func VerifySignatures(s *data.Signed, roleData data.BaseRole) error { logrus.Debugf("continuing b/c keyid lookup was nil: %s\n", sig.KeyID) continue } + // Check that the signature key ID actually matches the content ID of the key + if key.ID() != sig.KeyID { + return ErrInvalidKeyType{} + } if err := VerifySignature(msg, sig, key); err != nil { logrus.Debugf("continuing b/c %s", err.Error()) continue From f7f8a961b5319770ff22d38add7242e186e76183 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Mon, 11 Apr 2016 17:28:14 -0700 Subject: [PATCH 2/3] Unit test for verifying invalid key ID Signed-off-by: Riyaz Faizullabhoy --- tuf/signed/verify.go | 2 +- tuf/signed/verify_test.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/tuf/signed/verify.go b/tuf/signed/verify.go index 0805bba65a..304de4c616 100644 --- a/tuf/signed/verify.go +++ b/tuf/signed/verify.go @@ -127,7 +127,7 @@ func VerifySignatures(s *data.Signed, roleData data.BaseRole) error { } // Check that the signature key ID actually matches the content ID of the key if key.ID() != sig.KeyID { - return ErrInvalidKeyType{} + return ErrInvalidKeyID{} } if err := VerifySignature(msg, sig, key); err != nil { logrus.Debugf("continuing b/c %s", err.Error()) diff --git a/tuf/signed/verify_test.go b/tuf/signed/verify_test.go index 4773ca9a21..83d8f04258 100644 --- a/tuf/signed/verify_test.go +++ b/tuf/signed/verify_test.go @@ -64,6 +64,25 @@ func TestMoreThanEnoughSigs(t *testing.T) { require.NoError(t, err) } +func TestValidSigWithIncorrectKeyID(t *testing.T) { + cs := NewEd25519() + k1, err := cs.Create("root", "", data.ED25519Key) + require.NoError(t, err) + roleWithKeys := data.BaseRole{Name: "root", Keys: data.Keys{"invalidIDA": k1}, Threshold: 1} + + meta := &data.SignedCommon{Type: "Root", Version: 1, Expires: data.DefaultExpires("root")} + + b, err := json.MarshalCanonical(meta) + require.NoError(t, err) + s := &data.Signed{Signed: (*json.RawMessage)(&b)} + Sign(cs, s, k1) + require.Equal(t, 1, len(s.Signatures)) + s.Signatures[0].KeyID = "invalidIDA" + err = Verify(s, roleWithKeys, 1) + require.Error(t, err) + require.IsType(t, ErrInvalidKeyID{}, err) +} + func TestDuplicateSigs(t *testing.T) { cs := NewEd25519() k, err := cs.Create("root", "", data.ED25519Key) From 55aee374e1760647c47bdb378e038545ebe83e3c Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 11 Apr 2016 17:34:35 -0700 Subject: [PATCH 3/3] fixing some certs tests as additional validation meant we weren't testing what we thought we were Signed-off-by: David Lawrence (github: endophage) --- certs/certs_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/certs/certs_test.go b/certs/certs_test.go index ee0cb5229c..62817ddd9c 100644 --- a/certs/certs_test.go +++ b/certs/certs_test.go @@ -340,7 +340,12 @@ func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType st testRoot, err := data.NewRoot( map[string]data.PublicKey{replRootKey.ID(): replRootKey}, - map[string]*data.RootRole{data.CanonicalRootRole: &rootRole.RootRole}, + map[string]*data.RootRole{ + data.CanonicalRootRole: &rootRole.RootRole, + data.CanonicalTargetsRole: &rootRole.RootRole, + data.CanonicalSnapshotRole: &rootRole.RootRole, + data.CanonicalTimestampRole: &rootRole.RootRole, + }, false, ) require.NoError(t, err, "Failed to create new root") @@ -399,7 +404,12 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str testRoot, err := data.NewRoot( map[string]data.PublicKey{replRootKey.ID(): replRootKey}, - map[string]*data.RootRole{data.CanonicalRootRole: &rootRole.RootRole}, + map[string]*data.RootRole{ + data.CanonicalRootRole: &rootRole.RootRole, + data.CanonicalTargetsRole: &rootRole.RootRole, + data.CanonicalSnapshotRole: &rootRole.RootRole, + data.CanonicalTimestampRole: &rootRole.RootRole, + }, false, ) require.NoError(t, err, "Failed to create new root")