Question: How to manage merge conflicts with MAC signature

[chroto]

Hi. Great stuff here. Putting a PoC together for the team using git as a storage and distribution mechanism. We are currently using https://www.passwordstore.org/ and I'm attracted to sops for its support for AWS KMS.

What are your recommendations for settling merges since they (almost?) always result in a conflict on the mac signature with neither signature going to be the correct one.

I'm comfortable with sops --ignore-mac and :wq to re-generate a mac, but this could be a roadblock for adoption since it's additional training to micro-manage the MAC every time there is a merge.

Any recommendations for reducing the scope of this problem? How do you folks handle this issue?

[jvehent]

That's a very valid point, thanks for bringing it up. Conflict resolution has not been an issue yet, because we manage secrets in small files and it's very unlikely two persons will be modifying the same file at the same time. Generally speaking, I think a lot of small files is better than a single big one.

I have not looked into what git provides, but maybe we could invoke a sops command to merge two versions of a file when a conflict occur? That way, sops would verify both MACs, then merge the files and generate a new MAC. It might end up tricky to implement, but worth investigating.

[twolfson]

I took a shot at this tonight. I couldn't find anything straightforward with .gitattributes but following @jvehent's suggestion, I started on a custom merge configuration for SOPS. Here's a proof of concept demonstrating it handling decryption (which includes MAC validation) of source files and opening the default mergetool:

Code available here:

https://gist.github.com/twolfson/962b1eb776ce9947a09d4924d91fd8b2/b18db5fbcea4f3dd4632c9df8d94f14b644c1e52

There is a snag, we need a SOPS command to replace an entire files contents while maintaining the keys. After that is built, the script should be more/less good to go (maybe port to Python for portability).

Additionally, there is nothing to handle a key rotation. Maybe we should have other commands which dump metadata so we can diff/mergetool those as well?

I should also mention that another short term alternative would be to leverage well tested git hooks for merge/rebase like those in victorious-git:

https://github.com/twolfson/victorious-git

Edit: Locked down revision for gist

[jvehent]

Very nice analysis @twolfson ! :+1: I only skimmed through your scripts, but my first impression is that we need a bunch of local git configuration, which hurts user experience a bit. Do you think these could be wrapped under a sops --merge <file> command which handles all the magical git stuff?

[twolfson]

Upon reviewing my code, the .git/config + .gitattributes are more designed for automatic dependency resolution when running git merge (there shouldn't be a mergetool window open, it's all supposed to be automated).

Instead there's git mergetool which is used by users when a conflict is encountered. We should be able to use that without any .git/config or .gitattributes:

git mergetool --tool=./sops-mergetool.sh example.yaml

Unfortunately, I'm not in a good spot to try that out at the moment but will later.

[twolfson]

Outside of that I have a couple more notes:

• For replacing an entire file's contents, we should be able to accept input from STDIN but I don't think it works (tried last night)
  • cat foo.yaml | sops example.yaml
  • sops example.yaml < foo.yaml
• For handling changing meta data like keys, maybe we should pass through meta data to the mergetool as well (maybe via a command like sops --decrypt-with-meta). For example:

# Inside the 1 column of the mergetool
foo: bar
sops:
    mac: ABCDEF
    version: 1.7
    pgp:
    -   fp: 1022470DE3F0BC54BC6AB62DE05550BC07FB1A0A
# ...

Edit: Removed data: key from YAML example

[jvehent]

For replacing an entire file's contents, we should be able to accept input from STDIN but I don't think it works (tried last night)

Two solutions:

• sops -e foo.yaml > example.yaml
• cp foo.yaml example.yaml && sops -e -i example.yaml

That omits --pgp and --kms, but those can be driven through a .sops.yaml file.

For handling changing meta data like keys, maybe we should pass through meta data to the mergetool as well (maybe via a command like sops --decrypt-with-meta).

I think you're looking for sops --show_master_keys example.yaml which is already implemented.

[twolfson]

With respect to STDIN, is foo.yaml raw data (e.g. foo: bar)? If so, how does sops -e know which keys to use when outputting content?

With respect to sops --show_master_keys, is there a way to re-encrypt from that output format? That is convert

foo: bar
sops:
    mac: ABCDEF
    version: 1.7
    pgp:
    -   fp: 1022470DE3F0BC54BC6AB62DE05550BC07FB1A0A

to

foo: ENC[AES256_GCM,data:9l9h,iv:c/QuxSHYuqV4DWLbMMg1MKEKVbsP14sKFYChHSjxJeU=,tag:EDyqP6ckK0aRhE/XcNt6WQ==,type:str]
sops:
    mac: ENC[AES256_GCM,data:eiKgBSVnGKMIMvLKYees7Q74MXCw+XYSjjOXIWuyJfRuHTNU8dFsKjBhgZBL6RHqu4M5LkzS0MAfg0l9GMV1obnnzD0noht8N/Ge3PQ8QXf2aYYsciDsmBkGksQHC2XWHLF7eheLuicHlyZ2S73i+VAs/gOBywwXR+59x58Vntc=,iv:UlCCFAdOs1HuGrya8gCeoWnW8xj4Q2IarmRxbkwPV5I=,tag:z7l7aYnRqWdfL9wnPXDr8w==,type:str]
    version: 1.7
    pgp:
    -   fp: 1022470DE3F0BC54BC6AB62DE05550BC07FB1A0A
# ...

[twolfson]

Ah, I have answered my own question in both respects. -e accepts data that already has a sops key and will use that as its keys

https://github.com/mozilla/sops/blob/f63597f901f50f07ff72452b4bdb485518b85de7/sops/init.py#L511-L516

So the answers are:

• No, foo.yaml isn't raw data. It is data + a SOPS branch
• Yes, we can re-encrypt sops --show_master_keys via sops -e

[twolfson]

Alright, I explored git-mergetool. Unfortunately, we would need to have users set up .git/config with a [mergetool "sops-mergetool"] to get it working. That being said, the gist is now focused on git-mergetool and feels like a good experience (after the .git/config):

https://gist.github.com/twolfson/962b1eb776ce9947a09d4924d91fd8b2/1978689a51b7b2f388121fe71a530eae145863fa

[twolfson]

After thinking for a bit, there's no reason why that script has to be used via git mergetool (and consequently configured via .git/config). As I learned during iterations, we can generate all the necessary files from the conflicted state:

https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_manual_remerge

git show :1:example.yaml # Contents of `$BASE`
git show :2:example.yaml # Contents of `$LOCAL`
git show :3:example.yaml # Contents of `$REMOTE`

With respect to keeping the tool agnostic (e.g. agnostic to git/hg/svg), I don't think it's plausible for git as we need to find its mergetool configuration.

[cjbottaro]

I'm using SOPS via Helm Secrets. Is there a way to turn on --ignore-mac via a sops.yaml file or some global config?

Also, I can't believe this isn't a bigger issue. We ran into this almost immediately, and we're not even that big of an engineering team.

[autrilla]

I'm using SOPS via Helm Secrets. Is there a way to turn on --ignore-mac via a sops.yaml file or some global config?

I think this is a bad idea. You should use --ignore-mac reaaaaaally sparingly. It's the only thing that ensures your file hasn't been tampered with by e.g. reordering entries.

Also, I can't believe this isn't a bigger issue. We ran into this almost immediately, and we're not even that big of an engineering team.

This is probably because of your workflow. I've never had a single merge conflict with our SOPS-encrypted files at Mozilla in over two years, and we're a 10-engineer team. We try to isolate secrets into one file per 'application' and environment.

[reidab]

This is probably because of your workflow. I've never had a single merge conflict with our SOPS-encrypted files at Mozilla in over two years, and we're a 10-engineer team. We try to isolate secrets into one file per 'application' and environment.

I agree that global disabling MAC checking is a bad idea, but I'd be curious to know more about your workflow. Like @cjbottaro, I'm using sops + helm-secrets.

We currently store secrets files in the same repos as our application code, one file per environment. The place where we most often hit MAC conflicts is when two feature branches each require adding an additional key to the secrets file, and are both merged down to master. Git cleanly merges the parallel additions, so the only conflict left to resolve at merge time is the MAC.

My current workaround, like the OP mentions, is re-saving each conflicted file sops --ignore-mac, but this doesn't seem ideal.

[autrilla]

@reidab we don't use helm-secrets, so I'm not particularly sure about the viability of our workflow in your case. We currently store secrets file in a separate repo from the application code, and the infrastructure code is in a different repo as well. We do have one file per environment and per application, so that's a similarity between our workflows. We don't hit the issue you hit because our PRs that add secrets are extremely short-lived. We basically handle secrets completely independently from the code that uses them.

I'm not sure how else this could be handled, and I'm definitely open to ideas. I suppose we could write a sops mergetool command that does the merge for you, checking the MAC of each of the branches, and then opening the (diffed) decrypted files in your editor and reencrypting the result.

[cjbottaro]

@autrilla

I think this is a bad idea. You should use --ignore-mac reaaaaaally sparingly. It's the only thing that ensures your file hasn't been tampered with by e.g. reordering entries.

I understand. Is there a way to put that setting in the .sops.yaml file though?

[autrilla]

I understand. Is there a way to put that setting in the .sops.yaml file though?

Probably not. I don't think we want to make this easy. If you really want it, I'd say you should set up a bash alias or something like that.

[jvehent]

I agree with @autrilla . Allowing the user to disable security every time is a bad idea. I like this approach better:

I suppose we could write a sops mergetool command that does the merge for you, checking the MAC of each of the branches, and then opening the (diffed) decrypted files in your editor and reencrypting the result.

[ChristianWeissCG]

How many conflicts you get, depends on:

• your workflow
• frequency of parallel changes
• the number of leave nodes in your yaml file

Sops was designed which security in mind, not with being compatible with many workflow.

You should always use sops as an interactive editor (sops filename.yaml) or modify value by value using sops --set '["path"]["to"]["key"] "newValue"'. This ensures that checksum (called "mac") is always correct and also ["sops"]["lastmodified"] and ["sops"]["version"] are maintained.

Try to avoid the non-interactive full-file workflow: decryt + change + encrypt, as it introduces in fact a key rotation and puts a lot of noise to a diff, even for not changed values and not changed comments.

This also applies to 3-way-merges. So the 3-way-merge tool should be structure-aware (better then line-based diffs) and it needs to ensure that the "selected" value is applied on top of the encryted version of your "base" file . The 3-way-merge tool should apply a sequence of single-value-patches via sops --set '["path"]["to"]["key"] "newValue"' in the background while you are seeing the plain text version of all 3 file versions. The 3-way-merge tool should display only the plain text version without the sops section. That way you should not be asked to solve conflicts in ["sops"]["mac"], ["sops"]["lastmodified"] and ["sops"]["version"] - this conflict should be solved under the hood and automatically by the above procedure.

I recomment to not use comments in yaml files. Comments are allowed by the spec but treated by that spec as stepmom children, by declaring them as "presentational detail" (similar to indention and other formating stuff). For that reason many tools like yamldiff or yamlpatch ignore comments completly. While yaml was designed as a format for applications and humans, but sops ecosystem turned out to be focused on applications. Humans are now stepmom children of the yaml spec. Not using comments would reduce the noise (encrypted comments) and also would help if diff and patch tools ignore comments.

If i really want to have a comment for '["path"]["to"]["key"] "myValue"' then i tend to add a new leave node above that key: '["path"]["to"]["key_comment"] "MyCommentGoesHere"'. And as i do not want to have my comment being encrypted i usually use '["path"]["to"]["key_comment_unencrypted"] "MyCommentGoesHere"' - this helps when reviewing it at github, gitlab or bitbucket (in the browser) or in my IDE, without forcing me to decrypt all files always. Config options that are not security related also get "_unencrypted" suffix, which also reduces a lot of noise and as they are not part of the mac, changes to them do not trigger a sops-specific conflict.

I hope we will see better tool integration in the future. Hope to see a sops --verify (to verify mac without having a master key; ci server), a sops --diff (to create a structure-aware patch file; not line-based like unix diff) and a sops --applyPatch to easy review and move config data between feature branches.

BTW: 2-way sopsdiff support via git.textconv is useless on 3-way-merges / conflicts.

[autrilla]

Hope to see a sops --verify (to verify mac without having a master key; ci server)

This is not possible, because the MAC is authenticated with the data key, which is not available unless you have a master key.

[ChristianWeissCG]

ok, thanks for this detail.

Having a plain text mac and a signature sounds like a better option as its purpose is to verify that nothing has changed, but not to hide that data. That way a sops command line tool at my build server could verify that a sops-encrypted file is valid, without being a master key owner (not involved in encryption), just need one public key of a person that signed the checksum (should be possible to be signed by more then one person) and need to define trust to at least one of that persons once.

This is not neccessary a backward compatibility break - we could use separate key in sops section for that (optional; default on newer sops releases).

[autrilla]

If the MAC isn't authenticated someone can just modify the file by reordering, duplicating, or even swapping values and recomputing the MAC, which defeats its purpose.

We'd be happy to take a patch for some way to improve the merge conflict workflow though.

[ChristianWeissCG]

MAC is encrypted with AES_GCM. What i ask for is to not encrypt it, just calculate and then sign that MAC with e.g. GPG (https://www.gnupg.org/gph/en/manual/x135.html):

In addition to authenticating branches of the tree using keys as additional data, sops computes a MAC on all the values to ensure that no value has been added or removed fraudulently.

This sounds like a simple calculation without encryption involved. A (GPG) signature on top of it should be enough. An attacker would need to change the valuse/structure, re-calculate a valid MAC and needs a valid master key from a trusted signee to trick the validation process, by creating a valid signature. But if a master key is no longer private then your encryption is also broken - so it is not worst then current solution.

Can you provide more details on how the MAC is calculated? (i am not sure if i understand "authenticating branches" correctly)

[autrilla]

MAC is encrypted with AES_GCM. What i ask for is to not encrypt it, just calculate and then sign that MAC with e.g. GPG (https://www.gnupg.org/gph/en/manual/x135.html):

What if the file is only encrypted with AWS KMS? Our master keys can be symmetric keys, which totally defeat your purpose. If you only want support for GPG, you could just sign the file yourself.

What if it's encrypted with both AWS KMS and GPG? Should the MAC only be signed by GPG?

This sounds like a simple calculation without encryption involved. A (GPG) signature on top of it should be enough. An attacker would need to change the valuse/structure, re-calculate a valid MAC and needs a valid master key from a trusted signee to trick the validation process, by creating a valid signature. But if a master key is no longer private then your encryption is also broken - so it is not worst then current solution.

Sure, it is a simple calculation with no encryption involved. But we still need a way to sign/authenticate the data that works for all our master keys. I don't have any formal security training, but my understanding is that this is not possible with symmetric keys.

Also, perhaps this should be discussed in a separate issue? It's not really relevant to how managing merge conflicts should work.

Can you provide more details on how the MAC is calculated? (i am not sure if i understand "authenticating branches" correctly)

Sure. The relevant code is here.

[ChristianWeissCG]

Thanks for the link. True. This should go to a separat ticket: #437

[ChristianWeissCG]

"--verify" is not required to implement a "sops auto merge conflict resolver", but it may prevents to commit broken sops files to branches (git hook), which will bring the "sops auto merge conflict resolver" to fail.

[stpierre]

Very surprised to see this still outstanding. :/ We've been using SOPS for about a month and have already run into this issue, in spite of using very small (per-application, per-environment) files. But if the Mozilla team is only 10 people that could explain part of it.

As a workaround, it'd be awfully nice to just have a sops --regenerate-mac command or similar, so that after a 3-way merge is resolved we can regenerate it without having to edit with --ignore-mac, make a change, and save. (At least for me, just sops --ignore-mac and exiting immediately doesn't work; it detects that no change has been made and does nothing.)

[jvehent]

we use many small files. that's why we don't run into it.

[jvehent]

As a workaround, it'd be awfully nice to just have a sops --regenerate-mac command or similar, so that after a 3-way merge is resolved we can regenerate it without having to edit with --ignore-mac, make a change, and save. (At least for me, just sops --ignore-mac and exiting immediately doesn't work; it detects that no change has been made and does nothing.)

That goes along the line of the sops mergetool idea. Someone just needs to implement it.

[stpierre]

in spite of using very small (per-application, per-environment) files.

So do we. You also have a very small team.

An excellent tool that doesn't work for medium-sized teams or medium-sized files is actually not so excellent. :(

[jvehent]

It's a matter of someone being motivated enough to write the fix and solve the issue for everyone. Are you that person?