Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically export creators key to the store #2159

Merged
merged 2 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,4 @@ If any of the above don't work check out the [troubleshooting section](#troubles
## Releasing

See [docs/releases.md](docs/releases.md).

39 changes: 39 additions & 0 deletions docs/hacking.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Hacking on gopass

Note: See [CONTRIBUTING.md](../CONTRIBUTING.md) for an overview.

This document provides an overview on how to develop on gopass.

## Setting up an isolated development environment

### With GPG

`gopass` should fully respect `GOPASS_HOMEDIR` overriding all gopass internal paths.
However it will still use your normal GPG keyring and configuration. To override this
you will need to set `GNUPGHOME` as well and possibly generate a new keyring.

```bash
$ export GOPASS_DEBUG_LOG=/tmp/gp1.log
$ export GOPASS_HOMEDIR=/tmp/gp1
$ mkdir -p $GOPASS_HOMEDIR
$ export GNUPGHOME=$GOPASS_HOMEDIR/.gnupg
# Make sure that you're using the correct keyring.
$ gpg -K
gpg: directory '/tmp/gp1/.gnupg' created
gpg: keybox '/tmp/gp1/.gnupg/pubring.kbx' created
gpg: /tmp/gp1/.gnupg/trustdb.gpg: trustdb created
$ gpg --gen-key
$ go build && ./gopass setup --crypto gpg --storage gitfs
```

### With age

Using `age` is recommended for development since it's easier to set up. Setting
`GOPASS_HOMEDIR` should be sufficient to ensure an isolated environment.

```bash
$ export GOPASS_DEBUG_LOG=/tmp/gp1.log
$ export GOPASS_HOMEDIR=/tmp/gp1
$ mkdir -p $GOPASS_HOMEDIR
$ go build && ./gopass setup --crypto age --storage gitfs
```
16 changes: 15 additions & 1 deletion internal/action/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (s *Action) clone(ctx context.Context, repo, mount, path string) error {

// make sure the parent directory exists.
if parentPath := filepath.Dir(path); !fsutil.IsDir(parentPath) {
if err := os.MkdirAll(parentPath, 0700); err != nil {
if err := os.MkdirAll(parentPath, 0o700); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that 0700 is already octal and the way we are used to see permission in the unix world.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a gofumpt change. I don't have a strong opinion on this, but since we don't have strong community of "style reviewers" here I'd prefer to rely on automation as much as possible, i.e. follow the conventions that the tools suggest.

return exit.Error(exit.Unknown, err, "Failed to create parent directory for clone: %s", err)
}
}
Expand Down Expand Up @@ -200,7 +200,21 @@ func (s *Action) cloneCheckDecryptionKeys(ctx context.Context, mount string) err
return nil
}

var exported bool
if sub, err := s.Store.GetSubStore(mount); err == nil {
debug.Log("exporting public keys: %v", idSet.Elements())
exported, err = sub.ExportMissingPublicKeys(ctx, idSet.Elements())
if err != nil {
debug.Log("failed to export missing public keys: %w", err)
}
} else {
debug.Log("failed to get sub store: %s", err)
}

out.Noticef(ctx, "Please ask the owner of the password store to add one of your keys: %s", strings.Join(idSet.Elements(), ", "))
if exported {
out.Noticef(ctx, "The missing keys were exported to the password store. Run `gopass sync` to push them.")
}
Comment on lines +203 to +217
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice addition, especially for usage with age.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I did never test this with age. It should work, but ... 🤔


return nil
}
Expand Down
15 changes: 10 additions & 5 deletions internal/action/recipients.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import (
"github.com/urfave/cli/v2"
)

var (
removalWarning = `
var removalWarning = `


@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Expand All @@ -31,7 +30,6 @@ This feature is only meant for revoking access to any added or changed
credentials.

`
)

// RecipientsPrint prints all recipients per store.
func (s *Action) RecipientsPrint(c *cli.Context) error {
Expand Down Expand Up @@ -95,8 +93,15 @@ func (s *Action) RecipientsAdd(c *cli.Context) error {
for _, r := range recipients {
keys, err := crypto.FindRecipients(ctx, r)
if err != nil {
out.Printf(ctx, "WARNING: Failed to list public key %q: %s", r, err)
if !force {
out.Warningf(ctx, "Failed to list public key %q: %s", r, err)
var imported bool
if sub, err := s.Store.GetSubStore(store); err == nil {
if err := sub.ImportMissingPublicKeys(ctx, r); err != nil {
out.Warningf(ctx, "Failed to import missing public keys: %s", err)
}
imported = err == nil
}
if !force && !imported {
continue
}
keys = []string{r}
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type Config struct {
// New creates a new config with sane default values.
func New() *Config {
return &Config{
AutoImport: true,
AutoImport: false,
AnomalRoil marked this conversation as resolved.
Show resolved Hide resolved
ClipTimeout: 45,
ExportKeys: true,
Mounts: make(map[string]string),
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestNewConfig(t *testing.T) {

cfg := config.New()
cs := cfg.String()
assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:true, ClipTimeout:45, ExportKeys:true, NoPager:false, Notifications:true,`)
assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:false, ClipTimeout:45, ExportKeys:true, NoPager:false, Notifications:true,`)
assert.Contains(t, cs, `SafeContent:false, Mounts:map[string]string{},`)

cfg = &config.Config{
Expand Down
3 changes: 2 additions & 1 deletion internal/store/leaf/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (s *Store) Crypto() backend.Crypto {

// ImportMissingPublicKeys will try to import any missing public keys from the
// .public-keys folder in the password store.
func (s *Store) ImportMissingPublicKeys(ctx context.Context) error {
func (s *Store) ImportMissingPublicKeys(ctx context.Context, newrs ...string) error {
// do not import any keys for age, where public key == key id
// TODO: do not hard code exceptions, ask the backend if it supports it
if _, ok := s.crypto.(*age.Age); ok {
Expand All @@ -39,6 +39,7 @@ func (s *Store) ImportMissingPublicKeys(ctx context.Context) error {
if err != nil {
return fmt.Errorf("failed to get recipients: %w", err)
}
rs = append(rs, newrs...)
for _, r := range rs {
debug.Log("Checking recipients %s ...", r)
// check if this recipient is missing
Expand Down