From aa4ea747f9c35ddd950b71fa857f706ee3cf7785 Mon Sep 17 00:00:00 2001 From: Hayden B Date: Fri, 1 Mar 2024 08:47:59 -0800 Subject: [PATCH] Revert "use ecdsa for keytype. But when signing, support both formats (#1130)" This reverts commit 150691097d500359d4290d6ae45e1c028f7d7b6e. --- cmd/tuf/app/add-delegation.go | 4 +-- cmd/tuf/app/init.go | 6 ++-- cmd/tuf/app/sign.go | 26 +++++----------- cmd/verify/app/keys.go | 21 +++++-------- pkg/keys/keys.go | 20 ++++-------- pkg/keys/keys_test.go | 4 +-- scripts/step-2.sh | 2 +- tests/e2e_test.go | 58 +++++++++++++++++------------------ tests/utils.go | 4 +-- 9 files changed, 58 insertions(+), 87 deletions(-) diff --git a/cmd/tuf/app/add-delegation.go b/cmd/tuf/app/add-delegation.go index bc4d1d59..ca2533c0 100644 --- a/cmd/tuf/app/add-delegation.go +++ b/cmd/tuf/app/add-delegation.go @@ -140,9 +140,7 @@ func DelegationCmd(ctx context.Context, opts *DelegationOptions) error { if err != nil { return err } - // When adding the delegation we don't need to carry over the - // older deprecated format. - tufKey, err := pkeys.ConstructTufKeyFromPublic(ctx, publicKey, false) + tufKey, err := pkeys.ConstructTufKeyFromPublic(ctx, publicKey) if err != nil { return err } diff --git a/cmd/tuf/app/init.go b/cmd/tuf/app/init.go index fb567f62..30d62f87 100644 --- a/cmd/tuf/app/init.go +++ b/cmd/tuf/app/init.go @@ -180,8 +180,7 @@ func InitCmd(ctx context.Context, directory string, } // Construct TUF key. - // Only create keys using the current key type - tufKey, err := pkeys.ConstructTufKey(ctx, signer, false) + tufKey, err := pkeys.ConstructTufKey(ctx, signer) if err != nil { return err } @@ -342,8 +341,7 @@ func getKeysFromDir(dir string) ([]*data.PublicKey, error) { if err != nil { return nil, err } - // Only add keys with the new format - tufKey, err := pkeys.EcdsaTufKey(key.PublicKey, false) + tufKey, err := pkeys.EcdsaTufKey(key.PublicKey) if err != nil { return nil, err } diff --git a/cmd/tuf/app/sign.go b/cmd/tuf/app/sign.go index 211ac5e0..e8e59112 100644 --- a/cmd/tuf/app/sign.go +++ b/cmd/tuf/app/sign.go @@ -54,7 +54,6 @@ func Sign() *ffcli.Command { sk = flagset.Bool("sk", false, "indicates use of a hardware key for signing") key = flagset.String("key", "", "reference to a signer for signing") bumpVersion = flagset.Bool("bump-version", false, "bumps the version; useful for re-signing without changes") - addOldType = flagset.Bool("add-old-type", false, "Add a signature using the old key type") ) flagset.Var(&roles, "roles", "role(s) to sign") return &ffcli.Command{ @@ -81,7 +80,7 @@ func Sign() *ffcli.Command { if err != nil { return err } - return SignCmd(ctx, *repository, roles, signer, *bumpVersion, *addOldType) + return SignCmd(ctx, *repository, roles, signer, *bumpVersion) }, } } @@ -122,8 +121,8 @@ func checkMetaForRole(store tuf.LocalStore, role []string) error { return nil } -func SignCmd(ctx context.Context, directory string, roles []string, - signer signature.Signer, bumpVersion, addOldType bool) error { +func SignCmd(ctx context.Context, directory string, roles []string, signer signature.Signer, + bumpVersion bool) error { store := tuf.FileSystemStore(directory, nil) if err := checkMetaForRole(store, roles); err != nil { @@ -136,7 +135,7 @@ func SignCmd(ctx context.Context, directory string, roles []string, return err } } - if err := SignMeta(ctx, store, name+".json", signer, addOldType); err != nil { + if err := SignMeta(ctx, store, name+".json", signer); err != nil { return err } } @@ -149,8 +148,7 @@ func SignCmd(ctx context.Context, directory string, roles []string, // Note that if you were using old format exclusively (for testing), then this will // have no impact on repository validity: extraneous key IDs for the role that are // not attested to in the trusted root or parent delegation will be ignored. -func SignMeta(ctx context.Context, store tuf.LocalStore, name string, - signer signature.Signer, addOldType bool) error { +func SignMeta(ctx context.Context, store tuf.LocalStore, name string, signer signature.Signer) error { fmt.Printf("Signing metadata for %s... \n", name) s, err := repo.GetSignedMeta(store, name) if err != nil { @@ -178,20 +176,10 @@ func SignMeta(ctx context.Context, store tuf.LocalStore, name string, } // Get TUF public IDs associated to the signer. - pubKey, err := keys.ConstructTufKey(ctx, signer, false) - var candidateKeyIDs []string + pubKey, err := keys.ConstructTufKey(ctx, signer) if err != nil { return err } - candidateKeyIDs = append(candidateKeyIDs, pubKey.IDs()...) - - if addOldType { - oldKey, err := keys.ConstructTufKey(ctx, signer, true) - if err != nil { - return err - } - candidateKeyIDs = append(candidateKeyIDs, oldKey.IDs()...) - } role := strings.TrimSuffix(name, ".json") roleSigningKeys, err := repo.GetSigningKeyIDsForRole(role, store) @@ -201,7 +189,7 @@ func SignMeta(ctx context.Context, store tuf.LocalStore, name string, // Filter key IDs with the role signing key map. var keyIDs []string - for _, id := range candidateKeyIDs { + for _, id := range pubKey.IDs() { // We check to make sure that the key ID is associated with the role's keys. if _, ok := roleSigningKeys[id]; !ok { continue diff --git a/cmd/verify/app/keys.go b/cmd/verify/app/keys.go index 66ae54fb..3e8f8e6d 100644 --- a/cmd/verify/app/keys.go +++ b/cmd/verify/app/keys.go @@ -71,23 +71,18 @@ func verifySigningKeys(dirname string, rootCA *x509.Certificate) (*KeyMap, error log.Printf("error verifying key %d: %s", key.SerialNumber, err) return nil, err } - - for _, bv := range []bool{true, false} { - tufKey, err := keys.EcdsaTufKey(key.PublicKey, bv) - if err != nil { - return nil, err - } - if len(tufKey.IDs()) == 0 { - return nil, errors.New("error getting key ID") - } - keyMap[tufKey.IDs()[0]] = key + tufKey, err := keys.EcdsaTufKey(key.PublicKey) + if err != nil { + return nil, err } + if len(tufKey.IDs()) == 0 { + return nil, errors.New("error getting key ID") + } + keyMap[tufKey.IDs()[0]] = key log.Printf("\nVERIFIED KEY WITH SERIAL NUMBER %d\n", key.SerialNumber) log.Printf("TUF key ids: \n") - for kid := range keyMap { - log.Printf("\t%s ", kid) - } + log.Printf("\t%s ", tufKey.IDs()[0]) } } // Note we use relative path here to simplify things. diff --git a/pkg/keys/keys.go b/pkg/keys/keys.go index 21c07000..031c1e1e 100644 --- a/pkg/keys/keys.go +++ b/pkg/keys/keys.go @@ -95,23 +95,15 @@ func ToSigningKey(serialNumber int, pubKey []byte, deviceCert []byte, keyCert [] } // EcdsaTufKey returns a PEM-encoded TUF public key for an ecdsa key. -func EcdsaTufKey(pub *ecdsa.PublicKey, deprecatedType bool) (*data.PublicKey, error) { +func EcdsaTufKey(pub *ecdsa.PublicKey) (*data.PublicKey, error) { keyValBytes, err := json.Marshal(keys.EcdsaVerifier{ PublicKey: &keys.PKIXPublicKey{PublicKey: pub}}) if err != nil { return nil, err } - - var keyType data.KeyType - if deprecatedType { - keyType = data.KeyTypeECDSA_SHA2_P256_OLD_FMT - } else { - keyType = data.KeyTypeECDSA_SHA2_P256 - } - return &data.PublicKey{ // TODO: Update to new format for next key signing - Type: keyType, + Type: data.KeyTypeECDSA_SHA2_P256_OLD_FMT, Scheme: data.KeySchemeECDSA_SHA2_P256, Algorithms: data.HashAlgorithms, Value: keyValBytes, @@ -206,19 +198,19 @@ func (key SigningKey) Verify(root *x509.Certificate) error { } // ConstructTufKey constructs a TUF public key from a given signer. -func ConstructTufKey(ctx context.Context, signer signature.Signer, deprecated bool) (*data.PublicKey, error) { +func ConstructTufKey(ctx context.Context, signer signature.Signer) (*data.PublicKey, error) { pub, err := signer.PublicKey(options.WithContext(ctx)) if err != nil { return nil, err } - return ConstructTufKeyFromPublic(ctx, pub, deprecated) + return ConstructTufKeyFromPublic(ctx, pub) } // ConstructTufKey constructs a TUF public key from a public key -func ConstructTufKeyFromPublic(_ context.Context, pubKey crypto.PublicKey, deprecated bool) (*data.PublicKey, error) { +func ConstructTufKeyFromPublic(_ context.Context, pubKey crypto.PublicKey) (*data.PublicKey, error) { switch kt := pubKey.(type) { case *ecdsa.PublicKey: - return EcdsaTufKey(kt, deprecated) + return EcdsaTufKey(kt) default: return nil, fmt.Errorf("ConstructTufKeyFromPublic: key type %s not supported", kt) } diff --git a/pkg/keys/keys_test.go b/pkg/keys/keys_test.go index f472ca1e..032f74e2 100644 --- a/pkg/keys/keys_test.go +++ b/pkg/keys/keys_test.go @@ -160,7 +160,7 @@ func TestToSigningKey(t *testing.T) { t.Errorf("unexpected error generating signing key (%s): %s", tt.name, err) } if tt.expectSuccess { - pemPubKey, err := EcdsaTufKey(key.PublicKey, true) + pemPubKey, err := EcdsaTufKey(key.PublicKey) if err != nil { t.Errorf("unexpected error generating PEM TUF public key: %s", err) } @@ -184,7 +184,7 @@ func TestGetSigningKey(t *testing.T) { } t.Run("valid signing key with PEM", func(t *testing.T) { - signingKeyPem, err := ConstructTufKey(ctx, signingKey, true) + signingKeyPem, err := ConstructTufKey(ctx, signingKey) if err != nil { t.Fatal(err) } diff --git a/scripts/step-2.sh b/scripts/step-2.sh index 3206e749..c40c2801 100755 --- a/scripts/step-2.sh +++ b/scripts/step-2.sh @@ -38,7 +38,7 @@ checkout_branch read -n1 -r -s -p "Insert your Yubikey, then press any key to continue...\n" # Sign the root and targets with hardware key -./tuf sign -repository "$REPO" -roles root -roles targets -sk -add-old-type true +./tuf sign -repository "$REPO" -roles root -roles targets -sk # Ask user to remove key (and replace with SSH security key) read -n1 -r -s -p "Remove your Yubikey, then press any key to continue...\n" diff --git a/tests/e2e_test.go b/tests/e2e_test.go index 728d1987..2141e218 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -188,11 +188,11 @@ func TestSignRootTargets(t *testing.T) { // Sign root and targets. rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } - pubKey, err := keys.ConstructTufKey(ctx, rootSigner, false) + pubKey, err := keys.ConstructTufKey(ctx, rootSigner) if err != nil { t.Fatal(err) } @@ -252,7 +252,7 @@ func TestSnapshotUnvalidatedFails(t *testing.T) { // Now sign root and targets with 1/1 threshold key. rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -301,7 +301,7 @@ func TestPublishSuccess(t *testing.T) { // Sign root & targets rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -359,10 +359,10 @@ func TestRotateRootKey(t *testing.T) { // Sign root & targets with key 1 rootSigner1 := stack.getSigner(t, rootKeyRef1) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner1, false, false); err != nil { + rootSigner1, false); err != nil { t.Fatal(err) } - rootTufKey1, err := keys.ConstructTufKey(ctx, rootSigner1, false) + rootTufKey1, err := keys.ConstructTufKey(ctx, rootSigner1) if err != nil { t.Fatal(err) } @@ -383,7 +383,7 @@ func TestRotateRootKey(t *testing.T) { t.Fatalf("expected root role") } rootSigner2 := stack.getSigner(t, rootKeyRef2) - rootTufKey2, err := keys.ConstructTufKey(ctx, rootSigner2, false) + rootTufKey2, err := keys.ConstructTufKey(ctx, rootSigner2) if err != nil { t.Fatal(err) } @@ -415,7 +415,7 @@ func TestRotateRootKey(t *testing.T) { t.Fatalf("expected root role") } rootSigner3 := stack.getSigner(t, rootKeyRef3) - rootTufKey3, err := keys.ConstructTufKey(ctx, rootSigner3, false) + rootTufKey3, err := keys.ConstructTufKey(ctx, rootSigner3) if err != nil { t.Fatal(err) } @@ -434,7 +434,7 @@ func TestRotateRootKey(t *testing.T) { // Sign root & targets if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, rootSigner1, - false, false); err != nil { + false); err != nil { t.Fatal(err) } @@ -465,7 +465,7 @@ func TestRotateTarget(t *testing.T) { // Sign root & targets rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -505,7 +505,7 @@ func TestRotateTarget(t *testing.T) { // Sign root & targets if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -552,7 +552,7 @@ func TestConsistentSnapshotFlip(t *testing.T) { // Sign root & targets rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -590,7 +590,7 @@ func TestConsistentSnapshotFlip(t *testing.T) { // Sign root & targets if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } // Sign snapshot and timestamp @@ -651,7 +651,7 @@ func TestSnapshotKeyRotate(t *testing.T) { // Sign root & targets with key 1 rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -671,7 +671,7 @@ func TestSnapshotKeyRotate(t *testing.T) { t.Fatalf("expected snapshot role") } snapshotSigner1 := stack.getSigner(t, stack.snapshotRef) - snapshotTufKey1, err := keys.ConstructTufKey(ctx, snapshotSigner1, false) + snapshotTufKey1, err := keys.ConstructTufKey(ctx, snapshotSigner1) if err != nil { t.Fatal(err) } @@ -693,7 +693,7 @@ func TestSnapshotKeyRotate(t *testing.T) { // Sign root & targets with key 1 if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -712,7 +712,7 @@ func TestSnapshotKeyRotate(t *testing.T) { t.Fatalf("expected snapshot role") } snapshotSigner2 := stack.getSigner(t, stack.snapshotRef) - snapshotTufKey2, err := keys.ConstructTufKey(ctx, snapshotSigner2, false) + snapshotTufKey2, err := keys.ConstructTufKey(ctx, snapshotSigner2) if err != nil { t.Fatal(err) } @@ -752,7 +752,7 @@ func TestProdTargetsConfig(t *testing.T) { // Sign root & targets rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -820,7 +820,7 @@ func TestDelegationsClearedOnInit(t *testing.T) { // Sign root & targets with key 1 rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } @@ -870,13 +870,13 @@ func TestSignWithVersionBump(t *testing.T) { // Sign root & targets with key 1 rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } // Sign delegation dSigner := stack.getSigner(t, delegationKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"delegation"}, - dSigner, false, false); err != nil { + dSigner, false); err != nil { t.Fatal(err) } @@ -892,7 +892,7 @@ func TestSignWithVersionBump(t *testing.T) { // Increment the delegation metadata if err := app.SignCmd(ctx, stack.repoDir, []string{"delegation"}, - dSigner, true, false); err != nil { + dSigner, true); err != nil { t.Fatal(err) } @@ -975,10 +975,10 @@ func TestRotateRootKeyTwiceAfter(t *testing.T) { // Sign root & targets with key 1 rootSigner1 := stack.getSigner(t, rootKeyRef1) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner1, false, false); err != nil { + rootSigner1, false); err != nil { t.Fatal(err) } - pubKey1, err := keys.ConstructTufKey(ctx, rootSigner1, false) + pubKey1, err := keys.ConstructTufKey(ctx, rootSigner1) if err != nil { t.Fatal(err) } @@ -1007,7 +1007,7 @@ func TestRotateRootKeyTwiceAfter(t *testing.T) { } // Sign root & targets if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, rootSigner1, - false, false); err != nil { + false); err != nil { t.Fatal(err) } @@ -1066,11 +1066,11 @@ func TestGetPublicKeyFromRepo(t *testing.T) { // Sign root & targets with key rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } - cosignKeyID := "8679120321f0dda9e41c5de3c68e913d383bc51266691007b7b4b7f99868ee6f" + cosignKeyID := "314ae73abd3012fc73bfcc3783e31d03852716597642b891d6a33155c4baf600" keyID, err := app.GetKeyIDForRole(stack.repoDir, "delegation") if err != nil { t.Fatal(err) @@ -1155,13 +1155,13 @@ func TestDelegationNullCustomMetadata(t *testing.T) { // Sign root & targets with key 1 rootSigner := stack.getSigner(t, rootKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"root", "targets"}, - rootSigner, false, false); err != nil { + rootSigner, false); err != nil { t.Fatal(err) } // Sign delegation dSigner := stack.getSigner(t, delegationKeyRef) if err := app.SignCmd(ctx, stack.repoDir, []string{"path"}, - dSigner, false, false); err != nil { + dSigner, false); err != nil { t.Fatal(err) } diff --git a/tests/utils.go b/tests/utils.go index 81762003..6875a012 100644 --- a/tests/utils.go +++ b/tests/utils.go @@ -145,7 +145,7 @@ func (s *repoTestStack) snapshot(t *testing.T) { if err != nil { t.Fatal(err) } - if err := app.SignCmd(s.ctx, s.repoDir, []string{"snapshot"}, snapshotSigner, false, true); err != nil { + if err := app.SignCmd(s.ctx, s.repoDir, []string{"snapshot"}, snapshotSigner, false); err != nil { t.Fatal(err) } } @@ -158,7 +158,7 @@ func (s *repoTestStack) timestamp(t *testing.T) { if err != nil { t.Fatal(err) } - if err := app.SignCmd(s.ctx, s.repoDir, []string{"timestamp"}, timestampSigner, false, true); err != nil { + if err := app.SignCmd(s.ctx, s.repoDir, []string{"timestamp"}, timestampSigner, false); err != nil { t.Fatal(err) } }