diff --git a/client/client_test.go b/client/client_test.go index e42e68067..6daffaafa 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2422,6 +2422,7 @@ func TestPublishRemoveDelegationKeyFromDelegationRole(t *testing.T) { require.NoError(t, err) cl, err := changelist.NewFileChangelist(filepath.Join(ownerRepo.tufRepoPath, "changelist")) + require.NoError(t, err) require.NoError(t, cl.Add(changelist.NewTUFChange( changelist.ActionUpdate, "targets/a", @@ -3055,6 +3056,7 @@ func TestFullAddDelegationChangefileApplicable(t *testing.T) { }) change := newCreateDelegationChange(delegationName, tdJSON) cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist")) + require.NoError(t, err) addChange(cl, change, delegationName) changes := getChanges(t, repo) @@ -3104,6 +3106,7 @@ func TestFullRemoveDelegationChangefileApplicable(t *testing.T) { }) change := newUpdateDelegationChange(delegationName, tdJSON) cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist")) + require.NoError(t, err) addChange(cl, change, delegationName) changes = getChanges(t, repo) diff --git a/passphrase/passphrase.go b/passphrase/passphrase.go index 349329e0a..432500361 100644 --- a/passphrase/passphrase.go +++ b/passphrase/passphrase.go @@ -49,141 +49,157 @@ func PromptRetriever() notary.PassRetriever { return PromptRetrieverWithInOut(os.Stdin, os.Stdout, nil) } -// PromptRetrieverWithInOut returns a new Retriever which will provide a -// prompt using the given in and out readers. The passphrase will be cached -// such that subsequent prompts will produce the same passphrase. -// aliasMap can be used to specify display names for TUF key aliases. If aliasMap -// is nil, a sensible default will be used. -func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]string) notary.PassRetriever { - userEnteredTargetsSnapshotsPass := false - targetsSnapshotsPass := "" - userEnteredRootsPass := false - rootsPass := "" - - return func(keyName string, alias string, createNew bool, numAttempts int) (string, bool, error) { - if alias == tufRootAlias && createNew && numAttempts == 0 { - fmt.Fprintln(out, tufRootKeyGenerationWarning) - } - if numAttempts > 0 { - if !createNew { - fmt.Fprintln(out, "Passphrase incorrect. Please retry.") - } - } - - // Figure out if we should display a different string for this alias - displayAlias := alias - if aliasMap != nil { - if val, ok := aliasMap[alias]; ok { - displayAlias = val - } +type boundRetriever struct { + in io.Reader + out io.Writer + aliasMap map[string]string + userEnteredTargetsSnapshotPass bool + targetsSnapshotPass string + userEnteredRootPass bool + rootPass string +} +func (br *boundRetriever) getPassphrase(keyName, alias string, createNew bool, numAttempts int) (string, bool, error) { + if numAttempts == 0 { + if alias == tufRootAlias && createNew { + fmt.Fprintln(br.out, tufRootKeyGenerationWarning) } - // First, check if we have a password cached for this alias. - if numAttempts == 0 { - if userEnteredTargetsSnapshotsPass && (alias == tufSnapshotAlias || alias == tufTargetsAlias) { - return targetsSnapshotsPass, false, nil - } - if userEnteredRootsPass && (alias == "root") { - return rootsPass, false, nil - } + if br.userEnteredTargetsSnapshotPass && (alias == tufSnapshotAlias || alias == tufTargetsAlias) { + return br.targetsSnapshotPass, false, nil } - - if numAttempts > 3 && !createNew { - return "", true, ErrTooManyAttempts + if br.userEnteredRootPass && (alias == "root") { + return br.rootPass, false, nil } - - // If typing on the terminal, we do not want the terminal to echo the - // password that is typed (so it doesn't display) - if term.IsTerminal(0) { - state, err := term.SaveState(0) - if err != nil { - return "", false, err - } - term.DisableEcho(0, state) - defer term.RestoreTerminal(0, state) + } else if !createNew { // per `if`, numAttempts > 0 if we're at this `else` + if numAttempts > 3 { + return "", true, ErrTooManyAttempts } + fmt.Fprintln(br.out, "Passphrase incorrect. Please retry.") + } - stdin := bufio.NewReader(in) + // passphrase not cached and we're not aborting, get passphrase from user! + return br.requestPassphrase(keyName, alias, createNew, numAttempts) +} - indexOfLastSeparator := strings.LastIndex(keyName, string(filepath.Separator)) - if indexOfLastSeparator == -1 { - indexOfLastSeparator = 0 - } +func (br *boundRetriever) requestPassphrase(keyName, alias string, createNew bool, numAttempts int) (string, bool, error) { + // Figure out if we should display a different string for this alias + displayAlias := alias + if val, ok := br.aliasMap[alias]; ok { + displayAlias = val + } - var shortName string - if len(keyName) > indexOfLastSeparator+idBytesToDisplay { - if indexOfLastSeparator > 0 { - keyNamePrefix := keyName[:indexOfLastSeparator] - keyNameID := keyName[indexOfLastSeparator+1 : indexOfLastSeparator+idBytesToDisplay+1] - shortName = keyNameID + " (" + keyNamePrefix + ")" - } else { - shortName = keyName[indexOfLastSeparator : indexOfLastSeparator+idBytesToDisplay] - } + // If typing on the terminal, we do not want the terminal to echo the + // password that is typed (so it doesn't display) + if term.IsTerminal(0) { + state, err := term.SaveState(0) + if err != nil { + return "", false, err } + term.DisableEcho(0, state) + defer term.RestoreTerminal(0, state) + } - withID := fmt.Sprintf(" with ID %s", shortName) - if shortName == "" { - withID = "" - } + indexOfLastSeparator := strings.LastIndex(keyName, string(filepath.Separator)) + if indexOfLastSeparator == -1 { + indexOfLastSeparator = 0 + } - if createNew { - fmt.Fprintf(out, "Enter passphrase for new %s key%s: ", displayAlias, withID) - } else if displayAlias == "yubikey" { - fmt.Fprintf(out, "Enter the %s for the attached Yubikey: ", keyName) + var shortName string + if len(keyName) > indexOfLastSeparator+idBytesToDisplay { + if indexOfLastSeparator > 0 { + keyNamePrefix := keyName[:indexOfLastSeparator] + keyNameID := keyName[indexOfLastSeparator+1 : indexOfLastSeparator+idBytesToDisplay+1] + shortName = keyNameID + " (" + keyNamePrefix + ")" } else { - fmt.Fprintf(out, "Enter passphrase for %s key%s: ", displayAlias, withID) + shortName = keyName[indexOfLastSeparator : indexOfLastSeparator+idBytesToDisplay] } + } - passphrase, err := stdin.ReadBytes('\n') - fmt.Fprintln(out) - if err != nil { - return "", false, err - } + withID := fmt.Sprintf(" with ID %s", shortName) + if shortName == "" { + withID = "" + } - retPass := strings.TrimSpace(string(passphrase)) - - if !createNew { - if alias == tufSnapshotAlias || alias == tufTargetsAlias { - userEnteredTargetsSnapshotsPass = true - targetsSnapshotsPass = retPass - } - if alias == tufRootAlias { - userEnteredRootsPass = true - rootsPass = retPass - } - return retPass, false, nil - } + switch { + case createNew: + fmt.Fprintf(br.out, "Enter passphrase for new %s key%s: ", displayAlias, withID) + case displayAlias == "yubikey": + fmt.Fprintf(br.out, "Enter the %s for the attached Yubikey: ", keyName) + default: + fmt.Fprintf(br.out, "Enter passphrase for %s key%s: ", displayAlias, withID) + } - if len(retPass) < 8 { - fmt.Fprintln(out, "Passphrase is too short. Please use a password manager to generate and store a good random passphrase.") - return "", false, ErrTooShort - } + stdin := bufio.NewReader(br.in) + passphrase, err := stdin.ReadBytes('\n') + fmt.Fprintln(br.out) + if err != nil { + return "", false, err + } - fmt.Fprintf(out, "Repeat passphrase for new %s key%s: ", displayAlias, withID) - confirmation, err := stdin.ReadBytes('\n') - fmt.Fprintln(out) + retPass := strings.TrimSpace(string(passphrase)) + + if createNew { + err = br.verifyAndConfirmPassword(stdin, retPass, displayAlias, withID) if err != nil { return "", false, err } - confirmationStr := strings.TrimSpace(string(confirmation)) + } - if retPass != confirmationStr { - fmt.Fprintln(out, "Passphrases do not match. Please retry.") - return "", false, ErrDontMatch - } + br.cachePassword(alias, retPass) - if alias == tufSnapshotAlias || alias == tufTargetsAlias { - userEnteredTargetsSnapshotsPass = true - targetsSnapshotsPass = retPass - } - if alias == tufRootAlias { - userEnteredRootsPass = true - rootsPass = retPass - } + return retPass, false, nil +} + +func (br *boundRetriever) verifyAndConfirmPassword(stdin *bufio.Reader, retPass, displayAlias, withID string) error { + if len(retPass) < 8 { + fmt.Fprintln(br.out, "Passphrase is too short. Please use a password manager to generate and store a good random passphrase.") + return ErrTooShort + } + + fmt.Fprintf(br.out, "Repeat passphrase for new %s key%s: ", displayAlias, withID) + confirmation, err := stdin.ReadBytes('\n') + fmt.Fprintln(br.out) + if err != nil { + return err + } + confirmationStr := strings.TrimSpace(string(confirmation)) - return retPass, false, nil + if retPass != confirmationStr { + fmt.Fprintln(br.out, "Passphrases do not match. Please retry.") + return ErrDontMatch } + return nil +} + +func (br *boundRetriever) cachePassword(alias, retPass string) { + if alias == tufSnapshotAlias || alias == tufTargetsAlias { + br.userEnteredTargetsSnapshotPass = true + br.targetsSnapshotPass = retPass + } + if alias == tufRootAlias { + br.userEnteredRootPass = true + br.rootPass = retPass + } +} + +// PromptRetrieverWithInOut returns a new Retriever which will provide a +// prompt using the given in and out readers. The passphrase will be cached +// such that subsequent prompts will produce the same passphrase. +// aliasMap can be used to specify display names for TUF key aliases. If aliasMap +// is nil, a sensible default will be used. +func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]string) notary.PassRetriever { + bound := &boundRetriever{ + in: in, + out: out, + aliasMap: aliasMap, + userEnteredTargetsSnapshotPass: false, + targetsSnapshotPass: "", + userEnteredRootPass: false, + rootPass: "", + } + + return bound.getPassphrase } // ConstantRetriever returns a new Retriever which will return a constant string diff --git a/server/handlers/validation_test.go b/server/handlers/validation_test.go index 906fcb44c..e841c29c7 100644 --- a/server/handlers/validation_test.go +++ b/server/handlers/validation_test.go @@ -226,7 +226,7 @@ func TestValidateGetCurrentTimestampBroken(t *testing.T) { } serverCrypto := testutils.CopyKeys(t, cs, data.CanonicalTimestampRole) - updates, err = validateUpdate(serverCrypto, gun, updates, store) + _, err = validateUpdate(serverCrypto, gun, updates, store) require.Error(t, err) require.IsType(t, data.ErrNoSuchRole{}, err) } @@ -624,7 +624,7 @@ func TestRootRotationNotSignedWithOldKeysForOldRole(t *testing.T) { sn, err = repo.SignSnapshot(data.DefaultExpires(data.CanonicalSnapshotRole)) require.NoError(t, err) - root, targets, snapshot, timestamp, err = getUpdates(r, tg, sn, ts) + root, _, snapshot, _, err = getUpdates(r, tg, sn, ts) require.NoError(t, err) _, err = validateUpdate(serverCrypto, gun, []storage.MetaUpdate{root, snapshot}, store) @@ -730,6 +730,7 @@ func TestValidateSnapshotGenerateWithPrev(t *testing.T) { if u.Role == data.CanonicalSnapshotRole { curr := &data.SignedSnapshot{} err = json.Unmarshal(u.Data, curr) + require.NoError(t, err) require.Equal(t, prev.Signed.Version+1, curr.Signed.Version) require.Equal(t, u.Version, curr.Signed.Version) } @@ -763,7 +764,7 @@ func TestValidateSnapshotGeneratePrevCorrupt(t *testing.T) { store.UpdateCurrent(gun, snapshot) serverCrypto := testutils.CopyKeys(t, cs, data.CanonicalTimestampRole, data.CanonicalSnapshotRole) - updates, err = validateUpdate(serverCrypto, gun, updates, store) + _, err = validateUpdate(serverCrypto, gun, updates, store) require.Error(t, err) require.IsType(t, &json.SyntaxError{}, err) } @@ -819,7 +820,7 @@ func TestValidateSnapshotGenerateNoTargets(t *testing.T) { updates := []storage.MetaUpdate{root} serverCrypto := testutils.CopyKeys(t, cs, data.CanonicalTimestampRole, data.CanonicalSnapshotRole) - updates, err = validateUpdate(serverCrypto, gun, updates, store) + _, err = validateUpdate(serverCrypto, gun, updates, store) require.Error(t, err) } @@ -846,7 +847,7 @@ func TestValidateSnapshotGenerate(t *testing.T) { store.UpdateCurrent(gun, root) serverCrypto := testutils.CopyKeys(t, cs, data.CanonicalTimestampRole, data.CanonicalSnapshotRole) - updates, err = validateUpdate(serverCrypto, gun, updates, store) + _, err = validateUpdate(serverCrypto, gun, updates, store) require.NoError(t, err) } diff --git a/server/storage/database_test.go b/server/storage/database_test.go index 88bb51574..2c3657a58 100644 --- a/server/storage/database_test.go +++ b/server/storage/database_test.go @@ -298,6 +298,7 @@ func TestSQLGetKeyNoKey(t *testing.T) { t, query.Error, "Inserting timestamp into empty DB should succeed") cipher, public, err = dbStore.GetKey("testGUN", data.CanonicalTimestampRole) + require.NoError(t, err) require.Equal(t, "testCipher", cipher, "Returned cipher was incorrect") require.Equal(t, []byte("1"), public, "Returned pubkey was incorrect") diff --git a/signer/api/api_test.go b/signer/api/api_test.go index 3f2d54266..fec2dd63f 100644 --- a/signer/api/api_test.go +++ b/signer/api/api_test.go @@ -199,7 +199,7 @@ func TestSoftwareSignWithInvalidRequestHandler(t *testing.T) { var sig *pb.Signature err = json.Unmarshal(jsonBlob, &sig) - + require.Error(t, err) require.Equal(t, 400, res.StatusCode) } diff --git a/trustmanager/x509utils_test.go b/trustmanager/x509utils_test.go index 383b8fef0..11710cb38 100644 --- a/trustmanager/x509utils_test.go +++ b/trustmanager/x509utils_test.go @@ -187,7 +187,7 @@ func TestRSAX509PublickeyID(t *testing.T) { actualTUFKey := CertToKey(cert) actualTUFID, err := X509PublicKeyID(actualTUFKey) - + require.NoError(t, err) require.Equal(t, sameWayTUFID, actualTUFID) require.Equal(t, expectedTUFID, actualTUFID) } diff --git a/trustmanager/yubikey/yubikeystore.go b/trustmanager/yubikey/yubikeystore.go index 0d351bdeb..a45772863 100644 --- a/trustmanager/yubikey/yubikeystore.go +++ b/trustmanager/yubikey/yubikeystore.go @@ -902,13 +902,12 @@ func login(ctx IPKCS11Ctx, session pkcs11.SessionHandle, passRetriever notary.Pa return trustmanager.ErrAttemptsExceeded{} } - // Try to convert PEM encoded bytes back to a PrivateKey using the passphrase + // attempt to login. Loop if failed err = ctx.Login(session, userFlag, passwd) if err == nil { return nil } } - return nil } func buildKeyMap(keys map[string]yubiSlot) map[string]trustmanager.KeyInfo {