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

Reordering of fields in encrypted data causes decryption to fail. #833

Closed
jutley opened this issue Mar 10, 2021 · 6 comments
Closed

Reordering of fields in encrypted data causes decryption to fail. #833

jutley opened this issue Mar 10, 2021 · 6 comments

Comments

@jutley
Copy link

jutley commented Mar 10, 2021

For some reason, reordering fields in the output of sops -e causes the data to be unable to be decrypted by sops -d. Relying on the ordering of fields in a YAML document is incorrect usage, so this is clearly a bug.

➜  cat << EOF > input.yaml
foo: foo1
bar: bar1
EOF

➜  sops -e --kms=...  input.yaml > output.yaml

➜  head -2 output.yaml 
foo: ENC[AES256_GCM,data:iDIDaw==,iv:VEUE0K3YABM19whbUUVlkdPj65rqN3LvI0aV2jtNSyQ=,tag:Ub6s2QS71gNeoMG8BxCGEg==,type:str]
bar: ENC[AES256_GCM,data:r7UCvw==,iv:0o97dpfHoNmWHo4aE5DcO32dMi8JHiMpa7Zqw827CQk=,tag:hgwyHLQQvALX+9nMKomKhA==,type:str]

➜  sops -d output.yaml
foo: foo1
bar: bar1

➜  # Flip order of foo and bar in output.yaml

➜  head -2 output.yaml 
bar: ENC[AES256_GCM,data:r7UCvw==,iv:0o97dpfHoNmWHo4aE5DcO32dMi8JHiMpa7Zqw827CQk=,tag:hgwyHLQQvALX+9nMKomKhA==,type:str]
foo: ENC[AES256_GCM,data:iDIDaw==,iv:VEUE0K3YABM19whbUUVlkdPj65rqN3LvI0aV2jtNSyQ=,tag:Ub6s2QS71gNeoMG8BxCGEg==,type:str]

➜  sops -d output.yaml
MAC mismatch. File has 91F96DCE270B5CFA8909310CAAD8436E0ADFDB329BC5ED98FC11D3A8D7FE20844CF3E97D07C5547B6956DA34E00DD9686F6EA473C9BB1A14C1FB49AB300FA2C0, computed 3E8BC683F1CD809CF86E6C05504D12D1BDA2C43A23FE2A7AAEF2A5A1E19F55FF4A754D643E8F7F33A739F2462120DCA995AC75B59D36E5A80FF91DACB4560A75
@autrilla
Copy link
Contributor

This is intended and not a bug.

@jutley
Copy link
Author

jutley commented Mar 10, 2021

Is there anywhere I can get more information on why this is intended? YAML keys are unordered. This implementation violates the specification.
https://yaml.org/spec/current.html#id2508372

Additionally, it means that it can be difficult to process the YAML output with other applications, since they may reorder the fields (which is completely valid).

@autrilla
Copy link
Contributor

Comments are also not part of the YAML data, and we preserve them anyway, because it's what most users want. SOPS does not operate on YAML data, it operates on YAML files.

We aren't going to change this, people rely on it.

@autrilla
Copy link
Contributor

Additionally, it means that it can be difficult to process the YAML output with other applications, since they may reorder the fields (which is completely valid).

I suggest you have those applications work on the decrypted YAML and not the encrypted SOPS files. Any sort of processing you're going to do on that file is going to mess things up. You can use --ignore-mac too, but I strongly suggest you don't.

@felixfontein
Copy link
Contributor

felixfontein commented Mar 10, 2021

Is there anywhere I can get more information on why this is intended?

Sops computes a MAC of all values to be able to detect tampering with the file's contents. MACs require correct ordering.

(What sops could do is order all keys alphabetically, but that would be a breaking change.)

@jutley
Copy link
Author

jutley commented Mar 10, 2021

Sounds like the data fields should be implemented as a list rather than an object, though I understand that this would be a breaking change.

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

No branches or pull requests

3 participants