Merge pull request #86 from fiws/limit-set-size

Limit set size

keyring.go

@@ -3,13 +3,18 @@ package keyring
 import "errors"
 
 // provider set in the init function by the relevant os file e.g.:
-// keyring_linux.go
+// keyring_unix.go
 var provider Keyring = fallbackServiceProvider{}
 
 var (
 	// ErrNotFound is the expected error if the secret isn't found in the
 	// keyring.
 	ErrNotFound = errors.New("secret not found in keyring")
+	// ErrSetDataTooBig is returned if `Set` was called with too much data.
+	// On MacOS: The combination of service, username & password should not exceed ~3000 bytes
+	// On Windows: The service is limited to 32KiB while the password is limited to 2560 bytes
+	// On Linux/Unix: There is no theoretical limit but performance suffers with big values (>100KiB)
+	ErrSetDataTooBig = errors.New("data passed to Set was too big")
 )
 
 // Keyring provides a simple set/get interface for a keyring service.

keyring_darwin.go

@@ -15,6 +15,7 @@
 package keyring
 
 import (
+	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -28,7 +29,8 @@ const (
 	execPathKeychain = "/usr/bin/security"
 
 	// encodingPrefix is a well-known prefix added to strings encoded by Set.
-	encodingPrefix = "go-keyring-encoded:"
+	encodingPrefix       = "go-keyring-encoded:"
+	base64EncodingPrefix = "go-keyring-base64:"
 )
 
 type macOSXKeychain struct{}
@@ -37,8 +39,7 @@ type macOSXKeychain struct{}
 // 	return exec.Command(execPathKeychain).Run() != exec.ErrNotFound
 // }
 
-// Set stores stores user and pass in the keyring under the defined service
-// name.
+// Get password from macos keyring given service and user name.
 func (k macOSXKeychain) Get(service, username string) (string, error) {
 	out, err := exec.Command(
 		execPathKeychain,
@@ -57,17 +58,20 @@ func (k macOSXKeychain) Get(service, username string) (string, error) {
 	if strings.HasPrefix(trimStr, encodingPrefix) {
 		dec, err := hex.DecodeString(trimStr[len(encodingPrefix):])
 		return string(dec), err
+	} else if strings.HasPrefix(trimStr, base64EncodingPrefix) {
+		dec, err := base64.StdEncoding.DecodeString(trimStr[len(base64EncodingPrefix):])
+		return string(dec), err
 	}
 
 	return trimStr, nil
 }
 
-// Set stores a secret in the keyring given a service name and a user.
+// Set stores a secret in the macos keyring given a service name and a user.
 func (k macOSXKeychain) Set(service, username, password string) error {
 	// if the added secret has multiple lines or some non ascii,
 	// osx will hex encode it on return. To avoid getting garbage, we
 	// encode all passwords
-	password = encodingPrefix + hex.EncodeToString([]byte(password))
+	password = base64EncodingPrefix + base64.StdEncoding.EncodeToString([]byte(password))
 
 	cmd := exec.Command(execPathKeychain, "-i")
 	stdIn, err := cmd.StdinPipe()
@@ -80,6 +84,10 @@ func (k macOSXKeychain) Set(service, username, password string) error {
 	}
 
 	command := fmt.Sprintf("add-generic-password -U -s %s -a %s -w %s\n", shellescape.Quote(service), shellescape.Quote(username), shellescape.Quote(password))
+	if len(command) > 4096 {
+		return ErrSetDataTooBig
+	}
+
 	if _, err := io.WriteString(stdIn, command); err != nil {
 		return err
 	}

keyring_test.go

@@ -1,6 +1,10 @@
 package keyring
 
-import "testing"
+import (
+	"runtime"
+	"strings"
+	"testing"
+)
 
 const (
 	service  = "test-service"
@@ -16,6 +20,18 @@ func TestSet(t *testing.T) {
 	}
 }
 
+func TestSetTooLong(t *testing.T) {
+	extraLongPassword := "ba" + strings.Repeat("na", 5000)
+	err := Set(service, user, extraLongPassword)
+
+	if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
+		// should fail on those platforms
+		if err != ErrSetDataTooBig {
+			t.Errorf("Should have failed, got: %s", err)
+		}
+	}
+}
+
 // TestGetMultiline tests getting a multi-line password from the keyring
 func TestGetMultiLine(t *testing.T) {
 	multilinePassword := `this password

keyring_windows.go

@@ -24,6 +24,22 @@ func (k windowsKeychain) Get(service, username string) (string, error) {
 // Set stores stores user and pass in the keyring under the defined service
 // name.
 func (k windowsKeychain) Set(service, username, password string) error {
+	// password may not exceed 2560 bytes (https://github.com/jaraco/keyring/issues/540#issuecomment-968329967)
+	if len(password) > 2560 {
+		return ErrSetDataTooBig
+	}
+
+	// service may not exceed 512 bytes (might need more testing)
+	if len(service) >= 512 {
+		return ErrSetDataTooBig
+	}
+
+	// service may not exceed 32k but problems occur before that
+	// so we limit it to 30k
+	if len(service) > 1024*30 {
+		return ErrSetDataTooBig
+	}
+
 	cred := wincred.NewGenericCredential(k.credName(service, username))
 	cred.UserName = username
 	cred.CredentialBlob = []byte(password)