secretservice: replace pkg/errors for Go stdlib errors, and fix unhan…

…dled error (#111) * secretservice: fix SecretService.PromptAndWait discarding error This code was always returning a nil error instead of the error produced by the org.freedesktop.Secret.Prompt.Prompt call. Signed-off-by: Sebastiaan van Stijn <[email protected]> * secretservice: SecretService.openSessionRaw explicitly handle error pkg/errors' errors.Wrap function implicitly discards nil-errors. While this is convenient, it also can be err-prone, as this behavior differs from go stdlib, making it easy to miss conditions where the reader assumes an error is returned (but in reality no error). This patch updates the code to explicitly handle non-nil errors to prevent accidental regressions if this code would be rewritten using go stdlib. Signed-off-by: Sebastiaan van Stijn <[email protected]> * secretservice: rename var that shadowed type Signed-off-by: Sebastiaan van Stijn <[email protected]> * secretservice: fix GoDoc comment Signed-off-by: Sebastiaan van Stijn <[email protected]> * secretservice: replace pkg/errors for Go stdlib errors The pkg/errors dependency was introduced with the secretservice implementation in 7f2ef9f in March 2019. go1.13 (September 2019) introduced native support for unwrapping errors, no longer requiring this dependency to be used, and the pkg/errors module has been archived (as feature complete). While pkg/errors does have some advantages (for example, it can provide a stack trace), this functionality doesn't appear to be used in this module, and the pkg/errors package is not used in other implementations (for macOS). This patch removes the dependency, replacing its use for the equivalent in Go stdlib. Signed-off-by: Sebastiaan van Stijn <[email protected]> --------- Signed-off-by: Sebastiaan van Stijn <[email protected]>
keybase · Feb 27, 2025 · dd79abb · dd79abb
1 parent 7f41edf
commit dd79abb
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 30 deletions.
diff --git a/go.mod b/go.mod
@@ -4,7 +4,6 @@ go 1.21
 
 require (
 	github.com/keybase/dbus v0.0.0-20220506165403-5aa21ea2c23a
-	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.10.0
 	golang.org/x/crypto v0.32.0
 )

diff --git a/go.sum b/go.sum
@@ -2,8 +2,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/keybase/dbus v0.0.0-20220506165403-5aa21ea2c23a h1:K0EAzgzEQHW4Y5lxrmvPMltmlRDzlhLfGmots9EHUTI=
 github.com/keybase/dbus v0.0.0-20220506165403-5aa21ea2c23a/go.mod h1:YPNKjjE7Ubp9dTbnWvsP3HT+hYnY6TfXzubYTBeUxc8=
-github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
-github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=

diff --git a/secretservice/dh_ietf1024_sha256_aes128_cbc_pkcs7.go b/secretservice/dh_ietf1024_sha256_aes128_cbc_pkcs7.go
@@ -17,11 +17,11 @@ import (
 	"crypto/cipher"
 	cryptorand "crypto/rand"
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"io"
 	"math/big"
 
-	errors "github.com/pkg/errors"
 	"golang.org/x/crypto/hkdf"
 )
 

diff --git a/secretservice/secretservice.go b/secretservice/secretservice.go
@@ -1,11 +1,12 @@
 package secretservice
 
 import (
+	"errors"
+	"fmt"
 	"math/big"
 	"time"
 
 	dbus "github.com/keybase/dbus"
-	errors "github.com/pkg/errors"
 )
 
 // SecretServiceInterface
@@ -69,7 +70,7 @@ const DefaultSessionOpenTimeout = 10 * time.Second
 func NewService() (*SecretService, error) {
 	conn, err := dbus.ConnectSessionBus()
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to open dbus connection")
+		return nil, fmt.Errorf("failed to open dbus connection: %w", err)
 	}
 	signalCh := make(chan *dbus.Signal, 16)
 	conn.Signal(signalCh)
@@ -101,7 +102,10 @@ func (s *SecretService) openSessionRaw(mode AuthenticationMode, sessionAlgorithm
 	err = s.ServiceObj().
 		Call("org.freedesktop.Secret.Service.OpenSession", NilFlags, mode, sessionAlgorithmInput).
 		Store(&resp.algorithmOutput, &resp.path)
-	return resp, errors.Wrap(err, "failed to open secretservice session")
+	if err != nil {
+		return sessionOpenResponse{}, fmt.Errorf("failed to open secretservice session: %w", err)
+	}
+	return resp, nil
 }
 
 // OpenSession
@@ -125,17 +129,17 @@ func (s *SecretService) OpenSession(mode AuthenticationMode) (session *Session,
 		session.Public = public
 		sessionAlgorithmInput = dbus.MakeVariant(public.Bytes()) // math/big.Int.Bytes is big endian
 	default:
-		return nil, errors.Errorf("unknown authentication mode %v", mode)
+		return nil, fmt.Errorf("unknown authentication mode %v", mode)
 	}
 
 	sessionOpenCh := make(chan sessionOpenResponse)
 	errCh := make(chan error)
 	go func() {
-		sessionOpenResponse, err := s.openSessionRaw(mode, sessionAlgorithmInput)
+		resp, err := s.openSessionRaw(mode, sessionAlgorithmInput)
 		if err != nil {
 			errCh <- err
 		} else {
-			sessionOpenCh <- sessionOpenResponse
+			sessionOpenCh <- resp
 		}
 	}()
 
@@ -152,15 +156,15 @@ func (s *SecretService) OpenSession(mode AuthenticationMode) (session *Session,
 	case err := <-errCh:
 		return nil, err
 	case <-time.After(s.sessionOpenTimeout):
-		return nil, errors.Errorf("timed out after %s", s.sessionOpenTimeout)
+		return nil, fmt.Errorf("timed out after %s", s.sessionOpenTimeout)
 	}
 
 	switch mode {
 	case AuthenticationInsecurePlain:
 	case AuthenticationDHAES:
 		theirPublicBigEndian, ok := sessionAlgorithmOutput.Value().([]byte)
 		if !ok {
-			return nil, errors.Errorf("failed to coerce algorithm output value to byteslice")
+			return nil, errors.New("failed to coerce algorithm output value to byteslice")
 		}
 		group := rfc2409SecondOakleyGroup()
 		theirPublic := new(big.Int)
@@ -171,7 +175,7 @@ func (s *SecretService) OpenSession(mode AuthenticationMode) (session *Session,
 		}
 		session.AESKey = aesKey
 	default:
-		return nil, errors.Errorf("unknown authentication mode %v", mode)
+		return nil, fmt.Errorf("unknown authentication mode %v", mode)
 	}
 
 	return session, nil
@@ -182,13 +186,13 @@ func (s *SecretService) CloseSession(session *Session) {
 	s.Obj(session.Path).Call("org.freedesktop.Secret.Session.Close", NilFlags)
 }
 
-// SearchColleciton
+// SearchCollection
 func (s *SecretService) SearchCollection(collection dbus.ObjectPath, attributes Attributes) (items []dbus.ObjectPath, err error) {
 	err = s.Obj(collection).
 		Call("org.freedesktop.Secret.Collection.SearchItems", NilFlags, attributes).
 		Store(&items)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to search collection")
+		return nil, fmt.Errorf("failed to search collection: %w", err)
 	}
 	return items, nil
 }
@@ -211,15 +215,15 @@ func (s *SecretService) CreateItem(collection dbus.ObjectPath, properties map[st
 	case ReplaceBehaviorReplace:
 		replace = true
 	default:
-		return "", errors.Errorf("unknown replace behavior %v", replaceBehavior)
+		return "", fmt.Errorf("unknown replace behavior %d", replaceBehavior)
 	}
 
 	var prompt dbus.ObjectPath
 	err = s.Obj(collection).
 		Call("org.freedesktop.Secret.Collection.CreateItem", NilFlags, properties, secret, replace).
 		Store(&item, &prompt)
 	if err != nil {
-		return "", errors.Wrap(err, "failed to create item")
+		return "", fmt.Errorf("failed to create item: %w", err)
 	}
 	_, err = s.PromptAndWait(prompt)
 	if err != nil {
@@ -235,7 +239,7 @@ func (s *SecretService) DeleteItem(item dbus.ObjectPath) (err error) {
 		Call("org.freedesktop.Secret.Item.Delete", NilFlags).
 		Store(&prompt)
 	if err != nil {
-		return errors.Wrap(err, "failed to delete item")
+		return fmt.Errorf("failed to delete item: %w", err)
 	}
 	_, err = s.PromptAndWait(prompt)
 	if err != nil {
@@ -248,11 +252,11 @@ func (s *SecretService) DeleteItem(item dbus.ObjectPath) (err error) {
 func (s *SecretService) GetAttributes(item dbus.ObjectPath) (attributes Attributes, err error) {
 	attributesV, err := s.Obj(item).GetProperty("org.freedesktop.Secret.Item.Attributes")
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get attributes")
+		return nil, fmt.Errorf("failed to get attributes: %w", err)
 	}
 	attributesMap, ok := attributesV.Value().(map[string]string)
 	if !ok {
-		return nil, errors.Errorf("failed to coerce item attributes")
+		return nil, errors.New("failed to coerce item attributes")
 	}
 	return Attributes(attributesMap), nil
 }
@@ -264,12 +268,12 @@ func (s *SecretService) GetSecret(item dbus.ObjectPath, session Session) (secret
 		Call("org.freedesktop.Secret.Item.GetSecret", NilFlags, session.Path).
 		Store(&secretI)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get secret")
+		return nil, fmt.Errorf("failed to get secret: %w", err)
 	}
 	secret := new(Secret)
 	err = dbus.Store(secretI, &secret.Session, &secret.Parameters, &secret.Value, &secret.ContentType)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to unmarshal get secret result")
+		return nil, fmt.Errorf("failed to unmarshal get secret result: %w", err)
 	}
 
 	switch session.Mode {
@@ -282,7 +286,7 @@ func (s *SecretService) GetSecret(item dbus.ObjectPath, session Session) (secret
 		}
 		secretPlaintext = plaintext
 	default:
-		return nil, errors.Errorf("cannot make secret for authentication mode %v", session.Mode)
+		return nil, fmt.Errorf("cannot make secret for authentication mode %v", session.Mode)
 	}
 
 	return secretPlaintext, nil
@@ -299,11 +303,11 @@ func (s *SecretService) Unlock(items []dbus.ObjectPath) (err error) {
 		Call("org.freedesktop.Secret.Service.Unlock", NilFlags, items).
 		Store(&dummy, &prompt)
 	if err != nil {
-		return errors.Wrap(err, "failed to unlock items")
+		return fmt.Errorf("failed to unlock items: %w", err)
 	}
 	_, err = s.PromptAndWait(prompt)
 	if err != nil {
-		return errors.Wrap(err, "failed to prompt")
+		return fmt.Errorf("failed to prompt: %w", err)
 	}
 	return nil
 }
@@ -316,11 +320,11 @@ func (s *SecretService) LockItems(items []dbus.ObjectPath) (err error) {
 		Call("org.freedesktop.Secret.Service.Lock", NilFlags, items).
 		Store(&dummy, &prompt)
 	if err != nil {
-		return errors.Wrap(err, "failed to lock items")
+		return fmt.Errorf("failed to lock items: %w", err)
 	}
 	_, err = s.PromptAndWait(prompt)
 	if err != nil {
-		return errors.Wrap(err, "failed to prompt")
+		return fmt.Errorf("failed to prompt: %w", err)
 	}
 	return nil
 }
@@ -342,7 +346,7 @@ func (s *SecretService) PromptAndWait(prompt dbus.ObjectPath) (paths *dbus.Varia
 	}
 	call := s.Obj(prompt).Call("org.freedesktop.Secret.Prompt.Prompt", NilFlags, "Keyring Prompt")
 	if call.Err != nil {
-		return nil, errors.Wrap(err, "failed to prompt")
+		return nil, fmt.Errorf("failed to prompt: %w", call.Err)
 	}
 	for {
 		var result PromptCompletedResult
@@ -359,7 +363,7 @@ func (s *SecretService) PromptAndWait(prompt dbus.ObjectPath) (paths *dbus.Varia
 			}
 			err = dbus.Store(signal.Body, &result.Dismissed, &result.Paths)
 			if err != nil {
-				return nil, errors.Wrap(err, "failed to unmarshal prompt result")
+				return nil, fmt.Errorf("failed to unmarshal prompt result: %w", err)
 			}
 			if result.Dismissed {
 				return nil, PromptDismissedError{errors.New("prompt dismissed")}
@@ -401,6 +405,6 @@ func (session *Session) NewSecret(secretBytes []byte) (Secret, error) {
 			ContentType: "application/octet-stream",
 		}, nil
 	default:
-		return Secret{}, errors.Errorf("cannot make secret for authentication mode %v", session.Mode)
+		return Secret{}, fmt.Errorf("cannot make secret for authentication mode %v", session.Mode)
 	}
 }