From ff54ca140f3310b424d4c50c688b46d360b0cb24 Mon Sep 17 00:00:00 2001 From: Volodymyr Khoroz Date: Tue, 23 Jan 2024 17:42:07 +0200 Subject: [PATCH] Chore: more verbose error messages for missing TUF private keys Signed-off-by: Volodymyr Khoroz --- .../keys/tuf_updates_delete_offline_key.go | 2 +- .../keys/tuf_updates_rotate_offline_key.go | 6 ++--- .../keys/tuf_updates_sign_prod_targets.go | 2 +- subcommands/keys/tuf_utils.go | 23 +++++++++++++------ subcommands/waves/init.go | 2 +- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/subcommands/keys/tuf_updates_delete_offline_key.go b/subcommands/keys/tuf_updates_delete_offline_key.go index 10f36dc2..5045469b 100644 --- a/subcommands/keys/tuf_updates_delete_offline_key.go +++ b/subcommands/keys/tuf_updates_delete_offline_key.go @@ -104,7 +104,7 @@ func doTufUpdatesDeleteOfflineKey(cmd *cobra.Command, args []string) { fmt.Println("= Delete keyid:", keyId) if keyId == "" { oldKey, err := FindOneTufSigner(newCiRoot, creds, validKeyIds) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, ErrMsgReadingTufKey(roleName, "current")) keyId = oldKey.Id } else if !slices.Contains(validKeyIds, keyId) { subcommands.DieNotNil(fmt.Errorf( diff --git a/subcommands/keys/tuf_updates_rotate_offline_key.go b/subcommands/keys/tuf_updates_rotate_offline_key.go index de2bd5d5..1d9ec20e 100644 --- a/subcommands/keys/tuf_updates_rotate_offline_key.go +++ b/subcommands/keys/tuf_updates_rotate_offline_key.go @@ -183,7 +183,7 @@ func doTufUpdatesRotateOfflineTargetsKey(cmd *cobra.Command) { // Seaching for old key in curCiRoot supports several rotations in one transaction. oldestKey, err = FindOneTufSigner(curCiRoot, targetsCreds, subcommands.SliceRemove(curCiRoot.Signed.Roles["targets"].KeyIDs, onlineTargetsId)) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, ErrMsgReadingTufKey(tufRoleNameTargets, "current")) } targetsProdMap, err := api.ProdTargetsList(factory, false) @@ -226,7 +226,7 @@ func replaceOfflineRootKey( ) (TufSigner, OfflineCreds) { oldKids := root.Signed.Roles["root"].KeyIDs oldKey, err := FindOneTufSigner(root, creds, oldKids) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, ErrMsgReadingTufKey(tufRoleNameRoot, "current")) oldKids = subcommands.SliceRemove(oldKids, oldKey.Id) kp := genTufKeyPair(keyType) @@ -242,7 +242,7 @@ func replaceOfflineTargetsKey( oldKids := root.Signed.Roles["targets"].KeyIDs if len(oldKids) > 1 { oldKey, err := FindOneTufSigner(root, creds, subcommands.SliceRemove(oldKids, onlineTargetsId)) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, ErrMsgReadingTufKey(tufRoleNameTargets, "current")) oldKids = subcommands.SliceRemove(oldKids, oldKey.Id) } diff --git a/subcommands/keys/tuf_updates_sign_prod_targets.go b/subcommands/keys/tuf_updates_sign_prod_targets.go index da366282..8fa30a60 100644 --- a/subcommands/keys/tuf_updates_sign_prod_targets.go +++ b/subcommands/keys/tuf_updates_sign_prod_targets.go @@ -69,7 +69,7 @@ For example, add a new offline TUF targets key, before signing production target } signer, err := FindOneTufSigner(newCiRoot, creds, subcommands.SliceRemove(newCiRoot.Signed.Roles["targets"].KeyIDs, onlineTargetsId)) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, ErrMsgReadingTufKey(tufRoleNameTargets, "current")) var newTargetsProdSigs, newTargetsWaveSigs map[string][]tuf.Signature diff --git a/subcommands/keys/tuf_utils.go b/subcommands/keys/tuf_utils.go index 4f26d275..aa6dcd70 100644 --- a/subcommands/keys/tuf_utils.go +++ b/subcommands/keys/tuf_utils.go @@ -24,7 +24,7 @@ import ( "github.com/foundriesio/fioctl/subcommands" ) -var errFoundNoKey = errors.New("Found no active signing key") +var errFoundNoKey = errors.New("Found no private key") type OfflineCreds map[string][]byte @@ -42,6 +42,10 @@ type TufKeyPair struct { atsPubBytes []byte } +func ErrMsgReadingTufKey(role, treat string) string { + return fmt.Sprintf("Error reading %s TUF %s private key from a specified file:\n", treat, role) +} + func ParseTufKeyType(s string) TufKeyType { t, err := parseTufKeyType(s) subcommands.DieNotNil(err) @@ -232,9 +236,9 @@ func FindOneTufSigner(root *client.AtsTufRoot, creds OfflineCreds, keyids []stri var signers []TufSigner if signers, err = findTufSigners(root, creds, keyids); err == nil { if len(signers) == 0 { - err = fmt.Errorf("%w for: %v.", errFoundNoKey, keyids) + err = fmt.Errorf("%w for key IDs: %v.", errFoundNoKey, keyids) } else if len(signers) > 1 { - err = fmt.Errorf(`Found more than one active signing key for: %v. + err = fmt.Errorf(`Found more than one active private key for key IDs: %v. This is an unsupported and insecure way to store private keys. Please, provide a keys file which contains a single active signing key.`, keyids) } else { @@ -248,7 +252,7 @@ func checkNoTufSigner(root *client.AtsTufRoot, creds OfflineCreds, keyids []stri var signers []TufSigner if signers, err = findTufSigners(root, creds, keyids); err == nil { if len(signers) > 0 { - err = errors.New("It is not allowed to store more than one active signing key into one file") + err = errors.New("It is not allowed to store more than one active private key into one file.") } } return @@ -432,12 +436,12 @@ func signNewTufRoot(curCiRoot, newCiRoot, newProdRoot *client.AtsTufRoot, creds signers := make([]TufSigner, 0, 2) newKey, newErr := FindOneTufSigner(newCiRoot, creds, newCiRoot.Signed.Roles["root"].KeyIDs) if !errors.Is(newErr, errFoundNoKey) { - subcommands.DieNotNil(newErr) + subcommands.DieNotNil(newErr, ErrMsgReadingTufKey(tufRoleNameRoot, "new")) signers = append(signers, newKey) } oldKey, oldErr := FindOneTufSigner(curCiRoot, creds, curCiRoot.Signed.Roles["root"].KeyIDs) if !errors.Is(oldErr, errFoundNoKey) { - subcommands.DieNotNil(oldErr) + subcommands.DieNotNil(oldErr, ErrMsgReadingTufKey(tufRoleNameRoot, "current")) if len(signers) == 0 || oldKey.Id != newKey.Id { signers = append(signers, oldKey) } @@ -445,7 +449,12 @@ func signNewTufRoot(curCiRoot, newCiRoot, newProdRoot *client.AtsTufRoot, creds // At this point either oldKey or newKey was found, or both newErr and oldErr are errFoundNoKey if len(signers) == 0 { - subcommands.DieNotNil(fmt.Errorf("%s\n%s", oldErr, newErr)) + if oldErr.Error() == newErr.Error() { // TUF root key is not being rotated + subcommands.DieNotNil(oldErr, ErrMsgReadingTufKey(tufRoleNameRoot, "current")) + } else { // TUF root key is being rotated + subcommands.DieNotNil(fmt.Errorf( + "%s %s\n %s", ErrMsgReadingTufKey(tufRoleNameRoot, "current and new"), oldErr, newErr)) + } } fmt.Println("= Signing new TUF root") diff --git a/subcommands/waves/init.go b/subcommands/waves/init.go index fb9ff835..d2588a0e 100644 --- a/subcommands/waves/init.go +++ b/subcommands/waves/init.go @@ -222,7 +222,7 @@ Please, run "fioctl keys tuf rotate-offline-key --role=targets" in order to crea } signer, err := keys.FindOneTufSigner(root, offlineKeys, signerKids) - subcommands.DieNotNil(err) + subcommands.DieNotNil(err, keys.ErrMsgReadingTufKey("targets", "current")) signatures, err := keys.SignTufMeta(meta, signer) subcommands.DieNotNil(err, "Failed to sign new targets") return signatures