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

gopass-summon-provider outputs first line only #1652

Closed
OJFord opened this issue Nov 13, 2020 · 13 comments · Fixed by #1712
Closed

gopass-summon-provider outputs first line only #1652

OJFord opened this issue Nov 13, 2020 · 13 comments · Fixed by #1712

Comments

@OJFord
Copy link

OJFord commented Nov 13, 2020

Summary

I wasn't aware until #1627 that I should be using gopass-summon-provider rather than gopass directly with summon. Having resolved that, the output changed from multiline (preceded by the deprecation warning mentioned in that issue, that surely should be sent to stderr instead of stdout, and it wouldn't have been an issue anyway?) to first line only.

Steps To Reproduce

  1. echo -e '1\n2' | pass insert -m repro
  2. summon --yaml='REPRO: !var repro' sh -c 'echo $REPRO'

Expected behavior

Well, this is debatable I suppose. I noticed it because I tried to use if for an SSH key, and I only got the '----BEGIN...' line.

I expected all of it.

Environment

  • OS: Linux
  • OS version: 5.9.8-arch1-1
  • gopass Version:
gopass 1.10.1 (2020-09-13 00:16:24) go1.15.2 linux amd64
<root>     - gpg 2.2.23 -   fs 0.1.0
Available Crypto Backends: age, gpgcli, plain
Available Storage Backends: fs, gitfs, ondisk
  • Installation method: Arch package community/gopass

Additional context

@marco-m
Copy link
Contributor

marco-m commented Nov 18, 2020

Was it working before? I mean: putting a multiline text like the SSH key into an environment variable?

Because, if foo is a SSH key (so multiline), if I do:

$ gopass show foo

This is correctly multiline, while if I do

$ export X=$(gopass show foo)

Then $X is a single line (that contains the whole key, with newlines replaced with spaces). This looks to me a shell behavior unrelated to gopass ? Or am I missing something ?

@AnomalRoil
Copy link
Member

I guess @ckolumbus could help more than me here, as I am yet to use Summon.

@ckolumbus
Copy link
Contributor

ckolumbus commented Nov 18, 2020

@OJFord I was going after the original pass format description (see the repo under "Data Organization"

One approach is to use the multi-line functionality of pass (--multiline or -m in insert), and store the password itself on the first line of the file [...]

This lead me to the implementation of treating the first line as secret. And as summon needs only the secret I'm outputting only the first line. We could change this, but it would need to be in line with the format spec on what comprises the actual secret.

@AnomalRoil do you have some exact details on how the 'secret' part is defined (differently) in gopass?

@OJFord
Copy link
Author

OJFord commented Nov 19, 2020

@marco-m What you write is true (without some change to shell options), but seems unrelated to the reproduction steps I listed?

@ckolumbus That is indeed 'one approach', though you could interpret that format itself (and I'm sure some tooling does) as being the first however many lines before one that matches (?P<key>.+): (?P<val>.+). Also, as that page says after your quote:

Another approach is to use folders, and store each piece of data inside a file in that folder.

If you have a multiline secret, the scheme you describe makes it unrepresentable, which seems unfortunate.

I do understand that you're trying to avoid a similar/opposite problem for people that use that multiple key/value pairs, key-less password on first line format though. Hmm. Maybe I start to realise why gopass introduced mime types! (Though I've had to disable it for now for not dissimilar reasons.)

Personally I think the summoned behaviour should match show - if you don't want the whole file, deal with it in the summon process. $SUMMONED_VAR | head -n1. If there are common use cases for post-processing that makes that annoying, handling (in a more general way than assuming first line of course) belongs in summon, for all of its providers, IMO.

@ckolumbus
Copy link
Contributor

@OJFord I implemented the gopass-summon-provider to support the interface of summon, independently of what gopass does to the show command. Actually a change in gopass broke the summon use-case and we decided to segregate the summon specific stuff out to decouple the two functionalities to be able to evolve them independently.

With this in mind I would not follow all show aspects but put the summon compatibility first. If there is a valid summon use-case for multi-line secrets then this is definitely in scope.
But here the comment of @marco-m comes into play: how do multi-line secrets work with environment variables, because summon communicates to the called process via the environment.

So in order to provided a solution, IMHO we would need to answer the two questions

  1. how do we define muti-line secrets
  2. how do multi-line secrets work with environment variables

Any input on these?

@marco-m
Copy link
Contributor

marco-m commented Nov 20, 2020

But here the comment of @marco-m comes into play: how do multi-line secrets work with environment variables, because summon communicates to the called process via the environment.

Exactly :-) This is what I meant :-)

@AnomalRoil
Copy link
Member

It seems that env var can work with multi-line variables:
https://stackoverflow.com/q/49457787/2757014

Now, I guess we should try to play with multi-line and Go's https://golang.org/pkg/os/#Setenv
How does summon work with env variable?

I took a look and the current code seems to just do:

out.Print(ctx, secret.Get("password"))

So this become now a matter of "does gopass support multi-line key-value strings" and I don't think we do (yet?).

Regarding your question, " do you have some exact details on how the 'secret' part is defined (differently) in gopass?", I'm afraid I'm not sure what it is you're asking.
The secret in gopass is whatever is in an entry.
If mime is set to false, then the password is considered to be the first line.
Otherwise, the password is the first Password: value key-value entry in the entry. So Get("Password") should work with both kv entries (if mime is set to false) and with mime entries (if mime is enabled.)

I would need to double check what happens is we don't have a Password key in the entry and mime is set to false... I'm not sure this is handled in a special way currently, so it might be an empty value.

@OJFord
Copy link
Author

OJFord commented Nov 20, 2020

If there is a valid summon use-case for multi-line secrets then this is definitely in scope.

Exactly :-) This is what I meant :-)

Ah, gotcha now, sorry marco-m.

Well, actually the simplest way it works and comes in to play is with summon's temporary files - don't quote this syntax but off the top of my head --yaml 'VAR: !var:file my/secret' puts the contents of my/secret in a temporary file at the location given by $VAR. (The temporary file is removed when the process summon starts with that env var exits.)

Which is actually what I was trying to do here, (just simplified the reproduction for the issue) but the contents of the (plaintext) file was just the first line of the secret.

@marco-m
Copy link
Contributor

marco-m commented Nov 22, 2020

I refreshed my knowledge of summon (I was a bit rusty) and I have to admit that @OJFord is spot on 100%.
Let's forget environment variables, they are somewhat a red herring in this case.

As @OJFord writes, summon can also take a secret from gopass and generate a temporary file. This is a very valid use case for SSH private keys (which are multilines).

Currently, also the temporary file contains only the first line, so making gopass as summon provider only half-working.

@ckolumbus
Copy link
Contributor

@marco-m full ack, also read the docs for summon and the original pass. To me, env vars were the main use case and I did not consider the file scenario.

One solution could be to control the single vs. multi line issue with an environment variable and also handle the mime format of gopass that way. For mime we would also need to control which key actually holds the secret to output.

AFAIK we cannot give command line parameters to summon providers to control this. I have to check.

@marco-m
Copy link
Contributor

marco-m commented Nov 23, 2020

Another solution (which I personally would prefer) would be to treat everything "raw", as discussed in #1602.

@marco-m
Copy link
Contributor

marco-m commented Nov 23, 2020

From https://cyberark.github.io/summon/#providers

Providers are easy to write. Given the identifier of a secret, they either return its value or an error.

This is their contract:

  • They take one argument, the identifier of a secret (a string).
  • If retrieval is successful, they return the value on stdout with exit code 0.
  • If an error occurs, they return an error message on stderr and a non-0 exit code.

So a summon secret is a raw sequence of bytes, no interpretation (which in my opinion is the right approach :-). Providers accept no flags.

@AnomalRoil
Copy link
Member

So, Mime is now out of the equation and parsing is disabled on input and can easily be disabled on output.

What needs to be done here?

@AnomalRoil AnomalRoil added this to the 1.x.x milestone Jan 12, 2021
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Jan 15, 2021
This commit removes the binaries that were migrated to their dedicated
git repos.

Fixes gopasspw#1673
Fixes gopasspw#1649
Fixes gopasspw#1652
Fixes gopasspw#1631
Fixes gopasspw#1165
Fixes gopasspw#1711
Fixes gopasspw#1670
Fixes gopasspw#1639

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Jan 15, 2021
This commit removes the binaries that were migrated to their dedicated
git repos.

Fixes gopasspw#1673
Fixes gopasspw#1649
Fixes gopasspw#1652
Fixes gopasspw#1631
Fixes gopasspw#1165
Fixes gopasspw#1711
Fixes gopasspw#1670
Fixes gopasspw#1639

RELEASE_NOTES=[CLEANUP] Remove migrated binaries

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
dominikschulz added a commit that referenced this issue Jan 15, 2021
This commit removes the binaries that were migrated to their dedicated
git repos.

Fixes #1673
Fixes #1649
Fixes #1652
Fixes #1631
Fixes #1165
Fixes #1711
Fixes #1670
Fixes #1639

RELEASE_NOTES=[CLEANUP] Remove migrated binaries

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
kpitt pushed a commit to kpitt/gopass that referenced this issue Jul 21, 2022
This commit removes the binaries that were migrated to their dedicated
git repos.

Fixes gopasspw#1673
Fixes gopasspw#1649
Fixes gopasspw#1652
Fixes gopasspw#1631
Fixes gopasspw#1165
Fixes gopasspw#1711
Fixes gopasspw#1670
Fixes gopasspw#1639

RELEASE_NOTES=[CLEANUP] Remove migrated binaries

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants