Skip to content

Commit d3bb109

Browse files
felixfonteinfelixfontein
authored
Merge pull request #1931 from felixfontein/age-error
Improve age identity loading
2 parents 92e5367 + e311a4f commit d3bb109

File tree

3 files changed

+114
-60
lines changed

3 files changed

+114
-60
lines changed

‎age/encrypted_keys.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (i *LazyScryptIdentity) Unwrap(stanzas []*age.Stanza) (fileKey []byte, err
104104
return fileKey, err
105105
}
106106

107-
func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
107+
func unwrapIdentities(location string, reader io.Reader) (ParsedIdentities, error) {
108108
b := bufio.NewReader(reader)
109109
p, _ := b.Peek(14) // length of "age-encryption" and "-----BEGIN AGE"
110110
peeked := string(p)
@@ -119,10 +119,10 @@ func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
119119
const privateKeySizeLimit = 1 << 24 // 16 MiB
120120
contents, err := io.ReadAll(io.LimitReader(r, privateKeySizeLimit))
121121
if err != nil {
122-
return nil, fmt.Errorf("failed to read '%s': %w", key, err)
122+
return nil, fmt.Errorf("failed to read '%s': %w", location, err)
123123
}
124124
if len(contents) == privateKeySizeLimit {
125-
return nil, fmt.Errorf("failed to read '%s': file too long", key)
125+
return nil, fmt.Errorf("failed to read '%s': file too long", location)
126126
}
127127
IncorrectPassphrase := func() {
128128
conn, err := gpgagent.NewConn()
@@ -134,7 +134,7 @@ func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
134134
log.Errorf("failed to close connection with gpg-agent: %s", err)
135135
}
136136
}(conn)
137-
err = conn.RemoveFromCache(key)
137+
err = conn.RemoveFromCache(location)
138138
if err != nil {
139139
log.Warnf("gpg-agent remove cache request errored: %s", err)
140140
return
@@ -145,7 +145,7 @@ func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
145145
Passphrase: func() (string, error) {
146146
conn, err := gpgagent.NewConn()
147147
if err != nil {
148-
passphrase, err := readSecret("Enter passphrase for identity " + key + ":")
148+
passphrase, err := readSecret(fmt.Sprintf("Enter passphrase for identity '%s':", location))
149149
if err != nil {
150150
return "", err
151151
}
@@ -159,9 +159,9 @@ func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
159159

160160
req := gpgagent.PassphraseRequest{
161161
// TODO is the cachekey good enough?
162-
CacheKey: key,
162+
CacheKey: location,
163163
Prompt: "Passphrase",
164-
Desc: fmt.Sprintf("Enter passphrase for identity '%s':", key),
164+
Desc: fmt.Sprintf("Enter passphrase for identity '%s':", location),
165165
}
166166
pass, err := conn.GetPassphrase(&req)
167167
if err != nil {
@@ -175,15 +175,15 @@ func unwrapIdentities(key string, reader io.Reader) (ParsedIdentities, error) {
175175
},
176176
IncorrectPassphrase: IncorrectPassphrase,
177177
NoMatchWarning: func() {
178-
log.Warnf("encrypted identity '%s' didn't match file's recipients", key)
178+
log.Warnf("encrypted identity '%s' didn't match file's recipients", location)
179179
},
180180
}}
181181
return ids, nil
182182
// An unencrypted age identity file.
183183
default:
184184
ids, err := parseIdentities(b)
185185
if err != nil {
186-
return nil, fmt.Errorf("failed to parse '%s' age identities: %w", key, err)
186+
return nil, fmt.Errorf("failed to parse '%s' age identities: %w", location, err)
187187
}
188188
return ids, nil
189189
}

‎age/keysource.go‎

Lines changed: 83 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -206,16 +206,41 @@ func (key *MasterKey) SetEncryptedDataKey(enc []byte) {
206206
key.EncryptedKey = string(enc)
207207
}
208208

209+
func formatError(msg string, err error, errs errSet, unusedLocations []string) error {
210+
var loadSuffix string
211+
if len(errs) > 0 {
212+
loadSuffix = fmt.Sprintf(". Errors while loading age identities: %s", errs.Error())
213+
}
214+
var unusedSuffix string
215+
if len(unusedLocations) > 0 {
216+
count := len(unusedLocations)
217+
if count == 1 {
218+
unusedSuffix = fmt.Sprintf(" '%s'", unusedLocations[0])
219+
} else if count == 2 {
220+
unusedSuffix = fmt.Sprintf("s '%s' and '%s'", unusedLocations[0], unusedLocations[1])
221+
} else {
222+
unusedSuffix = fmt.Sprintf("s '%s', and '%s'", strings.Join(unusedLocations[:count - 1], "', '"), unusedLocations[count - 1])
223+
}
224+
unusedSuffix = fmt.Sprintf(". Did not find keys in location%s.", unusedSuffix)
225+
}
226+
if err != nil {
227+
return fmt.Errorf("%s: %w%s%s", msg, err, loadSuffix, unusedSuffix)
228+
} else {
229+
return fmt.Errorf("%s%s%s", msg, loadSuffix, unusedSuffix)
230+
}
231+
}
232+
209233
// Decrypt decrypts the EncryptedKey with the parsed or loaded identities, and
210234
// returns the result.
211235
func (key *MasterKey) Decrypt() ([]byte, error) {
212236
var errs errSet
237+
var unusedLocations []string
213238
if len(key.parsedIdentities) == 0 {
214239
var ids ParsedIdentities
215-
ids, errs = key.loadIdentities()
240+
ids, unusedLocations, errs = key.loadIdentities()
216241
if len(ids) == 0 {
217242
log.Info("Decryption failed")
218-
return nil, fmt.Errorf("failed to load age identities: %w", errs)
243+
return nil, formatError("failed to load age identities", nil, errs, unusedLocations)
219244
}
220245
ids.ApplyToMasterKey(key)
221246
}
@@ -225,11 +250,7 @@ func (key *MasterKey) Decrypt() ([]byte, error) {
225250
r, err := age.Decrypt(ar, key.parsedIdentities...)
226251
if err != nil {
227252
log.Info("Decryption failed")
228-
var loadErrors string
229-
if len(errs) > 0 {
230-
loadErrors = fmt.Sprintf(". Errors while loading age identities: %s", errs.Error())
231-
}
232-
return nil, fmt.Errorf("failed to create reader for decrypting sops data key with age: %w%s", err, loadErrors)
253+
return nil, formatError("failed to create reader for decrypting sops data key with age", err, errs, unusedLocations)
233254
}
234255

235256
var b bytes.Buffer
@@ -269,29 +290,55 @@ func (key *MasterKey) TypeToIdentifier() string {
269290
// private key from the SopsAgeSshPrivateKeyFileEnv environment variable. If the
270291
// environment variable is not present, it will fall back to `~/.ssh/id_ed25519`
271292
// or `~/.ssh/id_rsa`. If no age SSH identity is found, it will return nil.
272-
func loadAgeSSHIdentity() (age.Identity, error) {
293+
func loadAgeSSHIdentities() ([]age.Identity, []string, errSet) {
294+
var identities []age.Identity
295+
var unusedLocations []string
296+
var errs errSet
297+
273298
sshKeyFilePath, ok := os.LookupEnv(SopsAgeSshPrivateKeyFileEnv)
274299
if ok {
275-
return parseSSHIdentityFromPrivateKeyFile(sshKeyFilePath)
300+
identity, err := parseSSHIdentityFromPrivateKeyFile(sshKeyFilePath)
301+
if err != nil {
302+
errs = append(errs, err)
303+
} else {
304+
identities = append(identities, identity)
305+
}
306+
} else {
307+
unusedLocations = append(unusedLocations, SopsAgeSshPrivateKeyFileEnv)
276308
}
277309

278310
userHomeDir, err := os.UserHomeDir()
279-
if err != nil || userHomeDir == "" {
311+
if err != nil {
312+
errs = append(errs, err)
313+
} else if userHomeDir == "" {
280314
log.Warnf("could not determine the user home directory: %v", err)
281-
return nil, nil
282-
}
283-
284-
sshEd25519PrivateKeyPath := filepath.Join(userHomeDir, ".ssh", "id_ed25519")
285-
if _, err := os.Stat(sshEd25519PrivateKeyPath); err == nil {
286-
return parseSSHIdentityFromPrivateKeyFile(sshEd25519PrivateKeyPath)
287-
}
315+
} else {
316+
sshEd25519PrivateKeyPath := filepath.Join(userHomeDir, ".ssh", "id_ed25519")
317+
if _, err := os.Stat(sshEd25519PrivateKeyPath); err == nil {
318+
identity, err := parseSSHIdentityFromPrivateKeyFile(sshEd25519PrivateKeyPath)
319+
if err != nil {
320+
errs = append(errs, err)
321+
} else {
322+
identities = append(identities, identity)
323+
}
324+
} else {
325+
unusedLocations = append(unusedLocations, sshEd25519PrivateKeyPath)
326+
}
288327

289-
sshRsaPrivateKeyPath := filepath.Join(userHomeDir, ".ssh", "id_rsa")
290-
if _, err := os.Stat(sshRsaPrivateKeyPath); err == nil {
291-
return parseSSHIdentityFromPrivateKeyFile(sshRsaPrivateKeyPath)
328+
sshRsaPrivateKeyPath := filepath.Join(userHomeDir, ".ssh", "id_rsa")
329+
if _, err := os.Stat(sshRsaPrivateKeyPath); err == nil {
330+
identity, err := parseSSHIdentityFromPrivateKeyFile(sshRsaPrivateKeyPath)
331+
if err != nil {
332+
errs = append(errs, err)
333+
} else {
334+
identities = append(identities, identity)
335+
}
336+
} else {
337+
unusedLocations = append(unusedLocations, sshRsaPrivateKeyPath)
338+
}
292339
}
293340

294-
return nil, nil
341+
return identities, unusedLocations, errs
295342
}
296343

297344
func getUserConfigDir() (string, error) {
@@ -307,22 +354,15 @@ func getUserConfigDir() (string, error) {
307354
// environment configurations (e.g. SopsAgeKeyEnv, SopsAgeKeyFileEnv,
308355
// SopsAgeSshPrivateKeyFileEnv, SopsAgeKeyUserConfigPath). It will load all
309356
// found references, and expects at least one configuration to be present.
310-
func (key *MasterKey) loadIdentities() (ParsedIdentities, errSet) {
311-
var identities ParsedIdentities
312-
313-
var errs errSet
314-
315-
sshIdentity, err := loadAgeSSHIdentity()
316-
if err != nil {
317-
errs = append(errs, fmt.Errorf("failed to get SSH identity: %w", err))
318-
} else if sshIdentity != nil {
319-
identities = append(identities, sshIdentity)
320-
}
357+
func (key *MasterKey) loadIdentities() (ParsedIdentities, []string, errSet) {
358+
identities, unusedLocations, errs := loadAgeSSHIdentities()
321359

322360
var readers = make(map[string]io.Reader, 0)
323361

324362
if ageKey, ok := os.LookupEnv(SopsAgeKeyEnv); ok {
325363
readers[SopsAgeKeyEnv] = strings.NewReader(ageKey)
364+
} else {
365+
unusedLocations = append(unusedLocations, SopsAgeKeyEnv)
326366
}
327367

328368
if ageKeyFile, ok := os.LookupEnv(SopsAgeKeyFileEnv); ok {
@@ -333,6 +373,8 @@ func (key *MasterKey) loadIdentities() (ParsedIdentities, errSet) {
333373
defer f.Close()
334374
readers[SopsAgeKeyFileEnv] = f
335375
}
376+
} else {
377+
unusedLocations = append(unusedLocations, SopsAgeKeyFileEnv)
336378
}
337379

338380
if ageKeyCmd, ok := os.LookupEnv(SopsAgeKeyCmdEnv); ok {
@@ -347,6 +389,8 @@ func (key *MasterKey) loadIdentities() (ParsedIdentities, errSet) {
347389
readers[SopsAgeKeyCmdEnv] = bytes.NewReader(out)
348390
}
349391
}
392+
} else {
393+
unusedLocations = append(unusedLocations, SopsAgeKeyCmdEnv)
350394
}
351395

352396
userConfigDir, err := getUserConfigDir()
@@ -358,23 +402,25 @@ func (key *MasterKey) loadIdentities() (ParsedIdentities, errSet) {
358402
if err != nil && !errors.Is(err, os.ErrNotExist) {
359403
errs = append(errs, fmt.Errorf("failed to open file: %w", err))
360404
} else if errors.Is(err, os.ErrNotExist) && len(readers) == 0 && len(identities) == 0 {
361-
// If we have no other readers, presence of the file is required.
362-
errs = append(errs, fmt.Errorf("failed to open file: %w", err))
405+
unusedLocations = append(unusedLocations, ageKeyFilePath)
363406
} else if err == nil {
364407
defer f.Close()
365408
readers[ageKeyFilePath] = f
366409
}
367410
}
368411

369-
for n, r := range readers {
370-
ids, err := unwrapIdentities(n, r)
412+
for location, r := range readers {
413+
ids, err := unwrapIdentities(location, r)
371414
if err != nil {
372415
errs = append(errs, err)
373416
} else {
374417
identities = append(identities, ids...)
418+
if len(ids) == 0 {
419+
unusedLocations = append(unusedLocations, location)
420+
}
375421
}
376422
}
377-
return identities, errs
423+
return identities, unusedLocations, errs
378424
}
379425

380426
// parseRecipient attempts to parse a string containing an encoded age public

‎age/keysource_test.go‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
369369
t.Setenv(SopsAgeKeyEnv, mockIdentity)
370370

371371
key := &MasterKey{}
372-
got, errs := key.loadIdentities()
372+
got, unusedLocations, errs := key.loadIdentities()
373373
assert.Len(t, errs, 0)
374374
assert.Len(t, got, 1)
375+
assert.Len(t, unusedLocations, 5)
375376
})
376377

377378
t.Run(SopsAgeKeyEnv+" multiple", func(t *testing.T) {
@@ -382,9 +383,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
382383
t.Setenv(SopsAgeKeyEnv, mockIdentity+"\n"+mockOtherIdentity)
383384

384385
key := &MasterKey{}
385-
got, errs := key.loadIdentities()
386+
got, unusedLocations, errs := key.loadIdentities()
386387
assert.Len(t, errs, 0)
387388
assert.Len(t, got, 2)
389+
assert.Len(t, unusedLocations, 5)
388390
})
389391

390392
t.Run(SopsAgeKeyFileEnv, func(t *testing.T) {
@@ -398,9 +400,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
398400
t.Setenv(SopsAgeKeyFileEnv, keyPath)
399401

400402
key := &MasterKey{}
401-
got, errs := key.loadIdentities()
403+
got, unusedLocations, errs := key.loadIdentities()
402404
assert.Len(t, errs, 0)
403405
assert.Len(t, got, 1)
406+
assert.Len(t, unusedLocations, 5)
404407
})
405408

406409
t.Run(SopsAgeKeyUserConfigPath, func(t *testing.T) {
@@ -416,9 +419,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
416419
assert.NoError(t, os.MkdirAll(filepath.Dir(keyPath), 0o700))
417420
assert.NoError(t, os.WriteFile(keyPath, []byte(mockIdentity), 0o644))
418421

419-
got, errs := (&MasterKey{}).loadIdentities()
422+
got, unusedLocations, errs := (&MasterKey{}).loadIdentities()
420423
assert.Len(t, errs, 0)
421424
assert.Len(t, got, 1)
425+
assert.Len(t, unusedLocations, 6)
422426
})
423427

424428
t.Run(SopsAgeSshPrivateKeyFileEnv, func(t *testing.T) {
@@ -435,20 +439,20 @@ func TestMasterKey_loadIdentities(t *testing.T) {
435439
t.Setenv(SopsAgeSshPrivateKeyFileEnv, keyPath)
436440

437441
key := &MasterKey{}
438-
got, errs := key.loadIdentities()
442+
got, unusedLocations, errs := key.loadIdentities()
439443
assert.Len(t, errs, 0)
440444
assert.Len(t, got, 1)
445+
assert.Len(t, unusedLocations, 5)
441446
})
442447

443448
t.Run("no identity", func(t *testing.T) {
444449
tmpDir := t.TempDir()
445450
overwriteUserConfigDir(t, tmpDir)
446451

447-
got, errs := (&MasterKey{}).loadIdentities()
448-
assert.Len(t, errs, 1)
449-
assert.Error(t, errs[0])
450-
assert.ErrorContains(t, errs[0], "failed to open file")
452+
got, unusedLocations, errs := (&MasterKey{}).loadIdentities()
453+
assert.Len(t, errs, 0)
451454
assert.Nil(t, got)
455+
assert.Len(t, unusedLocations, 7)
452456
})
453457

454458
t.Run("multiple identities", func(t *testing.T) {
@@ -468,9 +472,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
468472
assert.NoError(t, os.WriteFile(keyPath2, []byte(mockOtherIdentity), 0o644))
469473
t.Setenv(SopsAgeKeyFileEnv, keyPath2)
470474

471-
got, errs := (&MasterKey{}).loadIdentities()
475+
got, unusedLocations, errs := (&MasterKey{}).loadIdentities()
472476
assert.Len(t, errs, 0)
473477
assert.Len(t, got, 2)
478+
assert.Len(t, unusedLocations, 5)
474479
})
475480

476481
t.Run("parsing error", func(t *testing.T) {
@@ -481,11 +486,12 @@ func TestMasterKey_loadIdentities(t *testing.T) {
481486
t.Setenv(SopsAgeKeyEnv, "invalid")
482487

483488
key := &MasterKey{}
484-
got, errs := key.loadIdentities()
489+
got, unusedLocations, errs := key.loadIdentities()
485490
assert.Len(t, errs, 1)
486491
assert.Error(t, errs[0])
487492
assert.ErrorContains(t, errs[0], fmt.Sprintf("failed to parse '%s' age identities", SopsAgeKeyEnv))
488493
assert.Nil(t, got)
494+
assert.Len(t, unusedLocations, 5)
489495
})
490496

491497
t.Run(SopsAgeKeyCmdEnv, func(t *testing.T) {
@@ -496,9 +502,10 @@ func TestMasterKey_loadIdentities(t *testing.T) {
496502
t.Setenv(SopsAgeKeyCmdEnv, "echo '"+mockIdentity+"'")
497503

498504
key := &MasterKey{}
499-
got, errs := key.loadIdentities()
505+
got, unusedLocations, errs := key.loadIdentities()
500506
assert.Len(t, errs, 0)
501507
assert.Len(t, got, 1)
508+
assert.Len(t, unusedLocations, 5)
502509
})
503510

504511
t.Run("cmd error", func(t *testing.T) {
@@ -509,11 +516,12 @@ func TestMasterKey_loadIdentities(t *testing.T) {
509516
t.Setenv(SopsAgeKeyCmdEnv, "meow")
510517

511518
key := &MasterKey{}
512-
got, errs := key.loadIdentities()
513-
assert.Len(t, errs, 2)
519+
got, unusedLocations, errs := key.loadIdentities()
520+
assert.Len(t, errs, 1)
514521
assert.Error(t, errs[0])
515522
assert.ErrorContains(t, errs[0], "failed to execute command meow")
516523
assert.Nil(t, got)
524+
assert.Len(t, unusedLocations, 6)
517525
})
518526
}
519527

0 commit comments

Comments
 (0)