Skip to content

Commit 60f5e7c

Browse files
committed
refactor(*): stop using []byte that we zero out
In the end, we unavoidably pass strings to age.
1 parent db83e07 commit 60f5e7c

File tree

4 files changed

+36
-68
lines changed

4 files changed

+36
-68
lines changed

cmd/pago/main.go

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,7 @@ func (cmd *AddCmd) Run(config *Config) error {
160160
if generate {
161161
password, err = generatePassword(cmd.Pattern, cmd.Length)
162162
} else {
163-
var passwordBytes []byte
164-
passwordBytes, err = input.ReadNewPassword(config.Confirm)
165-
if err == nil {
166-
defer pago.Zero(passwordBytes)
167-
password = string(passwordBytes)
168-
}
163+
password, err = input.ReadNewPassword(config.Confirm)
169164
}
170165
}
171166
if err != nil {
@@ -318,40 +313,40 @@ func englishPlural(singular, plural string, count int) string {
318313
}
319314

320315
// decryptEntry decrypts a password entry, using the agent if available and configured.
321-
func decryptEntry(agentExecutable string, agentExpire time.Duration, agentMemlock bool, agentSocket, identities, passwordStore, name string) ([]byte, error) {
316+
func decryptEntry(agentExecutable string, agentExpire time.Duration, agentMemlock bool, agentSocket, identities, passwordStore, name string) (string, error) {
322317
if agentSocket == "" {
323318
// Agent is disabled, decrypt directly.
324319
return crypto.DecryptEntry(identities, passwordStore, name)
325320
}
326321

327322
file, err := pago.EntryFile(passwordStore, name)
328323
if err != nil {
329-
return nil, err
324+
return "", err
330325
}
331326

332327
encryptedData, err := os.ReadFile(file)
333328
if err != nil {
334-
return nil, fmt.Errorf("failed to read password file: %v", err)
329+
return "", fmt.Errorf("failed to read password file: %v", err)
335330
}
336331

337332
if err := agent.Ping(agentSocket); err != nil {
338333
// If ping fails, attempt to start the agent.
339334
identitiesText, err := crypto.DecryptIdentities(identities)
340335
if err != nil {
341-
return nil, err
336+
return "", err
342337
}
343338

344339
if err := agent.StartProcess(agentExecutable, agentExpire, agentMemlock, agentSocket, identitiesText); err != nil {
345-
return nil, fmt.Errorf("failed to start agent: %v", err)
340+
return "", fmt.Errorf("failed to start agent: %v", err)
346341
}
347342
}
348343

349344
content, err := agent.Decrypt(agentSocket, encryptedData)
350345
if err != nil {
351-
return nil, err
346+
return "", err
352347
}
353348

354-
return content, nil
349+
return string(content), nil
355350
}
356351

357352
// isTOML returns whether content is a TOML entry.
@@ -383,12 +378,10 @@ func generateOTP(otpURL string) (string, error) {
383378
// getPassword decrypts an entry and returns its content, or a specific key's
384379
// value if it's a TOML entry.
385380
func getPassword(agentExecutable string, agentExpire time.Duration, agentMemlock bool, agentSocket, identities, passwordStore, name, key string) (string, error) {
386-
contentBytes, err := decryptEntry(agentExecutable, agentExpire, agentMemlock, agentSocket, identities, passwordStore, name)
381+
content, err := decryptEntry(agentExecutable, agentExpire, agentMemlock, agentSocket, identities, passwordStore, name)
387382
if err != nil {
388383
return "", err
389384
}
390-
defer pago.Zero(contentBytes)
391-
content := string(contentBytes)
392385

393386
if !isTOML(content) {
394387
if key != "" {
@@ -606,7 +599,7 @@ func (cmd *EditCmd) Run(config *Config) error {
606599

607600
if entryExists(config.Store, name) {
608601
// Decrypt the existing entry content.
609-
contentBytes, err := decryptEntry(
602+
content, err = decryptEntry(
610603
config.AgentExecutable,
611604
config.Expire,
612605
config.Memlock,
@@ -618,8 +611,6 @@ func (cmd *EditCmd) Run(config *Config) error {
618611
if err != nil {
619612
return err
620613
}
621-
622-
content = string(contentBytes)
623614
} else if !cmd.Force {
624615
return fmt.Errorf("entry doesn't exist: %v", name)
625616
}
@@ -742,13 +733,12 @@ func (cmd *InitCmd) Run(config *Config) error {
742733
var buf bytes.Buffer
743734
armorWriter := armor.NewWriter(&buf)
744735

745-
passwordBytes, err := input.ReadNewPassword(config.Confirm)
736+
password, err := input.ReadNewPassword(config.Confirm)
746737
if err != nil {
747738
return fmt.Errorf("failed to read password: %v", err)
748739
}
749-
defer pago.Zero(passwordBytes)
750740

751-
recip, err := age.NewScryptRecipient(string(passwordBytes))
741+
recip, err := age.NewScryptRecipient(password)
752742
if err != nil {
753743
return fmt.Errorf("failed to create scrypt recipient: %w", err)
754744
}
@@ -964,16 +954,15 @@ func (cmd *RewrapCmd) Run(config *Config) error {
964954
return err
965955
}
966956

967-
newPasswordBytes, err := input.ReadNewPassword(config.Confirm)
957+
newPassword, err := input.ReadNewPassword(config.Confirm)
968958
if err != nil {
969959
return err
970960
}
971-
defer pago.Zero(newPasswordBytes)
972961

973962
var buf bytes.Buffer
974963
armorWriter := armor.NewWriter(&buf)
975964

976-
recip, err := age.NewScryptRecipient(string(newPasswordBytes))
965+
recip, err := age.NewScryptRecipient(newPassword)
977966
if err != nil {
978967
return fmt.Errorf("failed to create scrypt recipient: %w", err)
979968
}
@@ -1005,12 +994,10 @@ func (cmd *RewrapCmd) Run(config *Config) error {
1005994

1006995
// getTOMLKeys decrypts a TOML entry and returns a sorted list of its keys.
1007996
func getTOMLKeys(agentExecutable string, agentExpire time.Duration, agentMemlock bool, agentSocket, identities, passwordStore, name string) ([]string, error) {
1008-
contentBytes, err := decryptEntry(agentExecutable, agentExpire, agentMemlock, agentSocket, identities, passwordStore, name)
997+
content, err := decryptEntry(agentExecutable, agentExpire, agentMemlock, agentSocket, identities, passwordStore, name)
1009998
if err != nil {
1010999
return nil, err
10111000
}
1012-
defer pago.Zero(contentBytes)
1013-
content := string(contentBytes)
10141001

10151002
if !isTOML(content) {
10161003
return nil, fmt.Errorf("%q is not a TOML entry; cannot list keys", name)

crypto/crypto.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,13 @@ func DecryptIdentities(identitiesPath string) (string, error) {
188188
return "", fmt.Errorf("failed to read identities file: %v", err)
189189
}
190190

191-
passwordBytes, err := input.SecureRead("Enter password to unlock identities: ")
191+
password, err := input.SecureRead("Enter password to unlock identities: ")
192192
if err != nil {
193193
return "", fmt.Errorf("failed to read password: %v", err)
194194
}
195-
defer pago.Zero(passwordBytes)
196195

197196
// Create a passphrase-based identity and decrypt the private keys with it.
198-
identity, err := age.NewScryptIdentity(string(passwordBytes))
197+
identity, err := age.NewScryptIdentity(password)
199198
if err != nil {
200199
return "", fmt.Errorf("failed to create password-based identity: %v", err)
201200
}
@@ -214,38 +213,38 @@ func DecryptIdentities(identitiesPath string) (string, error) {
214213
}
215214

216215
// DecryptEntry decrypts a password entry from the store using identities from the identities file.
217-
func DecryptEntry(identitiesPath, passwordStore, name string) ([]byte, error) {
216+
func DecryptEntry(identitiesPath, passwordStore, name string) (string, error) {
218217
file, err := pago.EntryFile(passwordStore, name)
219218
if err != nil {
220-
return nil, err
219+
return "", err
221220
}
222221

223222
encryptedData, err := os.ReadFile(file)
224223
if err != nil {
225-
return nil, fmt.Errorf("failed to read password file: %v", err)
224+
return "", fmt.Errorf("failed to read password file: %v", err)
226225
}
227226

228227
identitiesText, err := DecryptIdentities(identitiesPath)
229228
if err != nil {
230-
return nil, err
229+
return "", err
231230
}
232231

233232
ids, err := ParseIdentities(identitiesText)
234233
if err != nil {
235-
return nil, fmt.Errorf("failed to parse identities: %v", err)
234+
return "", fmt.Errorf("failed to parse identities: %v", err)
236235
}
237236

238237
r, err := WrapDecrypt(bytes.NewReader(encryptedData), ids...)
239238
if err != nil {
240-
return nil, fmt.Errorf("failed to decrypt: %v", err)
239+
return "", fmt.Errorf("failed to decrypt: %v", err)
241240
}
242241

243242
content, err := io.ReadAll(r)
244243
if err != nil {
245-
return nil, fmt.Errorf("failed to read decrypted content: %v", err)
244+
return "", fmt.Errorf("failed to read decrypted content: %v", err)
246245
}
247246

248-
return content, nil
247+
return string(content), nil
249248
}
250249

251250
// EntryFile constructs the full file path for a given entry name in the store.

input/input.go

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package input
77

88
import (
99
"bufio"
10-
"bytes"
1110
"errors"
1211
"fmt"
1312
"os"
@@ -50,32 +49,26 @@ func PickEntry(store string, query string) (string, error) {
5049
}
5150

5251
// Read a password without echo if standard input is a terminal.
53-
func SecureRead(prompt string) ([]byte, error) {
52+
func SecureRead(prompt string) (string, error) {
5453
fmt.Fprint(os.Stderr, prompt)
5554

5655
fd := int(os.Stdin.Fd())
5756
if term.IsTerminal(fd) {
5857
password, err := term.ReadPassword(fd)
5958
fmt.Fprintln(os.Stderr)
6059
if err != nil {
61-
return nil, err
60+
return "", err
6261
}
6362

64-
return password, nil
63+
return string(password), nil
6564
}
6665

6766
scanner := bufio.NewScanner(os.Stdin)
6867
if !scanner.Scan() {
69-
return nil, scanner.Err()
68+
return "", scanner.Err()
7069
}
7170

72-
// scanner.Bytes() returns a slice that is valid only until the next Scan().
73-
// We need to copy it to ensure its persistence.
74-
pass := scanner.Bytes()
75-
passCopy := make([]byte, len(pass))
76-
copy(passCopy, pass)
77-
78-
return passCopy, nil
71+
return scanner.Text(), nil
7972
}
8073

8174
// AskYesNo prompts the user with a yes/no question and returns their boolean answer.
@@ -111,27 +104,24 @@ func AskYesNo(prompt string) (bool, error) {
111104

112105
// ReadNewPassword prompts the user to input a new password,
113106
// optionally asking for confirmation by re-entering it.
114-
func ReadNewPassword(confirm bool) ([]byte, error) {
107+
func ReadNewPassword(confirm bool) (string, error) {
115108
pass, err := SecureRead("Enter password: ")
116109
if err != nil {
117-
return nil, err
110+
return "", err
118111
}
119112

120113
if len(pass) == 0 {
121-
return nil, fmt.Errorf("empty password")
114+
return "", fmt.Errorf("empty password")
122115
}
123116

124117
if confirm {
125118
pass2, err := SecureRead("Enter password (again): ")
126119
if err != nil {
127-
pago.Zero(pass)
128-
return nil, err
120+
return "", err
129121
}
130-
defer pago.Zero(pass2)
131122

132-
if !bytes.Equal(pass, pass2) {
133-
pago.Zero(pass)
134-
return nil, fmt.Errorf("passwords do not match")
123+
if pass != pass2 {
124+
return "", fmt.Errorf("passwords do not match")
135125
}
136126
}
137127

util.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,6 @@ func ExitWithError(format string, value any) {
6464
os.Exit(1)
6565
}
6666

67-
// Zero zeroes-out a byte slice.
68-
// This is used for passwords.
69-
func Zero(b []byte) {
70-
for i := range b {
71-
b[i] = 0
72-
}
73-
}
74-
7567
// ListFiles walks a directory tree and returns a list of file names
7668
// that satisfy a given transformation/filter function.
7769
func ListFiles(root string, transform func(name string, info os.FileInfo) (bool, string)) ([]string, error) {

0 commit comments

Comments
 (0)