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

SOPS panics when input type is not binary but output type is and input contains a 'data' mapping key. #596

Closed
0anton opened this issue Dec 18, 2019 · 5 comments · Fixed by #1289
Labels

Comments

@0anton
Copy link

0anton commented Dec 18, 2019

Sops (latest version) panics, when decrypting yaml file with explicit type specification. Same file with .yaml extension is decrypted successfully.

[anton@vm ]$ diff auth.yaml.enc2 auth.enc.yaml

[anton@vm ]$ sops --version
sops 3.5.0 (latest)

[anton@vm ]$ sops -d auth.enc.yaml >/dev/null ; echo $?
0

[anton@vm ]$ sops --input-type yaml auth.yaml.enc2
panic: interface conversion: interface {} is sops.TreeBranch, not string

goroutine 1 [running]:
go.mozilla.org/sops/v3/stores/json.BinaryStore.EmitPlainFile(0xc00036b100, 0x1, 0x1, 0x242, 0x1b6, 0xc000010030, 0x0, 0x0)
	/home/ajvb/gocode/src/go.mozilla.org/sops/stores/json/store.go:50 +0x156
main.editTree(0x1211f80, 0xc000370ba0, 0x1226bc0, 0x190eba8, 0x1226b00, 0x190eba8, 0xc00010e640, 0x41, 0x0, 0xc00036c120, ...)
	/home/ajvb/gocode/src/go.mozilla.org/sops/cmd/sops/edit.go:126 +0xbbe
main.edit(0x1211f80, 0xc000370ba0, 0x1226bc0, 0x190eba8, 0x1226b00, 0x190eba8, 0xc00010e640, 0x41, 0x0, 0xc00036c120, ...)
	/home/ajvb/gocode/src/go.mozilla.org/sops/cmd/sops/edit.go:105 +0x2c5
main.main.func8(0xc000128dc0, 0x0, 0x0)
	/home/ajvb/gocode/src/go.mozilla.org/sops/cmd/sops/main.go:767 +0x23c7
gopkg.in/urfave/cli%2ev1.HandleAction(0xe62ca0, 0x10603e0, 0xc000128dc0, 0x0, 0x0)
	/home/ajvb/gocode/pkg/mod/gopkg.in/urfave/cli.v1@v1.20.0/app.go:490 +0xc8
gopkg.in/urfave/cli%2ev1.(*App).Run(0xc0002db860, 0xc0000d4000, 0x4, 0x4, 0x0, 0x0)
	/home/ajvb/gocode/pkg/mod/gopkg.in/urfave/cli.v1@v1.20.0/app.go:264 +0x58c
main.main()
	/home/ajvb/gocode/src/go.mozilla.org/sops/cmd/sops/main.go:823 +0x2de4

cat auth.enc.yaml 
apiVersion: ENC[AES256_GCM,data:igo=,iv:2TZajuaEplwhxOLoc2wo/T9CbPip1cVh1J/uw5mcUR8=,tag:6Np390u1MMouj4n3TyhCww==,type:str]
data:
    auth: ENC[AES256_GCM,data:Ghp/DXmLL98ujqDJ1rJMyOlVdGL9KtUPigpfOzzpRBxBiF0lGv3aCZQOyLAfZGdi9UwZAUJomo4=,iv:OdglsUCjRmW9nZ7BK+DIj0tgipD+UwFLzeeUo6TtWqg=,tag:jpdAL3JCTPSMpZCtg8/Qww==,type:str]
kind: ENC[AES256_GCM,data:H16uOwEp,iv:WYnXYG0/AheXt7mWB33idNl1VAy7VF3pmpRPaiZd634=,tag:ThEFBFmTYZylv27QokOhKw==,type:str]
metadata:
    creationTimestamp: null
    name: ENC[AES256_GCM,data:uitF6Yx0lSs=,iv:4wZfhMtOdvET4q8hcWXpVfil2piv6UX4kK2OOlj/ekY=,tag:QHWcgibKq2sgMjPXFqV/RA==,type:str]
sops:
    kms: []
    gcp_kms:
    -   resource_id: xxx
        created_at: '2019-12-18T21:17:49Z'
        enc: CiQA8Xd3G6oq6KSE0uLyRP2trk24LeMP7GhcINOf3oYcffYGggUSTBJKCgwV1Dc3s3RQrBnikj0SKD35HZm0UxNj/ubXrA7HipNIabReUTRHFm19ghVNuEhEJzwCkVWxvSkaEP1kDwpX7TBPoejP4VW2Eso=
    azure_kv: []
    lastmodified: '2019-12-18T21:17:49Z'
    mac: ENC[AES256_GCM,data:w1BLQbnPT0tHDElqsrCMC5NICG/rbkxVGxZgDFmjgFbFkWFOg6A3UoqhXMHTiiAmyBEDXLHk4pPtzRX7O/67+GRR3DwaM5Lt8AaYB0AkT7O60TCWx23IzCu/jdnrj9K7rCrqNh+Fbdmihv6g2U6wjG0GuTx2dsDTRdxPB15juvE=,iv:ushcpUzLozcklWBQYPNtHq1ZltPzr994nch5yQ0FmMY=,tag:nF85o6scy4krdQiNIjzmGQ==,type:str]
    pgp: []
    unencrypted_suffix: _unencrypted
    version: 3.4.0
@autrilla
Copy link
Contributor

I think your issue is that while you've specified an --input-type, you haven't specified an --output-type. So SOPS is loading the file as YAML (as you told it to), but then tries to emit it as binary. Usually this isn't a problem, but unfortunately your YAML has a top-level 'data' key, which is the key that SOPS uses to stuff binary data into.

Panicking definitely isn't the right thing to do here, though :). In general, outputting as binary when the input is not binary should probably be disallowed.

@autrilla autrilla changed the title Sops panics, when decrypting yaml file with explicit type specification SOPS panics when input type is not binary but output type is and input contains a 'data' mapping key. Dec 26, 2019
@autrilla autrilla added the bug label Dec 26, 2019
@0anton
Copy link
Author

0anton commented Dec 31, 2019 via email

@LandazuriPaul
Copy link

I came across this exact same issue with sops version 3.7.1. @autrilla's answer explaining the reason behind it and the "workaround" to set an explicit output-type helped me out.

Also, I second @0anton's point of view in that by default, SOPS should use the input type for output as well, since this is what most users would want I reckon.

But I think that if this isn't agreed upon and if SOPS was to maintain the current approach, it should probably mention it in the README (for example when the --input-type option is presented, in the Important information on types chapter).

How do the collaborators feel about the "default output type being the same as the input type"? Because if this isn't going to change anytime soon, this is a very slight change to a README that I could propose in a tiny PR, just to help future users :)

I really enjoy using this tool, and thank you all for your hard work!
Merry Christmas and have a Happy New Year everyone!

P.S.: just fun to see that this answer is almost exactly 2 years after your previous Happy New Year wishes @0anton 😄 ⛄

@felixfontein
Copy link
Contributor

Also, I second @0anton's point of view in that by default, SOPS should use the input type for output as well, since this is what most users would want I reckon.

This would be a breaking change. There probably are many uses already which only work because of the current behavior, and will break once it is changed.

For example, think of someone who wants sops encrypted data to always use YAML, even for binary file. They do sops --encrypt --output-type yaml binary-file > binary-file.sops, and then want to decode it with sops --decrypt --input-type yaml binary-file.sops > binary-file. With your change, they wouldn't get the binary file back, but a YAML file with a key data and the binary file as the value for that key.

@felixfontein
Copy link
Contributor

felixfontein commented Sep 16, 2023

I've implemented better error handling including a --output-type hint in #1289.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants