Merge pull request #623 from docker/vestigial_signatures

remove signatures that are no longer valid during signing
This commit is contained in:
David Lawrence 2016-03-17 09:26:43 -07:00
commit f943677613
5 changed files with 112 additions and 60 deletions

View File

@ -297,10 +297,7 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string
signedTestRoot, err := testRoot.ToSigned()
assert.NoError(t, err)
err = signed.Sign(cs, signedTestRoot, replRootKey)
assert.NoError(t, err)
err = signed.Sign(cs, signedTestRoot, origRootKey)
err = signed.Sign(cs, signedTestRoot, replRootKey, origRootKey)
assert.NoError(t, err)
// This call to ValidateRoot will succeed since we are using a valid PEM

View File

@ -22,10 +22,13 @@ import (
// Sign takes a data.Signed and a key, calculated and adds the signature
// to the data.Signed
// N.B. All public keys for a role should be passed so that this function
// can correctly clean up signatures that are no longer valid.
func Sign(service CryptoService, s *data.Signed, keys ...data.PublicKey) error {
logrus.Debugf("sign called with %d keys", len(keys))
signatures := make([]data.Signature, 0, len(s.Signatures)+1)
signingKeyIDs := make(map[string]struct{})
tufIDs := make(map[string]data.PublicKey)
ids := make([]string, 0, len(keys))
privKeys := make(map[string]data.PrivateKey)
@ -34,6 +37,7 @@ func Sign(service CryptoService, s *data.Signed, keys ...data.PublicKey) error {
for _, key := range keys {
canonicalID, err := utils.CanonicalKeyID(key)
ids = append(ids, canonicalID)
tufIDs[key.ID()] = key
if err != nil {
continue
}
@ -78,6 +82,20 @@ func Sign(service CryptoService, s *data.Signed, keys ...data.PublicKey) error {
// key is in the set of key IDs for which a signature has been created
continue
}
var (
k data.PublicKey
ok bool
)
if k, ok = tufIDs[sig.KeyID]; !ok {
// key is no longer a valid signing key
continue
}
if err := VerifySignature(s.Signed, sig, k); err != nil {
// signature is no longer valid
continue
}
// keep any signatures that still represent valid keys and are
// themselves valid
signatures = append(signatures, sig)
}
s.Signatures = signatures

View File

@ -21,11 +21,6 @@ type FailingCryptoService struct {
testKey data.PrivateKey
}
func (mts *FailingCryptoService) Sign(keyIDs []string, _ []byte) ([]data.Signature, error) {
sigs := make([]data.Signature, 0, len(keyIDs))
return sigs, nil
}
func (mts *FailingCryptoService) Create(_, _ string) (data.PublicKey, error) {
return mts.testKey, nil
}
@ -36,10 +31,10 @@ func (mts *FailingCryptoService) ListKeys(role string) []string {
func (mts *FailingCryptoService) ListAllKeys() map[string]string {
return map[string]string{
mts.testKey.ID(): "root",
mts.testKey.ID(): "targets",
mts.testKey.ID(): "snapshot",
mts.testKey.ID(): "timestamp",
mts.testKey.ID(): data.CanonicalRootRole,
mts.testKey.ID(): data.CanonicalTargetsRole,
mts.testKey.ID(): data.CanonicalSnapshotRole,
mts.testKey.ID(): data.CanonicalTimestampRole,
}
}
@ -69,14 +64,6 @@ type MockCryptoService struct {
testKey data.PrivateKey
}
func (mts *MockCryptoService) Sign(keyIDs []string, _ []byte) ([]data.Signature, error) {
sigs := make([]data.Signature, 0, len(keyIDs))
for _, keyID := range keyIDs {
sigs = append(sigs, data.Signature{KeyID: keyID})
}
return sigs, nil
}
func (mts *MockCryptoService) Create(_ string, _ string) (data.PublicKey, error) {
return mts.testKey, nil
}
@ -94,10 +81,10 @@ func (mts *MockCryptoService) ListKeys(role string) []string {
func (mts *MockCryptoService) ListAllKeys() map[string]string {
return map[string]string{
mts.testKey.ID(): "root",
mts.testKey.ID(): "targets",
mts.testKey.ID(): "snapshot",
mts.testKey.ID(): "timestamp",
mts.testKey.ID(): data.CanonicalRootRole,
mts.testKey.ID(): data.CanonicalTargetsRole,
mts.testKey.ID(): data.CanonicalSnapshotRole,
mts.testKey.ID(): data.CanonicalTimestampRole,
}
}
@ -119,16 +106,6 @@ type StrictMockCryptoService struct {
MockCryptoService
}
func (mts *StrictMockCryptoService) Sign(keyIDs []string, _ []byte) ([]data.Signature, error) {
sigs := make([]data.Signature, 0, len(keyIDs))
for _, keyID := range keyIDs {
if keyID == mts.testKey.ID() {
sigs = append(sigs, data.Signature{KeyID: keyID})
}
}
return sigs, nil
}
func (mts *StrictMockCryptoService) GetKey(keyID string) data.PublicKey {
if keyID == mts.testKey.ID() {
return data.PublicKeyFromPrivate(mts.testKey)
@ -142,10 +119,10 @@ func (mts *StrictMockCryptoService) ListKeys(role string) []string {
func (mts *StrictMockCryptoService) ListAllKeys() map[string]string {
return map[string]string{
mts.testKey.ID(): "root",
mts.testKey.ID(): "targets",
mts.testKey.ID(): "snapshot",
mts.testKey.ID(): "timestamp",
mts.testKey.ID(): data.CanonicalRootRole,
mts.testKey.ID(): data.CanonicalTargetsRole,
mts.testKey.ID(): data.CanonicalSnapshotRole,
mts.testKey.ID(): data.CanonicalTimestampRole,
}
}
@ -156,7 +133,7 @@ func (mts *StrictMockCryptoService) ImportRootKey(r io.Reader) error {
// Test signing and ensure the expected signature is added
func TestBasicSign(t *testing.T) {
cs := NewEd25519()
key, err := cs.Create("root", data.ED25519Key)
key, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
testData := data.Signed{}
@ -176,7 +153,7 @@ func TestBasicSign(t *testing.T) {
// with the same key ID
func TestReSign(t *testing.T) {
cs := NewEd25519()
key, err := cs.Create("root", data.ED25519Key)
key, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
testData := data.Signed{}
@ -198,8 +175,9 @@ func TestMultiSign(t *testing.T) {
cs := NewEd25519()
testData := data.Signed{}
key1, err := cs.Create("root", data.ED25519Key)
key1, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
Sign(cs, &testData, key1)
// reinitializing cs means it won't know about key1. We want
@ -207,8 +185,9 @@ func TestMultiSign(t *testing.T) {
// that the signature for key1 is left intact and the signature
// for key2 is added
cs = NewEd25519()
key2, err := cs.Create("root", data.ED25519Key)
key2, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
Sign(
cs,
&testData,
@ -229,7 +208,6 @@ func TestMultiSign(t *testing.T) {
}
}
require.Equal(t, 2, count)
}
func TestSignReturnsNoSigs(t *testing.T) {
@ -270,3 +248,59 @@ func TestSignWithX509(t *testing.T) {
require.Len(t, testData.Signatures, 1)
require.Equal(t, tufRSAx509Key.ID(), testData.Signatures[0].KeyID)
}
func TestSignRemovesValidSigByInvalidKey(t *testing.T) {
cs := NewEd25519()
testData := data.Signed{}
key1, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
Sign(cs, &testData, key1)
require.Len(t, testData.Signatures, 1)
require.Equal(t, key1.ID(), testData.Signatures[0].KeyID)
key2, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
// should remove key1 sig even though it's valid. It no longer appears
// in the list of signing keys for the role
Sign(
cs,
&testData,
key2,
)
require.Len(t, testData.Signatures, 1)
require.Equal(t, key2.ID(), testData.Signatures[0].KeyID)
}
func TestSignRemovesInvalidSig(t *testing.T) {
cs := NewEd25519()
testData := data.Signed{}
key1, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
Sign(cs, &testData, key1)
require.Len(t, testData.Signatures, 1)
require.Equal(t, key1.ID(), testData.Signatures[0].KeyID)
// we need cs to "forget" key1 so we can't sign with it
cs = NewEd25519()
key2, err := cs.Create(data.CanonicalRootRole, data.ED25519Key)
require.NoError(t, err)
// modify test data to invalidate key1 sig
testData.Signed = []byte{0xff}
// should remove key1 sig because it's out of date
Sign(
cs,
&testData,
key1,
key2,
)
require.Len(t, testData.Signatures, 1)
require.Equal(t, key2.ID(), testData.Signatures[0].KeyID)
}

View File

@ -2,6 +2,7 @@ package signed
import (
"errors"
"fmt"
"strings"
"time"
@ -124,16 +125,8 @@ func VerifySignatures(s *data.Signed, roleData data.BaseRole) error {
logrus.Debugf("continuing b/c keyid lookup was nil: %s\n", sig.KeyID)
continue
}
// method lookup is consistent due to Unmarshal JSON doing lower case for us.
method := sig.Method
verifier, ok := Verifiers[method]
if !ok {
logrus.Debugf("continuing b/c signing method is not supported: %s\n", sig.Method)
continue
}
if err := verifier.Verify(key, sig.Signature, msg); err != nil {
logrus.Debugf("continuing b/c signature was invalid\n")
if err := VerifySignature(msg, sig, key); err != nil {
logrus.Debugf("continuing b/c %s", err.Error())
continue
}
valid[sig.KeyID] = struct{}{}
@ -145,3 +138,18 @@ func VerifySignatures(s *data.Signed, roleData data.BaseRole) error {
return nil
}
// VerifySignature checks a single signature and public key against a payload
func VerifySignature(msg []byte, sig data.Signature, pk data.PublicKey) error {
// method lookup is consistent due to Unmarshal JSON doing lower case for us.
method := sig.Method
verifier, ok := Verifiers[method]
if !ok {
return fmt.Errorf("signing method is not supported: %s\n", sig.Method)
}
if err := verifier.Verify(pk, sig.Signature, msg); err != nil {
return fmt.Errorf("signature was invalid\n")
}
return nil
}

View File

@ -919,12 +919,7 @@ func (tr *Repo) SignTimestamp(expires time.Time) (*data.Signed, error) {
}
func (tr Repo) sign(signedData *data.Signed, role data.BaseRole) (*data.Signed, error) {
ks := role.ListKeys()
if len(ks) < 1 {
return nil, signed.ErrNoKeys{}
}
err := signed.Sign(tr.cryptoService, signedData, ks...)
if err != nil {
if err := signed.Sign(tr.cryptoService, signedData, role.ListKeys()...); err != nil {
return nil, err
}
return signedData, nil