Sops: Add ability to strip encrypted or unencrypted suffix on decryption

[kristaxox]

This would allow the use of sops without needing to update key names in the areas that they are used to have _encrypted or _unencrypted.

Encrypted example:

db:
    password_encrypted: ENC[...]

Upon decryption with a "strip suffix option":

db:
    password: pw123

[endorama]

Following https://github.com/mozilla/sops/issues/539 I'm going to work on this!

[autrilla]

I'm with @jvehent here https://github.com/mozilla/sops/issues/539#issuecomment-533555867. I'm not sure this is SOPS's job.

It's not that I don't think your use case is valid. As maintainers, I think we have to be careful what features we allow into our code base, since once they're in there, we basically have to support them forever.

[endorama]

I do understand that, but requiring everyone using that feature to have a wrapper script to remove those suffix if very painful for the users. As sops is already performing such a removal for key metadata, I would expect it to remove also the suffix (I would agree with @davidovich https://github.com/mozilla/sops/issues/539#issuecomment-533559825 in considering that a sops level metadata).

As maintainers, I think we have to be careful what features we allow into our code base,

As I'm not a maintainer, is not in my interest to just push some code without approval, but if your fear the maintenance costs may be worth discussing this looking at a possible implementation? 🙂

[autrilla]

I will gladly look at it, but I can make no guarantee that it will be merged.

[jvehent]

I understand the temptation to bake every use case into a single tool, but I really think you should follow the unix approach here, and make a simple separate tool that edits your yaml files the way you want.

Sops is a security tool, and every non-security use-case we support into it makes it larger and more difficult to audit.

[dalibor-aleksic-atomia]

I would also want to have this feature.

It messes my sops integration with existing tooling and writing another layer of transformation introduces unnecessary complexity.

Specifically, in my use-case where I use Ansible @endorama's sops_vars plugin from here, I would have to add additional transformation logic on top of that either via new plugin or bake-it in sops_vars plugin itself (but that is not the job for this plugin). Another way is to change the variable name in all existing playbook tasks.

I would agree with @endorama

As sops is already performing such a removal for key metadata, I would expect it to remove also the suffix

(I intuitively expected the removal of the suffix but discovered that this isn't a case)

and agree with @davidovich #539 (comment) about threating it as metadata/tag.

Currently, I'm using "workaround" to list all items that I want to encrypt them in encrypted_regex: (password|api_secret|access_token), but with encrypted_suffix I would have better control which properties to encrypt or not.

[endorama]

Sops is a security tool, and every non-security use-case we support into it makes it larger and more difficult to audit.

I would agree if this feature was not already present, in this case I believe is "partially" present: this missing piece makes using this feature a burden in different use cases.
I do agree that a reduced surface is good for security but IMO security tools should also be easy, intuitive and functional to use.

I'd like to highlight a couple of use cases that are currently only achievable with encrypted-regex or cannot be achieved at all.
(I would prefer not to require my team to use regex, they are brittle and fragile)

The first use case is Kubernetes secrets. As the Kubernetes API server has no clue what _encrypted or _unencrypted means, you either encrypt the entire file or use encrypted-regex.
The second use case is Helm chart secrets (for example in user supplied values.yaml files). In this case encrypting the entire file makes no sense, and regex can be impossible to write properly (YAML is structured and is possible to have secrets in different places across the file).

In both cases requiring an external tool makes everything more fragile and complex.
It's a pity that this suffix feature, very useful because ensure integrity even for unencrypted values, cannot be used in such use cases.

We can live without it, and I will happily contribute to other piece of the codebase, but please consider allowing this. Will make sops easier to integrate and more effective for your users.

[autrilla]

The second use case is Helm chart secrets (for example in user supplied values.yaml files). In this case encrypting the entire file makes no sense, and regex can be impossible to write properly (YAML is structured and is possible to have secrets in different places across the file).

FWIW, I've seen people keep two .yaml files, one for secrets, and one for non-secrets. You can pass helm additional values files, so that would solve this problem.

[dalibor-aleksic-atomia]

FWIW, I've seen people keep two .yaml files, one for secrets, and one for non-secrets. You can pass helm additional values files, so that would solve this problem.

I thought about that, but it is not always achievable. For example:

If you use var files in ansible in format

params.yml

database_hostname: example.com
database_login_username: admin

secrets.yml

database_login_password: admin_password

These files would be merged (variables with the same name in both files overwritten, but this is not a case) and the end result would be:

database_hostname: example.com
database_login_username: admin
database_login_password: admin_password

sops encrypt secrets.yml file and :tada:

BUT

for people who use hash/dictionary format

params.yml

database:
  hostname: example.com
  login:
    username: admin
    password: admin_password

and when they split it into different files

params.yml

database:
  hostname: example.com
  login:
    username: admin

secrets.yml

database:
  login:
    password: admin_password

End result would be

database:
  login:
    password: admin_password

as overwrites previously defined dictionaries.

Arguably you could change Ansible ANSIBLE_HASH_BEHAVIOUR to merge (described here) and have desired end result

but as their documentation says:

This setting controls how variables merge in Ansible. By default Ansible will override variables in specific precedence orders, as described in Variables. When a variable of higher precedence wins, it will replace the other value. Some users prefer that variables that are hashes (aka ‘dictionaries’ in Python terms) are merged. This setting is called ‘merge’. This is not the default behavior and it does not affect variables whose values are scalars (integers, strings) or arrays. We generally recommend not using this setting unless you think you have an absolute need for it, and playbooks in the official examples repos do not use this setting In version 2.0 a combine filter was added to allow doing this for a particular variable (described in Filters).

because it can cause unwanted issues with playbook that are not written in mind with this behavior.

[dalibor-aleksic-atomia]

What are security/maintainability/other concerns that prevent adding this feature?

[jvehent]

It messes my sops integration with existing tooling and writing another layer of transformation introduces unnecessary complexity.

That complexity is located in a separate script though, and does not make the maintainability of Sops more difficult.

The point I'm trying to make is that, while I'm sympathetic with your needs and use cases, we can't have a command line flag for every use case that people have. It's impossible to maintain down the road, and we'll end up breaking user workflows when we inevitably deprecate some of these functionalities.

Sops must focus on security: adding new encryption formats, or integrating with new KMS, and guaranteeing a high level of security for encrypted files. I think it's reasonable to say that file transformation use cases are out of scope and should be handled by external scripts, simply because we cannot predict how every user will want these transformations to happen.

[autrilla]

As a compromise, maybe we could have a script that ships within the sops binary that can be used for this, but is split off from all other functionality in a subcommand. What do you think @jvehent ?

[jvehent]

I'm open to ideas, and I'm not completely closed to merging the stuff in main sops, depending on complexity of the patch and where we think we should draw the line.

[fredgate]

I agree that sops can't support every use case. But the fact is that the --encrypted-suffix and --unencrypted-suffix exist, and that no tool can understand a file with these suffixes. So IMHO I think if someone uses these prefixes, he wants that these suffixes are no longer present in decrypted files. I would even say that I don't see a case where we might want to keep them.
An option to remove these prefixes would be good and backward compatible.

[autrilla]

It seems like there is a decent amount of people that want something like this. I think most people who have contributed to the SOPS codebase would agree that it is more complex than it needs to because we have added too many features too quickly, even when only one person found it useful.

I think I would be okay with what I mention in https://github.com/mozilla/sops/issues/543#issuecomment-541230066, but I would prefer to avoid a flag, since that is where most of the clutter in our CLI is. Something like sops -o foo.yaml -d foo.enc.yaml; sops stripsuffixes foo.yaml would be acceptable to me, since it can be kept fairly separate in code.