Skip to content

Commit 5da09fd

Browse files
euankvdemeester
authored andcommitted
pass: only init on run, and do so lazily
This also fixes the following issues: 1. Safe for concurrent initialization still (it was before in 'init', but the alternative to this PR is not) 2. Uses the same password directory during init as it does during runtime (the change to getPassDir in initialization logic. 3. Prints significantly better errors if initialization fails 4. Has slightly cleaner abstractions by hiding the initialization check in 'runPass' The 4th item there does mean there are a few cases where more work is done before erroring, but that amount of work is trivial and my manual audit didn't reveal anything that seemed worrying. Fixes #96, alternative to #106 Signed-off-by: Euan Kemp <[email protected]>
1 parent 26deb29 commit 5da09fd

File tree

1 file changed

+39
-36
lines changed

1 file changed

+39
-36
lines changed

pass/pass_linux.go

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package pass
66

77
import (
8-
"bytes"
98
"encoding/base64"
109
"errors"
1110
"fmt"
@@ -14,40 +13,63 @@ import (
1413
"os/exec"
1514
"path"
1615
"strings"
16+
"sync"
1717

1818
"github.com/docker/docker-credential-helpers/credentials"
1919
)
2020

2121
const PASS_FOLDER = "docker-credential-helpers"
2222

23-
var (
24-
PassInitialized bool
25-
)
23+
// Pass handles secrets using Linux secret-service as a store.
24+
type Pass struct{}
2625

27-
func init() {
26+
// Ideally these would be stored as members of Pass, but since all of Pass's
27+
// methods have value receivers, not pointer receivers, and changing that is
28+
// backwards incompatible, we assume that all Pass instances share the same configuration
29+
30+
// initializationMutex is held while initializing so that only one 'pass'
31+
// round-tripping is done to check pass is functioning.
32+
var initializationMutex sync.Mutex
33+
var passInitialized bool
34+
35+
func (p Pass) checkInitialized() error {
36+
initializationMutex.Lock()
37+
defer initializationMutex.Unlock()
38+
if passInitialized {
39+
return nil
40+
}
2841
// In principle, we could just run `pass init`. However, pass has a bug
2942
// where if gpg fails, it doesn't always exit 1. Additionally, pass
3043
// uses gpg2, but gpg is the default, which may be confusing. So let's
3144
// just explictily check that pass actually can store and retreive a
3245
// password.
3346
password := "pass is initialized"
34-
name := path.Join(PASS_FOLDER, "docker-pass-initialized-check")
47+
name := path.Join(getPassDir(), "docker-pass-initialized-check")
3548

36-
_, err := runPass(password, "insert", "-f", "-m", name)
49+
_, err := p.runPassHelper(password, "insert", "-f", "-m", name)
3750
if err != nil {
38-
return
51+
return fmt.Errorf("error initializing pass: %v", err)
3952
}
4053

41-
stored, err := runPass("", "show", name)
42-
PassInitialized = err == nil && stored == password
54+
stored, err := p.runPassHelper("", "show", name)
55+
if err != nil {
56+
return fmt.Errorf("error fetching password during initialization: %v", err)
57+
}
58+
if stored != password {
59+
return fmt.Errorf("error round-tripping password during initialization: %q != %q", password, stored)
60+
}
61+
passInitialized = true
62+
return nil
63+
}
4364

44-
if PassInitialized {
45-
runPass("", "rm", "-rf", name)
65+
func (p Pass) runPass(stdinContent string, args ...string) (string, error) {
66+
if err := p.checkInitialized(); err != nil {
67+
return "", err
4668
}
69+
return p.runPassHelper(stdinContent, args...)
4770
}
4871

49-
func runPass(stdin string, args ...string) (string, error) {
50-
var stdout, stderr bytes.Buffer
72+
func (p Pass) runPassHelper(stdinContent string, args ...string) (string, error) {
5173
cmd := exec.Command("pass", args...)
5274
cmd.Stdin = strings.NewReader(stdin)
5375
cmd.Stdout = &stdout
@@ -61,37 +83,26 @@ func runPass(stdin string, args ...string) (string, error) {
6183
return stdout.String(), nil
6284
}
6385

64-
// Pass handles secrets using Linux secret-service as a store.
65-
type Pass struct{}
66-
6786
// Add adds new credentials to the keychain.
6887
func (h Pass) Add(creds *credentials.Credentials) error {
69-
if !PassInitialized {
70-
return errors.New("pass store is uninitialized")
71-
}
72-
7388
if creds == nil {
7489
return errors.New("missing credentials")
7590
}
7691

7792
encoded := base64.URLEncoding.EncodeToString([]byte(creds.ServerURL))
7893

79-
_, err := runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encoded, creds.Username))
94+
_, err := h.runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encoded, creds.Username))
8095
return err
8196
}
8297

8398
// Delete removes credentials from the store.
8499
func (h Pass) Delete(serverURL string) error {
85-
if !PassInitialized {
86-
return errors.New("pass store is uninitialized")
87-
}
88-
89100
if serverURL == "" {
90101
return errors.New("missing server url")
91102
}
92103

93104
encoded := base64.URLEncoding.EncodeToString([]byte(serverURL))
94-
_, err := runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encoded))
105+
_, err := h.runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encoded))
95106
return err
96107
}
97108

@@ -123,10 +134,6 @@ func listPassDir(args ...string) ([]os.FileInfo, error) {
123134

124135
// Get returns the username and secret to use for a given registry server URL.
125136
func (h Pass) Get(serverURL string) (string, string, error) {
126-
if !PassInitialized {
127-
return "", "", errors.New("pass store is uninitialized")
128-
}
129-
130137
if serverURL == "" {
131138
return "", "", errors.New("missing server url")
132139
}
@@ -151,16 +158,12 @@ func (h Pass) Get(serverURL string) (string, string, error) {
151158
}
152159

153160
actual := strings.TrimSuffix(usernames[0].Name(), ".gpg")
154-
secret, err := runPass("", "show", path.Join(PASS_FOLDER, encoded, actual))
161+
secret, err := h.runPass("", "show", path.Join(PASS_FOLDER, encoded, actual))
155162
return actual, secret, err
156163
}
157164

158165
// List returns the stored URLs and corresponding usernames for a given credentials label
159166
func (h Pass) List() (map[string]string, error) {
160-
if !PassInitialized {
161-
return nil, errors.New("pass store is uninitialized")
162-
}
163-
164167
servers, err := listPassDir()
165168
if err != nil {
166169
return nil, err

0 commit comments

Comments
 (0)