diff --git a/dbus.go b/dbus.go index 65a8266..cda2903 100644 --- a/dbus.go +++ b/dbus.go @@ -1,8 +1,10 @@ +//nolint:err113 // dynamic errors in main are OK package main import ( "fmt" "os" + "strings" "github.com/godbus/dbus/v5" ) @@ -85,10 +87,11 @@ func (ss *SecretService) Close() error { // The attributes are a set of arbitrary name/value strings that were provided // when the secret was stored. // -// If more that one unlocked secret matches the attributes, a diagnostic error -// will be printed on stderr and the first matching secret will be returned. It +// Only secrets that match on all attributes and have no extra attributes are +// considered. If there are multiple exact matches, the first is returned. It // is not clear what the ordering of the secrets is, so the "first" secret may -// be arbitrary. +// be arbitrary. However, it should not be possible to have multiple secrets +// with the same attribues so this should not happen. // // Currently only unlocked secrets can be returned. If only a locked secret // matches the attributes, a diagnostic error will be printed to stderr and no @@ -108,20 +111,29 @@ func (ss *SecretService) Get(attrs map[string]string) (string, error) { return "", err } - switch { - case len(unlocked) > 1: - fmt.Fprintln(os.Stderr, "Warning: Got more than one secret back. Using only the first") - fallthrough - case len(unlocked) > 0: - secret, err := ss.getSecret(unlocked[0]) + // Find the first item with an exact attribute match. Sometimes + // attrs may be a subset of attributes that have been stored (e.g. + // may not contain a path), and we want to skip those. We return + // the secret of the first one found that matches. + for _, item := range unlocked { + ok, err := ss.attrsMatch(attrs, item) + if err != nil { + // We could continue to the next item but errors + // should not happen here, so lets surface them early. + return "", err + } + if !ok { + continue + } + secret, err := ss.getSecret(item) if err != nil { return "", err } return string(secret.Secret), nil - case len(locked) > 0: + } + + if len(locked) > 0 { fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock yet") - default: - // secret not found. return nothing } return "", nil @@ -152,15 +164,17 @@ func (ss *SecretService) Store(label string, attrs map[string]string, secret str return nil } -// Delete removes a secret matching the given attributes. If expectedSecret is -// not empty, then the secret matching the attributes will only be removed if -// the value of the secret stored matches expectedSecret. If expectedSecret is -// empty, then the secret will be removed if it just matches the attributes. +// Delete removes a secret matching the given attributes. If expectedPassword +// is not empty, then the secret matching the attributes will only be removed +// if the password in the value of the secret stored matches expectedPassword. +// If expectedPassword is empty, then the secret will be removed if it just +// matches the attributes. // -// If more that one unlocked secret matches the attributes, a diagnostic error -// will be printed on stderr and the first matching secret will be deleted. It +// Only secrets that match on all attributes and have no extra attributes are +// considered. If there are multiple exact matches, the first is returned. It // is not clear what the ordering of the secrets is, so the "first" secret may -// be arbitrary. +// be arbitrary. However, it should not be possible to have multiple secrets +// with the same attribues so this should not happen. // // Currently only unlocked secrets can be deleted. If only a locked secret // matches the attributes, a diagnostic error will be printed to stderr and no @@ -169,31 +183,49 @@ func (ss *SecretService) Store(label string, attrs map[string]string, secret str // If an error looking up the items occurs, an error returning the secret for // the selected item occurs, or the secret cannot be deleted, an error is // returned. -func (ss *SecretService) Delete(attrs map[string]string, expectedSecret string) error { +func (ss *SecretService) Delete(attrs map[string]string, expectedPassword string) error { unlocked, locked, err := ss.search(attrs) if err != nil { return err } + // Find the first item with an exact attribute match. Sometimes + // attrs may be a subset of attributes that have been stored (e.g. + // may not contain a path), and we want to skip those. Ensure that + // expectedSecret matches the stored secret value + // the secret of the first one found that matches. var itemPath dbus.ObjectPath - switch { - case len(unlocked) > 1: - fmt.Fprintln(os.Stderr, "Warning: Got more than one secret back. Erasing only the first") - fallthrough - case expectedSecret != "" && len(unlocked) > 0: - // We will only erase the secret when presented with a password if the password - // stored in the secret matches that password. - secret, err := ss.getSecret(unlocked[0]) + for _, item := range unlocked { + ok, err := ss.attrsMatch(attrs, item) if err != nil { + // We could continue to the next item but errors + // should not happen here, so lets surface them early. return err } - if string(secret.Secret) == expectedSecret { - itemPath = unlocked[0] + if !ok { + continue } - case len(locked) > 0: - fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock for erase yet") + // We will only erase the secret when presented with a password if the password + // stored in the secret matches that password. A secret can contain multiple + // fields separated by newlines. The password is the part before the first + // newline if there is one at all. + if expectedPassword != "" { + secret, err := ss.getSecret(item) + if err != nil { + return err + } + password, _, _ := strings.Cut(string(secret.Secret), "\n") + if password != expectedPassword { + continue + } + } + itemPath = item + break } + if !itemPath.IsValid() && len(locked) > 0 { + fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock for erase yet") + } if !itemPath.IsValid() { return nil } @@ -229,3 +261,31 @@ func (ss *SecretService) getSecret(itemPath dbus.ObjectPath) (secret Secret, err err = call.Store(&secret) return } + +// attrsMatch returns true if the given items have exactly the given +// attributes. If the item has extra or fewer attributes, or any values are +// different, false is returned. If the attributes of the item could be +// retrieved an error is returned. +func (ss *SecretService) attrsMatch(attrs map[string]string, itemPath dbus.ObjectPath) (bool, error) { + item := ss.conn.Object("org.freedesktop.secrets", itemPath) + prop, err := item.GetProperty("org.freedesktop.Secret.Item.Attributes") + if err != nil { + return false, err + } + + itemAttrs, ok := prop.Value().(map[string]string) + if !ok { + return false, fmt.Errorf("item attributes property is not a map: %v", itemPath) + } + + if len(itemAttrs) != len(attrs) { + return false, nil + } + for k, v1 := range attrs { + v2, ok := itemAttrs[k] + if !ok || v1 != v2 { + return false, nil + } + } + return true, nil +} diff --git a/main.go b/main.go index 56b541f..dab8baa 100644 --- a/main.go +++ b/main.go @@ -169,8 +169,7 @@ func (cmd *CmdStore) Run(gc *GitCredential, ss *SecretService) error { func (cmd *CmdErase) AfterApply(gc *GitCredential) error { cmd.validate(gc.Protocol != "", "protocol") cmd.validate(gc.Username != "", "username") - cmd.validate(gc.Host != "", "host") - cmd.validate(gc.Path != "", "path") + cmd.validate(gc.Host != "" || gc.Path != "", "host or path") return cmd.err } @@ -182,7 +181,7 @@ func (cmd *CmdErase) AfterApply(gc *GitCredential) error { // // [gitcredentials]: https://git-scm.com/docs/gitcredentials func (cmd *CmdErase) Run(gc *GitCredential, ss *SecretService) error { - return ss.Delete(makeAttrs(gc), formatSecretVal(gc)) + return ss.Delete(makeAttrs(gc), gc.Password) } // makeLabel returns a string describing the given GitCredential, used as a