Merge pull request #76 from thaJeztah/bump-0.5.2

Bump version 0.5.2

.travis.yml

@@ -9,7 +9,7 @@
   notifications:
     email: false
   go:
-    - 1.6
+    - 1.8
   install: make deps
   addons:
     apt:

CHANGELOG.md

@@ -4,6 +4,30 @@ This changelog tracks the releases of docker-credential-helpers.
 This project includes different binaries per platform.
 The platform released is identified after the tag name.
 
+## v0.5.2 (Mac OS X, Windows, Linux)
+
+- Add a `version` command to output the version
+- Fix storing URLs without scheme, and use `https://` by default
+
+## v0.5.1 (Go client, Mac OS X, Windows, Linux)
+
+- Redirect credential helpers' standard error to the caller's
+- Prevent invalid credentials and credentials queries
+
+## v0.5.0 (Mac OS X)
+
+- Add a label for Docker credentials and filter credentials lookup to filter keychain lookups
+
+## v0.4.2 (Mac OS X, Windows)
+
+- Fix osxkeychain list
+- macOS binary is now signed on release
+- Generate a `.exe` instead
+
+## v0.4.1 (Mac OS X)
+
+- Fixes to support older version of OSX (10.10, 10.11)
+
 ## v0.4.0 (Go client, Mac OS X, Windows, Linux)
 
 - Full implementation for OSX ready
@@ -11,7 +35,6 @@ The platform released is identified after the tag name.
 - Implement client.List, change list API
 - mac: delete credentials before adding them to avoid already exist error (fixes #37)
 
-
 ## v0.3.0 (Go client)
 
 - Add Go client library to talk with the native programs.

Makefile

@@ -1,7 +1,7 @@
 .PHONY: all deps osxkeychain secretservice test validate wincred
 
 TRAVIS_OS_NAME ?= linux
-VERSION = 0.5.1
+VERSION := $(shell grep 'const Version' credentials/version.go | awk -F'"' '{ print $$2 }')
 
 all: test
 
@@ -16,13 +16,14 @@ osxkeychain:
 	mkdir bin
 	go build -ldflags -s -o bin/docker-credential-osxkeychain osxkeychain/cmd/main_darwin.go
 
-codesign: osxkeychain
+osxcodesign: osxkeychain
 	$(eval SIGNINGHASH = $(shell security find-identity -v -p codesigning | grep "Developer ID Application: Docker Inc" | cut -d ' ' -f 4))
 	xcrun -log codesign -s $(SIGNINGHASH) --force --verbose bin/docker-credential-osxkeychain
 	xcrun codesign --verify --deep --strict --verbose=2 --display bin/docker-credential-osxkeychain
 
-osxrelease: clean test codesign
+osxrelease: clean vet_osx lint fmt test osxcodesign
 	mkdir -p release
+	@echo "\nPackaging version ${VERSION}\n"
 	cd bin && tar cvfz ../release/docker-credential-osxkeychain-v$(VERSION)-amd64.tar.gz docker-credential-osxkeychain
 
 secretservice:
@@ -40,14 +41,21 @@ test:
 vet: vet_$(TRAVIS_OS_NAME)
 	go vet ./credentials
 
+vet_win:
+	go vet ./wincred
+
 vet_osx:
 	go vet ./osxkeychain
 
 vet_linux:
 	go vet ./secretservice
 
-validate: vet
+lint:
 	for p in `go list ./... | grep -v /vendor/`; do \
 		golint $$p ; \
 	done
+
+fmt:
 	gofmt -s -l `ls **/*.go | grep -v vendor`
+
+validate: vet lint fmt

appveyor.yml

@@ -1,67 +1,24 @@
-version: "{build}"
+image: Visual Studio 2015
-
-# Source Config
-clone_folder: c:\gopath\src\github.com\docker\docker-credential-helpers
-
-# Build host
-
 environment:
-    global:
       GOPATH: c:\gopath
-      CGO_ENABLED: 1
-      GOVERSION: 1.6
-    matrix:
-      - platform: x86
-        GOARCH: 386
-        MSYS2_BITS: 32
-      - platform: x64
-        GOARCH: amd64
-        MSYS2_BITS: 64
 
-init:
+clone_folder: c:\gopath\src\github.com\docker\docker-credential-helpers
-  - git config --global core.autocrlf input
+clone_depth: 10
 
-# Build
+before_build:
+  - set PATH=%PATH%;C:\MinGW\bin;
+  - set PATH=%PATH%;C:\go18\bin;
+  - set GOROOT=C:\go18
 
-install:
+build_script:
-  # Install Go 1.6.
+  - mingw32-make vet_win wincred
-  - rmdir c:\go /s /q
-  - appveyor DownloadFile https://storage.googleapis.com/golang/go%GOVERSION%.windows-%GOARCH%.msi
-  - msiexec /i go%GOVERSION%.windows-%GOARCH%.msi /q
-  - set Path=c:\msys64\mingw%MSYS2_BITS%\bin;c:\go\bin;%Path%
-  - go version
-  - go env
-
-build: false
 
 test_script:
-  - go vet ./wincred
+  - mingw32-make test
-  - go test -v github.com/docker/docker-credential-helpers/wincred
 
-# Equivalent to `before_deploy` phase
+deploy: off
-after_test:
-    # build binary
-  - mkdir bin
-  - go build -o bin/docker-credential-wincred wincred/cmd/main_windows.go
-    # build zipfile, will look like docker-credential-wincred-v0.1.0-amd64.zip in the root directory
-  - cd bin && 7z a ../docker-credential-wincred-%APPVEYOR_REPO_TAG_NAME%-%GOARCH%.zip docker-credential-wincred
 
-# IMPORTANT All the artifacts need to be listed here, or they won't be uploaded to GitHub
 artifacts:
-  - path: docker-credential-wincred-$(APPVEYOR_REPO_TAG_NAME)-$(GOARCH).zip
+  - path: bin/docker-credential-wincred.exe
-    name: docker-credential-wincred-$(APPVEYOR_REPO_TAG_NAME)-$(GOARCH).zip
 
-deploy:
+configuration: Release
-  # All the zipped artifacts will be deployed
-  description: "Visit the [Changelog](https://github.com/docker/docker-credential-helpers/blob/master/CHANGELOG.md) for a detailed description of what's new in this release."
-  artifact: /.*\.zip/
-  auth_token:
-    secure: ixWmTXZs8aV5+9s6vPXziIcdMMLd+lBVINJ0K/Sy++2wllpRxUec4/TPVKUGLqvL
-  provider: GitHub
-  # deploy when a new tag is pushed
-  on:
-    appveyor_repo_tag: true
-
-branches:
-  only:
-    - master

credentials/credentials.go

@@ -33,11 +33,12 @@ func (c *Credentials) isValid() (bool, error) {
 	return true, nil
 }
 
-// Docker credentials should be labeled as such in credentials stores that allow labelling.
+// CredsLabel holds the way Docker credentials should be labeled as such in credentials stores that allow labelling.
 // That label allows to filter out non-Docker credentials too at lookup/search in macOS keychain,
 // Windows credentials manager and Linux libsecret. Default value is "Docker Credentials"
 var CredsLabel = "Docker Credentials"
 
+// SetCredsLabel is a simple setter for CredsLabel
 func SetCredsLabel(label string) {
 	CredsLabel = label
 }
@@ -50,7 +51,7 @@ func SetCredsLabel(label string) {
 func Serve(helper Helper) {
 	var err error
 	if len(os.Args) != 2 {
-		err = fmt.Errorf("Usage: %s <store|get|erase|list>", os.Args[0])
+		err = fmt.Errorf("Usage: %s <store|get|erase|list|version>", os.Args[0])
 	}
 
 	if err == nil {
@@ -74,6 +75,8 @@ func HandleCommand(helper Helper, key string, in io.Reader, out io.Writer) error
 		return Erase(helper, in)
 	case "list":
 		return List(helper, out)
+	case "version":
+		return PrintVersion(out)
 	}
 	return fmt.Errorf("Unknown credential action `%s`", key)
 }
@@ -175,3 +178,9 @@ func List(helper Helper, writer io.Writer) error {
 	}
 	return json.NewEncoder(writer).Encode(accts)
 }
+
+//PrintVersion outputs the current version.
+func PrintVersion(writer io.Writer) error {
+	fmt.Fprintln(writer, Version)
+	return nil
+}

credentials/error.go

@@ -43,7 +43,6 @@ func IsErrCredentialsNotFoundMessage(err string) bool {
 	return err == errCredentialsNotFoundMessage
 }
 
-
 // errCredentialsMissingServerURL represents an error raised
 // when the credentials object has no server URL or when no
 // server URL is provided to a credentials operation requiring
@@ -64,7 +63,6 @@ func (errCredentialsMissingUsername) Error() string {
 	return errCredentialsMissingUsernameMessage
 }
 
-
 // NewErrCredentialsMissingServerURL creates a new error for
 // errCredentialsMissingServerURL.
 func NewErrCredentialsMissingServerURL() error {
@@ -77,7 +75,6 @@ func NewErrCredentialsMissingUsername() error {
 	return errCredentialsMissingUsername{}
 }
 
-
 // IsCredentialsMissingServerURL returns true if the error
 // was an errCredentialsMissingServerURL.
 func IsCredentialsMissingServerURL(err error) bool {

credentials/version.go

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

osxkeychain/osxkeychain_darwin.go

@@ -135,30 +135,27 @@ func (h Osxkeychain) List() (map[string]string, error) {
 }
 
 func splitServer(serverURL string) (*C.struct_Server, error) {
-	u, err := url.Parse(serverURL)
+	u, err := parseURL(serverURL)
 	if err != nil {
 		return nil, err
 	}
 
-	hostAndPort := strings.Split(u.Host, ":")
-	host := hostAndPort[0]
-	var port int
-	if len(hostAndPort) == 2 {
-		p, err := strconv.Atoi(hostAndPort[1])
-		if err != nil {
-			return nil, err
-		}
-		port = p
-	}
-
 	proto := C.kSecProtocolTypeHTTPS
-	if u.Scheme != "https" {
+	if u.Scheme == "http" {
 		proto = C.kSecProtocolTypeHTTP
 	}
+	var port int
+	p := getPort(u)
+	if p != "" {
+		port, err = strconv.Atoi(p)
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	return &C.struct_Server{
 		proto: C.SecProtocolType(proto),
-		host:  C.CString(host),
+		host:  C.CString(getHostname(u)),
 		port:  C.uint(port),
 		path:  C.CString(u.Path),
 	}, nil
@@ -168,3 +165,32 @@ 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,6 +1,8 @@
 package osxkeychain
 
 import (
+	"errors"
+	"fmt"
 	"github.com/docker/docker-credential-helpers/credentials"
 	"testing"
 )
@@ -54,6 +56,194 @@ 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) {
+	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:2376", "foobar.docker.io"},
+
+		// stored with path, retrieved without
+		{"https://foobar.docker.io:1234/one/two", "https://foobar.docker.io:1234"},
+	}
+
+	helper := Osxkeychain{}
+	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)
+	}
+}
+
+// TestOSXKeychainHelperRetrieveStrict verifies that only matching secrets are
+// returned.
+func TestOSXKeychainHelperRetrieveStrict(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 := Osxkeychain{}
+	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)
+	}
+}
+
+// TestOSXKeychainHelperStoreRetrieve verifies that secrets stored in the
+// the keychain can be read back using the URL that was used to store them.
+func TestOSXKeychainHelperStoreRetrieve(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 := Osxkeychain{}
+	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 := Osxkeychain{}
 	_, _, err := helper.Get("https://adsfasdf.wrewerwer.com/asdfsdddd")

osxkeychain/url_go18.go

@@ -0,0 +1,13 @@
+//+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()
+}

osxkeychain/url_non_go18.go

@@ -0,0 +1,41 @@
+//+build !go1.8
+
+package osxkeychain
+
+import (
+	"net/url"
+	"strings"
+)
+
+func getHostname(u *url.URL) string {
+	return stripPort(u.Host)
+}
+
+func getPort(u *url.URL) string {
+	return portOnly(u.Host)
+}
+
+func stripPort(hostport string) string {
+	colon := strings.IndexByte(hostport, ':')
+	if colon == -1 {
+		return hostport
+	}
+	if i := strings.IndexByte(hostport, ']'); i != -1 {
+		return strings.TrimPrefix(hostport[:i], "[")
+	}
+	return hostport[:colon]
+}
+
+func portOnly(hostport string) string {
+	colon := strings.IndexByte(hostport, ':')
+	if colon == -1 {
+		return ""
+	}
+	if i := strings.Index(hostport, "]:"); i != -1 {
+		return hostport[i+len("]:"):]
+	}
+	if strings.Contains(hostport, "]") {
+		return ""
+	}
+	return hostport[colon+len(":"):]
+}

secretservice/secretservice_linux.go

@@ -105,8 +105,11 @@ func (h Secretservice) List() (map[string]string, error) {
 	if listLen == 0 {
 		return resp, nil
 	}
-	pathTmp := (*[1 << 30]*C.char)(unsafe.Pointer(pathsC))[:listLen:listLen]
+	// The maximum capacity of the following two slices is limited to (2^29)-1 to remain compatible
-	acctTmp := (*[1 << 30]*C.char)(unsafe.Pointer(acctsC))[:listLen:listLen]
+	// with 32-bit platforms. The size of a `*C.char` (a pointer) is 4 Byte on a 32-bit system
+	// and (2^29)*4 == math.MaxInt32 + 1. -- See issue golang/go#13656
+	pathTmp := (*[(1 << 29) - 1]*C.char)(unsafe.Pointer(pathsC))[:listLen:listLen]
+	acctTmp := (*[(1 << 29) - 1]*C.char)(unsafe.Pointer(acctsC))[:listLen:listLen]
 	for i := 0; i < listLen; i++ {
 		resp[C.GoString(pathTmp[i])] = C.GoString(acctTmp[i])
 	}

wincred/wincred_windows_test.go

@@ -1,8 +1,8 @@
 package wincred
 
 import (
-	"testing"
 	"strings"
+	"testing"
 
 	"github.com/docker/docker-credential-helpers/credentials"
 )
@@ -63,7 +63,7 @@ func TestWinCredHelper(t *testing.T) {
 	}
 
 	auths, err := helper.List()
-	if err != nil || len(auths) - len(oldauths) != 1 {
+	if err != nil || len(auths)-len(oldauths) != 1 {
 		t.Fatal(err)
 	}