Merge pull request #906 from docker/passphrase-retrieval-fixups

Passphrase fixups

cmd/notary/tuf.go

@@ -403,12 +403,12 @@ func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error {
 	if len(rootKeyList) < 1 {
 		cmd.Println("No root keys found. Generating a new root key...")
 		rootPublicKey, err := nRepo.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey)
-		rootKeyID = rootPublicKey.ID()
 		if err != nil {
 			return err
 		}
+		rootKeyID = rootPublicKey.ID()
 	} else {
-		// Choses the first root key available, which is initialization specific
+		// Chooses the first root key available, which is initialization specific
 		// but should return the HW one first.
 		rootKeyID = rootKeyList[0]
 		cmd.Printf("Root key found, using: %s\n", rootKeyID)

passphrase/passphrase.go

@@ -40,12 +40,22 @@ var (
 	// ErrTooManyAttempts is returned if the maximum number of passphrase
 	// entry attempts is reached.
 	ErrTooManyAttempts = errors.New("Too many attempts")
+
+	// ErrNoInput is returned if we do not have a valid input method for passphrases
+	ErrNoInput = errors.New("Please either use environment variables or STDIN with a terminal to provide key passphrases")
 )
 
 // PromptRetriever returns a new Retriever which will provide a prompt on stdin
-// and stdout to retrieve a passphrase. The passphrase will be cached such that
+// and stdout to retrieve a passphrase. stdin will be checked if it is a terminal,
+// else the PromptRetriever will error when attempting to retrieve a passphrase.
+// Upon successful passphrase retrievals, the passphrase will be cached such that
 // subsequent prompts will produce the same passphrase.
 func PromptRetriever() notary.PassRetriever {
+	if !term.IsTerminal(os.Stdin.Fd()) {
+		return func(string, string, bool, int) (string, bool, error) {
+			return "", false, ErrNoInput
+		}
+	}
 	return PromptRetrieverWithInOut(os.Stdin, os.Stdout, nil)
 }
 
@@ -85,13 +95,13 @@ func (br *boundRetriever) requestPassphrase(keyName, alias string, createNew boo
 
 	// 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 term.IsTerminal(os.Stdin.Fd()) {
+		state, err := term.SaveState(os.Stdin.Fd())
 		if err != nil {
 			return "", false, err
 		}
-		term.DisableEcho(0, state)
-		defer term.RestoreTerminal(0, state)
+		term.DisableEcho(os.Stdin.Fd(), state)
+		defer term.RestoreTerminal(os.Stdin.Fd(), state)
 	}
 
 	indexOfLastSeparator := strings.LastIndex(keyName, string(filepath.Separator))

passphrase/passphrase_test.go

@@ -42,6 +42,38 @@ func TestGetPassphraseForUsingDelegationKey(t *testing.T) {
 	}
 }
 
+// PromptRetrieverWithInOut prompts for passwords up to 10 times when creating
+func TestGetPassphraseLimitsShortPassphrases(t *testing.T) {
+	var in bytes.Buffer
+	var out bytes.Buffer
+
+	retriever := PromptRetrieverWithInOut(&in, &out, nil)
+
+	repeatedShortPass := strings.Repeat("a\n", 22)
+	_, err := in.WriteString(repeatedShortPass)
+	require.NoError(t, err)
+
+	_, _, err = retriever("randomRepo", "targets/randomRole", true, 0)
+	require.Error(t, err)
+	require.IsType(t, ErrTooManyAttempts, err)
+}
+
+// PromptRetrieverWithInOut prompts for passwords up to 10 times when creating
+func TestGetPassphraseLimitsMismatchingPassphrases(t *testing.T) {
+	var in bytes.Buffer
+	var out bytes.Buffer
+
+	retriever := PromptRetrieverWithInOut(&in, &out, nil)
+
+	repeatedShortPass := strings.Repeat("password\nmismatchingpass\n", 11)
+	_, err := in.WriteString(repeatedShortPass)
+	require.NoError(t, err)
+
+	_, _, err = retriever("randomRepo", "targets/randomRole", true, 0)
+	require.Error(t, err)
+	require.IsType(t, ErrTooManyAttempts, err)
+}
+
 // PromptRetrieverWithInOut prompts for creating delegations passwords if needed
 func TestGetPassphraseForCreatingDelegationKey(t *testing.T) {
 	var in bytes.Buffer
@@ -113,3 +145,11 @@ func TestRolePromptingAndCaching(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(text), "Enter passphrase for targets/delegation/new key with ID 0123456 (repo):")
 }
+
+// TestPromptRetrieverNeedsTerminal checks that PromptRetriever errors when not run with a terminal stdin
+func TestPromptRetrieverNeedsTerminal(t *testing.T) {
+	prompt := PromptRetriever()
+	_, _, err := prompt("repo/0123456789abcdef", "targets/delegation/new", false, 0)
+	require.Error(t, err)
+	require.IsType(t, ErrNoInput, err)
+}

trustmanager/keystore.go

@@ -142,13 +142,12 @@ func (s *GenericKeyStore) AddKey(keyInfo KeyInfo, privKey data.PrivateKey) error
 	name := filepath.Join(keyInfo.Gun, privKey.ID())
 	for attempts := 0; ; attempts++ {
 		chosenPassphrase, giveup, err = s.PassRetriever(name, keyInfo.Role, true, attempts)
-		if err != nil {
-			continue
+		if err == nil {
+			break
 		}
 		if giveup || attempts > 10 {
 			return ErrAttemptsExceeded{}
 		}
-		break
 	}
 
 	if chosenPassphrase != "" {