Bump version to 0.6.2

Signed-off-by: Ulrich VACHON <ulrich.vachon@docker.com>

.travis.yml

@@ -3,20 +3,21 @@
   sudo: required
   language: go
   dist: trusty
+  osx_image: xcode10.1
   os:
     - linux
     - osx
   notifications:
     email: false
   go:
-    - 1.8
+    - 1.12.x
-  install: make deps
   addons:
     apt:
       packages:
         - libsecret-1-dev
         - pass
   before_script:
+    - make deps
     - "export DISPLAY=:99.0"
     - if [[ "$TRAVIS_OS_NAME" == "linux"  ]]; then sh ci/before_script_linux.sh; fi
     - make validate
@@ -28,7 +29,7 @@
   deploy:
     provider: releases
     api_key:
-      secure: "cGs5cao/MeVQVnum+Pr/Tpv+w83NsqGVS3wxvi3LYEf2ON4Kkmtd+Alwi0YFkGPJmSY0jZOct8NVK/M70qSnIU4l+AAq9+3KSMv23u4xrmy2sQog3AF+Ve3Rac+iYwZHOWwGs9I67CSuVv0vjJNVsDsTVefc25lHJImjRvXIS4p9xYzRPeUDCoqAo/QMVE+vFiMyxydsvt8fhd0gZCjPYWEpyHe9tjZ1tr1HsHZKFAjVb6AmF45d8rvadPoVUuLaOtr35wDC3XRKEvCZUefQpwLkrNj7j2L1rVGlY1xTE2APpLtvfd7R1Mx6kSfS1Gm3Pwcv3mugadXIhecL0lsdnU+BANjX3VUiv4ryzTPbsge966mv9ZQYwAzgCQTWRtMNJqsAnPZTeAkiOntd+HMQbPpxljOxv1sjDPY+EIZesyB3yQRJI8vMxqFcAjxeRyLcBqEnRFC2nd/Ln0KZ7ZFu16FcpNqRojdBayyypuXKqAiBNwtp4ti/65x8eHfBJuNjJtNZkRsJEYam4CYMRLxds9plKQfkaZ8045PKpyXO8fMpUhrfqSVID4IrYvD+io6XoXtdR4Lk6isZ2EgrjdrqgdG70S5lwKihL4iAi2F2ZCWhngFhkeNVOZunEWE6qZMk5wKODajR9sixGDApGPZQVojHwCNRGILZaHZ39JCIj3s="
+      secure: "$GITHUB_TOKEN"
     # upload file artifacts using a glob expression.
     # It requires both options `file_glob` and `file`:
     # https://github.com/travis-ci/dpl/blob/master/lib/dpl/provider/releases.rb#L47-L53

MAINTAINERS

@@ -11,18 +11,13 @@
 [Org]
 	[Org."Core maintainers"]
 		people = [
-			"aaronlehmann",
-			"calavera",
 			"coolljt0725",
 			"cpuguy83",
 			"crosbymichael",
 			"dnephin",
-			"dongluochen",
 			"duglin",
 			"estesp",
-			"icecrime",
 			"jhowardmsft",
-			"lk4d4",
 			"mavenugo",
 			"mhbauer",
 			"n4ss",
@@ -45,16 +40,6 @@
 
 	# ADD YOURSELF HERE IN ALPHABETICAL ORDER
 
-	[people.aaronlehmann]
-	Name = "Aaron Lehmann"
-	Email = "aaron.lehmann@docker.com"
-	GitHub = "aaronlehmann"
-
-	[people.calavera]
-	Name = "David Calavera"
-	Email = "david.calavera@gmail.com"
-	GitHub = "calavera"
-
 	[people.coolljt0725]
 	Name = "Lei Jitang"
 	Email = "leijitang@huawei.com"
@@ -75,11 +60,6 @@
 	Email = "dnephin@gmail.com"
 	GitHub = "dnephin"
 
-	[people.dongluochen]
-	Name = "Dongluo Chen"
-	Email = "dongluo.chen@docker.com"
-	GitHub = "dongluochen"
-
 	[people.duglin]
 	Name = "Doug Davis"
 	Email = "dug@us.ibm.com"
@@ -90,21 +70,11 @@
 	Email = "estesp@linux.vnet.ibm.com"
 	GitHub = "estesp"
 
-	[people.icecrime]
-	Name = "Arnaud Porterie"
-	Email = "arnaud@docker.com"
-	GitHub = "icecrime"
-
 	[people.jhowardmsft]
 	Name = "John Howard"
 	Email = "jhoward@microsoft.com"
 	GitHub = "jhowardmsft"
 
-	[people.lk4d4]
-	Name = "Alexander Morozov"
-	Email = "lk4d4@docker.com"
-	GitHub = "lk4d4"
-
 	[people.mavenugo]
 	Name = "Madhu Venugopal"
 	Email = "madhu@docker.com"

Makefile

@@ -6,7 +6,7 @@ VERSION := $(shell grep 'const Version' credentials/version.go | awk -F'"' '{ pr
 all: test
 
 deps:
-	go get -u github.com/golang/lint/golint
+	go get -u golang.org/x/lint/golint
 
 clean:
 	rm -rf bin

README.md

@@ -16,7 +16,7 @@ The programs in this repository are written with the Go programming language. Th
 $ go get github.com/docker/docker-credential-helpers
 ```
 
-2 - Use `make` to build the program you want. That will leave any executable in the `bin` directory inside the repository.
+2 - Use `make` to build the program you want. That will leave an executable in the `bin` directory inside the repository.
 
 ```
 $ cd $GOPATH/docker/docker-credentials-helpers

appveyor.yml

@@ -2,13 +2,13 @@ image: Visual Studio 2015
 environment:
       GOPATH: c:\gopath
 
+stack: go 1.8.7
+
 clone_folder: c:\gopath\src\github.com\docker\docker-credential-helpers
 clone_depth: 10
 
 before_build:
   - set PATH=%PATH%;C:\MinGW\bin;
-  - set PATH=%PATH%;C:\go18\bin;
-  - set GOROOT=C:\go18
 
 build_script:
   - mingw32-make vet_win wincred

credentials/version.go

@@ -1,4 +1,4 @@
 package credentials
 
 // Version holds a string describing the current version
-const Version = "0.6.0"
+const Version = "0.6.2"

osxkeychain/osxkeychain_darwin.go

@@ -1,8 +1,8 @@
 package osxkeychain
 
 /*
-#cgo CFLAGS: -x objective-c -mmacosx-version-min=10.10
+#cgo CFLAGS: -x objective-c -mmacosx-version-min=10.11
-#cgo LDFLAGS: -framework Security -framework Foundation -mmacosx-version-min=10.10
+#cgo LDFLAGS: -framework Security -framework Foundation -mmacosx-version-min=10.11
 
 #include "osxkeychain_darwin.h"
 #include <stdlib.h>
@@ -10,12 +10,11 @@ package osxkeychain
 import "C"
 import (
 	"errors"
-	"net/url"
 	"strconv"
-	"strings"
 	"unsafe"
 
 	"github.com/docker/docker-credential-helpers/credentials"
+	"github.com/docker/docker-credential-helpers/registryurl"
 )
 
 // errCredentialsNotFound is the specific error message returned by OS X
@@ -113,6 +112,10 @@ func (h Osxkeychain) List() (map[string]string, error) {
 	if errMsg != nil {
 		defer C.free(unsafe.Pointer(errMsg))
 		goMsg := C.GoString(errMsg)
+		if goMsg == errCredentialsNotFound {
+			return make(map[string]string), nil
+		}
+
 		return nil, errors.New(goMsg)
 	}
 
@@ -135,7 +138,7 @@ func (h Osxkeychain) List() (map[string]string, error) {
 }
 
 func splitServer(serverURL string) (*C.struct_Server, error) {
-	u, err := parseURL(serverURL)
+	u, err := registryurl.Parse(serverURL)
 	if err != nil {
 		return nil, err
 	}
@@ -145,7 +148,7 @@ func splitServer(serverURL string) (*C.struct_Server, error) {
 		proto = C.kSecProtocolTypeHTTP
 	}
 	var port int
-	p := getPort(u)
+	p := registryurl.GetPort(u)
 	if p != "" {
 		port, err = strconv.Atoi(p)
 		if err != nil {
@@ -155,7 +158,7 @@ func splitServer(serverURL string) (*C.struct_Server, error) {
 
 	return &C.struct_Server{
 		proto: C.SecProtocolType(proto),
-		host:  C.CString(getHostname(u)),
+		host:  C.CString(registryurl.GetHostname(u)),
 		port:  C.uint(port),
 		path:  C.CString(u.Path),
 	}, nil
@@ -165,32 +168,3 @@ func freeServer(s *C.struct_Server) {
 	C.free(unsafe.Pointer(s.host))
 	C.free(unsafe.Pointer(s.path))
 }
-
-// parseURL parses and validates a given serverURL to an url.URL, and
-// returns an error if validation failed. Querystring parameters are
-// omitted in the resulting URL, because they are not used in the helper.
-//
-// If serverURL does not have a valid scheme, `//` is used as scheme
-// before parsing. This prevents the hostname being used as path,
-// and the credentials being stored without host.
-func parseURL(serverURL string) (*url.URL, error) {
-	// Check if serverURL has a scheme, otherwise add `//` as scheme.
-	if !strings.Contains(serverURL, "://") && !strings.HasPrefix(serverURL, "//") {
-		serverURL = "//" + serverURL
-	}
-
-	u, err := url.Parse(serverURL)
-	if err != nil {
-		return nil, err
-	}
-
-	if u.Scheme != "" && u.Scheme != "https" && u.Scheme != "http" {
-		return nil, errors.New("unsupported scheme: " + u.Scheme)
-	}
-	if getHostname(u) == "" {
-		return nil, errors.New("no hostname in URL")
-	}
-
-	u.RawQuery = ""
-	return u, nil
-}

osxkeychain/osxkeychain_darwin_test.go

@@ -1,10 +1,10 @@
 package osxkeychain
 
 import (
-	"errors"
 	"fmt"
-	"github.com/docker/docker-credential-helpers/credentials"
 	"testing"
+
+	"github.com/docker/docker-credential-helpers/credentials"
 )
 
 func TestOSXKeychainHelper(t *testing.T) {
@@ -56,46 +56,6 @@ func TestOSXKeychainHelper(t *testing.T) {
 	}
 }
 
-// TestOSXKeychainHelperParseURL verifies that a // "scheme" is added to URLs,
-// and that invalid URLs produce an error.
-func TestOSXKeychainHelperParseURL(t *testing.T) {
-	tests := []struct {
-		url         string
-		expectedURL string
-		err         error
-	}{
-		{url: "foobar.docker.io", expectedURL: "//foobar.docker.io"},
-		{url: "foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"},
-		{url: "//foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"},
-		{url: "http://foobar.docker.io:2376", expectedURL: "http://foobar.docker.io:2376"},
-		{url: "https://foobar.docker.io:2376", expectedURL: "https://foobar.docker.io:2376"},
-		{url: "https://foobar.docker.io:2376/some/path", expectedURL: "https://foobar.docker.io:2376/some/path"},
-		{url: "https://foobar.docker.io:2376/some/other/path?foo=bar", expectedURL: "https://foobar.docker.io:2376/some/other/path"},
-		{url: "/foobar.docker.io", err: errors.New("no hostname in URL")},
-		{url: "ftp://foobar.docker.io:2376", err: errors.New("unsupported scheme: ftp")},
-	}
-
-	for _, te := range tests {
-		u, err := parseURL(te.url)
-
-		if te.err == nil && err != nil {
-			t.Errorf("Error: failed to parse URL %q: %s", te.url, err)
-			continue
-		}
-		if te.err != nil && err == nil {
-			t.Errorf("Error: expected error %q, got none when parsing URL %q", te.err, te.url)
-			continue
-		}
-		if te.err != nil && err.Error() != te.err.Error() {
-			t.Errorf("Error: expected error %q, got %q when parsing URL %q", te.err, err, te.url)
-			continue
-		}
-		if u != nil && u.String() != te.expectedURL {
-			t.Errorf("Error: expected URL: %q, but got %q for URL: %q", te.expectedURL, u.String(), te.url)
-		}
-	}
-}
-
 // TestOSXKeychainHelperRetrieveAliases verifies that secrets can be accessed
 // through variations on the URL
 func TestOSXKeychainHelperRetrieveAliases(t *testing.T) {

osxkeychain/url_go18.go

@@ -1,13 +0,0 @@
-//+build go1.8
-
-package osxkeychain
-
-import "net/url"
-
-func getHostname(u *url.URL) string {
-	return u.Hostname()
-}
-
-func getPort(u *url.URL) string {
-	return u.Port()
-}

pass/pass_linux.go

@@ -5,6 +5,7 @@
 package pass
 
 import (
+	"bytes"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -13,139 +14,99 @@ import (
 	"os/exec"
 	"path"
 	"strings"
+	"sync"
 
 	"github.com/docker/docker-credential-helpers/credentials"
 )
 
 const PASS_FOLDER = "docker-credential-helpers"
 
-var (
-	PassInitialized bool
-)
-
-func init() {
-	// In principle, we could just run `pass init`. However, pass has a bug
-	// where if gpg fails, it doesn't always exit 1. Additionally, pass
-	// uses gpg2, but gpg is the default, which may be confusing. So let's
-	// just explictily check that pass actually can store and retreive a
-	// password.
-	password := "pass is initialized"
-	name := path.Join(PASS_FOLDER, "docker-pass-initialized-check")
-
-	_, err := runPass(password, "insert", "-f", "-m", name)
-	if err != nil {
-		return
-	}
-
-	stored, err := runPass("", "show", name)
-	PassInitialized = err == nil && stored == password
-
-	if PassInitialized {
-		runPass("", "rm", "-rf", name)
-	}
-}
-
-func runPass(stdinContent string, args ...string) (string, error) {
-	cmd := exec.Command("pass", args...)
-
-	stdin, err := cmd.StdinPipe()
-	if err != nil {
-		return "", err
-	}
-	defer stdin.Close()
-
-	stderr, err := cmd.StderrPipe()
-	if err != nil {
-		return "", err
-	}
-	defer stderr.Close()
-
-	stdout, err := cmd.StdoutPipe()
-	if err != nil {
-		return "", err
-	}
-	defer stdout.Close()
-
-	err = cmd.Start()
-	if err != nil {
-		return "", err
-	}
-
-	_, err = stdin.Write([]byte(stdinContent))
-	if err != nil {
-		return "", err
-	}
-	stdin.Close()
-
-	errContent, err := ioutil.ReadAll(stderr)
-	if err != nil {
-		return "", fmt.Errorf("error reading stderr: %s", err)
-	}
-
-	result, err := ioutil.ReadAll(stdout)
-	if err != nil {
-		return "", fmt.Errorf("Error reading stdout: %s", err)
-	}
-
-	cmdErr := cmd.Wait()
-	if cmdErr != nil {
-		return "", fmt.Errorf("%s: %s", cmdErr, errContent)
-	}
-
-	return string(result), nil
-}
-
 // Pass handles secrets using Linux secret-service as a store.
 type Pass struct{}
 
-// Add adds new credentials to the keychain.
+// Ideally these would be stored as members of Pass, but since all of Pass's
-func (h Pass) Add(creds *credentials.Credentials) error {
+// methods have value receivers, not pointer receivers, and changing that is
-	if !PassInitialized {
+// backwards incompatible, we assume that all Pass instances share the same configuration
-		return errors.New("pass store is uninitialized")
+
+// initializationMutex is held while initializing so that only one 'pass'
+// round-tripping is done to check pass is functioning.
+var initializationMutex sync.Mutex
+var passInitialized bool
+
+// CheckInitialized checks whether the password helper can be used. It
+// internally caches and so may be safely called multiple times with no impact
+// on performance, though the first call may take longer.
+func (p Pass) CheckInitialized() bool {
+	return p.checkInitialized() == nil
+}
+
+func (p Pass) checkInitialized() error {
+	initializationMutex.Lock()
+	defer initializationMutex.Unlock()
+	if passInitialized {
+		return nil
+	}
+	// We just run a `pass ls`, if it fails then pass is not initialized.
+	_, err := p.runPassHelper("", "ls")
+	if err != nil {
+		return fmt.Errorf("pass not initialized: %v", err)
+	}
+	passInitialized = true
+	return nil
+}
+
+func (p Pass) runPass(stdinContent string, args ...string) (string, error) {
+	if err := p.checkInitialized(); err != nil {
+		return "", err
+	}
+	return p.runPassHelper(stdinContent, args...)
+}
+
+func (p Pass) runPassHelper(stdinContent string, args ...string) (string, error) {
+	var stdout, stderr bytes.Buffer
+	cmd := exec.Command("pass", args...)
+	cmd.Stdin = strings.NewReader(stdinContent)
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err := cmd.Run()
+	if err != nil {
+		return "", fmt.Errorf("%s: %s", err, stderr.String())
 	}
 
+	// trim newlines; pass v1.7.1+ includes a newline at the end of `show` output
+	return strings.TrimRight(stdout.String(), "\n\r"), nil
+}
+
+// Add adds new credentials to the keychain.
+func (h Pass) Add(creds *credentials.Credentials) error {
 	if creds == nil {
 		return errors.New("missing credentials")
 	}
 
 	encoded := base64.URLEncoding.EncodeToString([]byte(creds.ServerURL))
 
-	_, err := runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encoded, creds.Username))
+	_, err := h.runPass(creds.Secret, "insert", "-f", "-m", path.Join(PASS_FOLDER, encoded, creds.Username))
 	return err
 }
 
 // Delete removes credentials from the store.
 func (h Pass) Delete(serverURL string) error {
-	if !PassInitialized {
-		return errors.New("pass store is uninitialized")
-	}
-
 	if serverURL == "" {
 		return errors.New("missing server url")
 	}
 
 	encoded := base64.URLEncoding.EncodeToString([]byte(serverURL))
-	_, err := runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encoded))
+	_, err := h.runPass("", "rm", "-rf", path.Join(PASS_FOLDER, encoded))
 	return err
 }
 
 func getPassDir() string {
-	passDir := os.ExpandEnv("$HOME/.password-store")
+	passDir := "$HOME/.password-store"
-	for _, e := range os.Environ() {
+	if envDir := os.Getenv("PASSWORD_STORE_DIR"); envDir != "" {
-		parts := strings.SplitN(e, "=", 2)
+		passDir = envDir
-		if len(parts) < 2 {
-			continue
-		}
-
-		if parts[0] != "PASSWORD_STORE_DIR" {
-			continue
-		}
-
-		passDir = parts[1]
-		break
 	}
-
+	return os.ExpandEnv(passDir)
-	return passDir
 }
 
 // listPassDir lists all the contents of a directory in the password store.
@@ -168,10 +129,6 @@ func listPassDir(args ...string) ([]os.FileInfo, error) {
 
 // Get returns the username and secret to use for a given registry server URL.
 func (h Pass) Get(serverURL string) (string, string, error) {
-	if !PassInitialized {
-		return "", "", errors.New("pass store is uninitialized")
-	}
-
 	if serverURL == "" {
 		return "", "", errors.New("missing server url")
 	}
@@ -180,7 +137,7 @@ func (h Pass) Get(serverURL string) (string, string, error) {
 
 	if _, err := os.Stat(path.Join(getPassDir(), PASS_FOLDER, encoded)); err != nil {
 		if os.IsNotExist(err) {
-			return "", "", nil;
+			return "", "", nil
 		}
 
 		return "", "", err
@@ -196,16 +153,12 @@ func (h Pass) Get(serverURL string) (string, string, error) {
 	}
 
 	actual := strings.TrimSuffix(usernames[0].Name(), ".gpg")
-	secret, err := runPass("", "show", path.Join(PASS_FOLDER, encoded, actual))
+	secret, err := h.runPass("", "show", path.Join(PASS_FOLDER, encoded, actual))
 	return actual, secret, err
 }
 
 // List returns the stored URLs and corresponding usernames for a given credentials label
 func (h Pass) List() (map[string]string, error) {
-	if !PassInitialized {
-		return nil, errors.New("pass store is uninitialized")
-	}
-
 	servers, err := listPassDir()
 	if err != nil {
 		return nil, err

registryurl/parse.go

@@ -0,0 +1,37 @@
+package registryurl
+
+import (
+	"errors"
+	"net/url"
+	"strings"
+)
+
+// Parse parses and validates a given serverURL to an url.URL, and
+// returns an error if validation failed. Querystring parameters are
+// omitted in the resulting URL, because they are not used in the helper.
+//
+// If serverURL does not have a valid scheme, `//` is used as scheme
+// before parsing. This prevents the hostname being used as path,
+// and the credentials being stored without host.
+func Parse(registryURL string) (*url.URL, error) {
+	// Check if registryURL has a scheme, otherwise add `//` as scheme.
+	if !strings.Contains(registryURL, "://") && !strings.HasPrefix(registryURL, "//") {
+		registryURL = "//" + registryURL
+	}
+
+	u, err := url.Parse(registryURL)
+	if err != nil {
+		return nil, err
+	}
+
+	if u.Scheme != "" && u.Scheme != "https" && u.Scheme != "http" {
+		return nil, errors.New("unsupported scheme: " + u.Scheme)
+	}
+
+	if GetHostname(u) == "" {
+		return nil, errors.New("no hostname in URL")
+	}
+
+	u.RawQuery = ""
+	return u, nil
+}

registryurl/parse_test.go

@@ -0,0 +1,46 @@
+package registryurl
+
+import (
+	"errors"
+	"testing"
+)
+
+// TestHelperParseURL verifies that a // "scheme" is added to URLs,
+// and that invalid URLs produce an error.
+func TestHelperParseURL(t *testing.T) {
+	tests := []struct {
+		url         string
+		expectedURL string
+		err         error
+	}{
+		{url: "foobar.docker.io", expectedURL: "//foobar.docker.io"},
+		{url: "foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"},
+		{url: "//foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"},
+		{url: "http://foobar.docker.io:2376", expectedURL: "http://foobar.docker.io:2376"},
+		{url: "https://foobar.docker.io:2376", expectedURL: "https://foobar.docker.io:2376"},
+		{url: "https://foobar.docker.io:2376/some/path", expectedURL: "https://foobar.docker.io:2376/some/path"},
+		{url: "https://foobar.docker.io:2376/some/other/path?foo=bar", expectedURL: "https://foobar.docker.io:2376/some/other/path"},
+		{url: "/foobar.docker.io", err: errors.New("no hostname in URL")},
+		{url: "ftp://foobar.docker.io:2376", err: errors.New("unsupported scheme: ftp")},
+	}
+
+	for _, te := range tests {
+		u, err := Parse(te.url)
+
+		if te.err == nil && err != nil {
+			t.Errorf("Error: failed to parse URL %q: %s", te.url, err)
+			continue
+		}
+		if te.err != nil && err == nil {
+			t.Errorf("Error: expected error %q, got none when parsing URL %q", te.err, te.url)
+			continue
+		}
+		if te.err != nil && err.Error() != te.err.Error() {
+			t.Errorf("Error: expected error %q, got %q when parsing URL %q", te.err, err, te.url)
+			continue
+		}
+		if u != nil && u.String() != te.expectedURL {
+			t.Errorf("Error: expected URL: %q, but got %q for URL: %q", te.expectedURL, u.String(), te.url)
+		}
+	}
+}

registryurl/url_go18.go

@@ -0,0 +1,15 @@
+//+build go1.8
+
+package registryurl
+
+import (
+	url "net/url"
+)
+
+func GetHostname(u *url.URL) string {
+	return u.Hostname()
+}
+
+func GetPort(u *url.URL) string {
+	return u.Port()
+}

registryurl/url_non_go18.go

@@ -1,17 +1,17 @@
 //+build !go1.8
 
-package osxkeychain
+package registryurl
 
 import (
-	"net/url"
+	url "net/url"
 	"strings"
 )
 
-func getHostname(u *url.URL) string {
+func GetHostname(u *url.URL) string {
 	return stripPort(u.Host)
 }
 
-func getPort(u *url.URL) string {
+func GetPort(u *url.URL) string {
 	return portOnly(u.Host)
 }
 

secretservice/secretservice_linux.go

@@ -93,7 +93,7 @@ func (h Secretservice) List() (map[string]string, error) {
 	var listLenC C.uint
 	err := C.list(credsLabelC, &pathsC, &acctsC, &listLenC)
 	if err != nil {
-		defer C.free(unsafe.Pointer(err))
+		defer C.g_error_free(err)
 		return nil, errors.New("Error from list function in secretservice_linux.c likely due to error in secretservice library")
 	}
 	defer C.freeListData(&pathsC, listLenC)

wincred/wincred_windows.go

@@ -2,10 +2,12 @@ package wincred
 
 import (
 	"bytes"
+	"net/url"
 	"strings"
 
 	winc "github.com/danieljoos/wincred"
 	"github.com/docker/docker-credential-helpers/credentials"
+	"github.com/docker/docker-credential-helpers/registryurl"
 )
 
 // Wincred handles secrets using the Windows credential service.
@@ -13,11 +15,12 @@ type Wincred struct{}
 
 // Add adds new credentials to the windows credentials manager.
 func (h Wincred) Add(creds *credentials.Credentials) error {
+	credsLabels := []byte(credentials.CredsLabel)
 	g := winc.NewGenericCredential(creds.ServerURL)
 	g.UserName = creds.Username
 	g.CredentialBlob = []byte(creds.Secret)
 	g.Persist = winc.PersistLocalMachine
-	g.Attributes = []winc.CredentialAttribute{{"label", []byte(credentials.CredsLabel)}}
+	g.Attributes = []winc.CredentialAttribute{{Keyword: "label", Value: credsLabels}}
 
 	return g.Write()
 }
@@ -36,10 +39,18 @@ func (h Wincred) Delete(serverURL string) error {
 
 // Get retrieves credentials from the windows credentials manager.
 func (h Wincred) Get(serverURL string) (string, string, error) {
-	g, _ := winc.GetGenericCredential(serverURL)
+	target, err := getTarget(serverURL)
+	if err != nil {
+		return "", "", err
+	} else if target == "" {
+		return "", "", credentials.NewErrCredentialsNotFound()
+	}
+
+	g, _ := winc.GetGenericCredential(target)
 	if g == nil {
 		return "", "", credentials.NewErrCredentialsNotFound()
 	}
+
 	for _, attr := range g.Attributes {
 		if strings.Compare(attr.Keyword, "label") == 0 &&
 			bytes.Compare(attr.Value, []byte(credentials.CredsLabel)) == 0 {
@@ -50,6 +61,71 @@ func (h Wincred) Get(serverURL string) (string, string, error) {
 	return "", "", credentials.NewErrCredentialsNotFound()
 }
 
+func getTarget(serverURL string) (string, error) {
+	s, err := registryurl.Parse(serverURL)
+	if err != nil {
+		return serverURL, nil
+	}
+
+	creds, err := winc.List()
+	if err != nil {
+		return "", err
+	}
+
+	var targets []string
+	for i := range creds {
+		attrs := creds[i].Attributes
+		for _, attr := range attrs {
+			if attr.Keyword == "label" && bytes.Equal(attr.Value, []byte(credentials.CredsLabel)) {
+				targets = append(targets, creds[i].TargetName)
+			}
+		}
+	}
+
+	if target, found := findMatch(s, targets, exactMatch); found {
+		return target, nil
+	}
+
+	if target, found := findMatch(s, targets, approximateMatch); found {
+		return target, nil
+	}
+
+	return "", nil
+}
+
+func findMatch(serverUrl *url.URL, targets []string, matches func(url.URL, url.URL) bool) (string, bool) {
+	for _, target := range targets {
+		tURL, err := registryurl.Parse(target)
+		if err != nil {
+			continue
+		}
+		if matches(*serverUrl, *tURL) {
+			return target, true
+		}
+	}
+	return "", false
+}
+
+func exactMatch(serverURL, target url.URL) bool {
+	return serverURL.String() == target.String()
+}
+
+func approximateMatch(serverURL, target url.URL) bool {
+	//if scheme is missing assume it is the same as target
+	if serverURL.Scheme == "" {
+		serverURL.Scheme = target.Scheme
+	}
+	//if port is missing assume it is the same as target
+	if serverURL.Port() == "" && target.Port() != "" {
+		serverURL.Host = serverURL.Host + ":" + target.Port()
+	}
+	//if path is missing assume it is the same as target
+	if serverURL.Path == "" {
+		serverURL.Path = target.Path
+	}
+	return serverURL.String() == target.String()
+}
+
 // List returns the stored URLs and corresponding usernames for a given credentials label.
 func (h Wincred) List() (map[string]string, error) {
 	creds, err := winc.List()

wincred/wincred_windows_test.go

@@ -1,6 +1,7 @@
 package wincred
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -86,6 +87,154 @@ func TestWinCredHelper(t *testing.T) {
 	}
 }
 
+// TestWinCredHelperRetrieveAliases verifies that secrets can be accessed
+// through variations on the URL
+func TestWinCredHelperRetrieveAliases(t *testing.T) {
+	tests := []struct {
+		storeURL string
+		readURL  string
+	}{
+		// stored with port, retrieved without
+		{"https://foobar.docker.io:2376", "https://foobar.docker.io"},
+
+		// stored as https, retrieved without scheme
+		{"https://foobar.docker.io", "foobar.docker.io"},
+
+		// stored with path, retrieved without
+		{"https://foobar.docker.io/one/two", "https://foobar.docker.io"},
+	}
+
+	helper := Wincred{}
+	defer func() {
+		for _, te := range tests {
+			helper.Delete(te.storeURL)
+		}
+	}()
+
+	// Clean store before testing.
+	for _, te := range tests {
+		helper.Delete(te.storeURL)
+	}
+
+	for _, te := range tests {
+		c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"}
+		if err := helper.Add(c); err != nil {
+			t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err)
+			continue
+		}
+		if _, _, err := helper.Get(te.readURL); err != nil {
+			t.Errorf("Error: failed to read secret for URL %q using %q", te.storeURL, te.readURL)
+		}
+		helper.Delete(te.storeURL)
+	}
+}
+
+// TestWinCredHelperRetrieveStrict verifies that only matching secrets are
+// returned.
+func TestWinCredHelperRetrieveStrict(t *testing.T) {
+	tests := []struct {
+		storeURL string
+		readURL  string
+	}{
+		// stored as https, retrieved using http
+		{"https://foobar.docker.io:2376", "http://foobar.docker.io:2376"},
+
+		// stored as http, retrieved using https
+		{"http://foobar.docker.io:2376", "https://foobar.docker.io:2376"},
+
+		// same: stored as http, retrieved without a scheme specified (hence, using the default https://)
+		{"http://foobar.docker.io", "foobar.docker.io:5678"},
+
+		// non-matching ports
+		{"https://foobar.docker.io:1234", "https://foobar.docker.io:5678"},
+
+		// non-matching ports TODO is this desired behavior? The other way round does work
+		//{"https://foobar.docker.io", "https://foobar.docker.io:5678"},
+
+		// non-matching paths
+		{"https://foobar.docker.io:1234/one/two", "https://foobar.docker.io:1234/five/six"},
+	}
+
+	helper := Wincred{}
+	defer func() {
+		for _, te := range tests {
+			helper.Delete(te.storeURL)
+		}
+	}()
+
+	// Clean store before testing.
+	for _, te := range tests {
+		helper.Delete(te.storeURL)
+	}
+
+	for _, te := range tests {
+		c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"}
+		if err := helper.Add(c); err != nil {
+			t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err)
+			continue
+		}
+		if _, _, err := helper.Get(te.readURL); err == nil {
+			t.Errorf("Error: managed to read secret for URL %q using %q, but should not be able to", te.storeURL, te.readURL)
+		}
+		helper.Delete(te.storeURL)
+	}
+}
+
+// TestWinCredHelperStoreRetrieve verifies that secrets stored in the
+// the keychain can be read back using the URL that was used to store them.
+func TestWinCredHelperStoreRetrieve(t *testing.T) {
+	tests := []struct {
+		url string
+	}{
+		{url: "foobar.docker.io"},
+		{url: "foobar.docker.io:2376"},
+		{url: "//foobar.docker.io:2376"},
+		{url: "https://foobar.docker.io:2376"},
+		{url: "http://foobar.docker.io:2376"},
+		{url: "https://foobar.docker.io:2376/some/path"},
+		{url: "https://foobar.docker.io:2376/some/other/path"},
+		{url: "https://foobar.docker.io:2376/some/other/path?foo=bar"},
+	}
+
+	helper := Wincred{}
+	defer func() {
+		for _, te := range tests {
+			helper.Delete(te.url)
+		}
+	}()
+
+	// Clean store before testing.
+	for _, te := range tests {
+		helper.Delete(te.url)
+	}
+
+	// Note that we don't delete between individual tests here, to verify that
+	// subsequent stores/overwrites don't affect storing / retrieving secrets.
+	for i, te := range tests {
+		c := &credentials.Credentials{
+			ServerURL: te.url,
+			Username:  fmt.Sprintf("user-%d", i),
+			Secret:    fmt.Sprintf("secret-%d", i),
+		}
+
+		if err := helper.Add(c); err != nil {
+			t.Errorf("Error: failed to store secret for URL: %s: %s", te.url, err)
+			continue
+		}
+		user, secret, err := helper.Get(te.url)
+		if err != nil {
+			t.Errorf("Error: failed to read secret for URL %q: %s", te.url, err)
+			continue
+		}
+		if user != c.Username {
+			t.Errorf("Error: expected username %s, got username %s for URL: %s", c.Username, user, te.url)
+		}
+		if secret != c.Secret {
+			t.Errorf("Error: expected secret %s, got secret %s for URL: %s", c.Secret, secret, te.url)
+		}
+	}
+}
+
 func TestMissingCredentials(t *testing.T) {
 	helper := Wincred{}
 	_, _, err := helper.Get("https://adsfasdf.wrewerwer.com/asdfsdddd")