Bug when decoding multi-line content in exec-env

[patricknelson]

It looks like when you encrypt a JSON file that has a string value that contains escaped sequences, those sequences are not unescaped properly.

EDIT: Actually, it looks like when you encrypt any data that has multiple lines, it is not unescaped properly when passed into environment variables via exec-env. See both JSON and YAML demos below (YAML samples are in my second comment).

---

I was able to confirm that directly editing the file via sops example.enc.json reveals the correct decrypted JSON contents when viewed inside of vim but when loading the contents via sops exec-eval example.enc.json [command], the environment variable contents isn't properly escaped.

My OS is Windows 10 (Version 10.0.18363.1198) running SOPS 3.6.1.

Use case:

To retain a Google Cloud service account JSON key in an environment variable that I can dump to a temporary file inside of a Docker container.

Example:

Create a JSON file file (e.g. example.plain.json) that contains a key representing my environment variable name K8S_SERVICE_ACCOUNT_KEY which then contains the escaped contents of my service account JSON file example.json and save it as a SOPS managed encrypted file called example.enc.json.

example.json (service account JSON file)

{
  "type": "service_account",
  "project_id": "example-project",
  "private_key_id": "abc123",
  "private_key": "-----BEGIN PRIVATE KEY-----\nthis is multiple lines\n-----END PRIVATE KEY-----\n",
  "client_email": "hello@example.com",
  "client_id": "1234567890",
  "auth_uri": "https://accounts.google.com/o/oauth2/auth",
  "token_uri": "https://oauth2.googleapis.com/token",
  "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
  "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/hello%40example.com"
}

example.plain.json (escaped and set inside string literal assigned to K8S_SERVICE_ACCOUNT_KEY)

{"K8S_SERVICE_ACCOUNT_KEY": "{\n  \"type\": \"service_account\",\n  \"project_id\": \"example-project\",\n  \"private_key_id\": \"abc123\",\n  \"private_key\": \"-----BEGIN PRIVATE KEY-----\\nthis is multiple lines\\n-----END PRIVATE KEY-----\\n\",\n  \"client_email\": \"hello@example.com\",\n  \"client_id\": \"1234567890\",\n  \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n  \"token_uri\": \"https://oauth2.googleapis.com/token\",\n  \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n  \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/hello%40example.com\"\n}\n"}

example.enc.json (sops managed/encrypted version of above file)

{
	"K8S_SERVICE_ACCOUNT_KEY": "ENC[AES256_GCM,data:+Ep7t9Y2xkGrhfmU/DVsjhHoUsOxO1kya0v0TpP9Zt8lCjsuHRI5AoojmghdXupDnD5C/g1y32UwNtcVKJZS1Mj3b3qj1tuQMzU79X8fPnbxAXpIX6QM8U5MBDiQtG8LJ9fnwIZuJ6UWez1AzlhhbKrSUQ3JI7XR3QHIFRb0ftfgPi6BFL1zV1hhFas31LUhhS+YwIwI/FWkwqxRtQYXMtpil5E7Qh93/+XhBdnnncpPze/DTnEUJQE42wJkQPzLzlUJHyYBPmC4eyiwiJX4S/cTtrQ6suboN6bPytCMqv9oPWIvUrVqLt0DTJ1wUSqrQJnv7mXM7wyWOmW2dO2IgkX7EIjQ4fSK1RlVGDHoctU/7kxgalry27hOWv3ag6ZamX7kS4x/geTSrZQwknsCch/wUVDYZ4BwWOkISNnscciWDQDY8VLej6pwO3qntuWY8ehfIMXOO0PU/MwjfHK8Q55cNzNi8calXoIvMKbo29D1VCwy3VNJK6F07ltInsvjXUpYb2QCkVgT5sKA0JBlAOUnW/OD/RE0N0g4lTSo/Ok8GJJzHPMcCdbBreVA8FiMDVp8P4C+klG1oa15J0wjWZk8MxAoTnhZo/0+OuBXYXUF8IIqeOuoktQlxUDIYyXetE+uOBOSq9uGDEZGQvOINeY9C4n64ffGRgXbP74E7nNpfxj+xWtuOHkumAtQIrL9htBWBHZY+aGmMm+ZtExpid3dNuy11ghn5ghor0Y=,iv:zJWaHadrNXb+Pg1c3TfO175Ap/2Ar3S47SWNQiFrNeM=,tag:SF8Eg3mUuxsTLJz+84kjkQ==,type:str]",
	"sops": {
		"kms": null,
		"gcp_kms": null,
		"azure_kv": null,
		"hc_vault": null,
		"lastmodified": "2020-12-10T06:07:37Z",
		"mac": "ENC[AES256_GCM,data:m3L1xwxeQw74PfU9fueGFSSQ2zxjXIwiWu76//j/InKewG5DjI6HWs2H01L41jlgT9BlqHTJuxkH9Hr2Iam/ba5Ncke7HfNi0psRIqSbjVqbwobeCG9WTX8cKWI5hnkrj87BsSr0nXSawE5ZTBXyz/gbbsZGqt1OW0OAZGPQ00U=,iv:IK4z/rccGe4et1t1PKFIX3IEG3eOduJpndMdbLYZqlA=,tag:UsRGoNpkml54UpRFI+siXQ==,type:str]",
		"pgp": [
			{
				"created_at": "2020-12-10T06:07:37Z",
				"enc": "-----BEGIN PGP MESSAGE-----\n\nwcBMAyUpShfNkFB/AQgAIuIKJ1DE/BnlcKib/o3XtLRxWNLz7mCdRKhA2suWft8d\n1x0VmNIcSvGl8euj2x2nFZ38hJTDqwMr0s4LgiUtvt1d9OLi/cXsK1cVCTKMQSgS\nSy2Wtqni1fXROgGOEyFVjmCP/KgsEsg8Wlv/0Zxa1D+M6wuimaP8tqr+ty5YLVn8\nj8lJTfmwlJrpwjKBac3lGHY52zXNREUcNH/cewnygJrgP4QR67rGPt/GHDNdWWSY\nlXOkfckzBcu4HUm77YZOZwaiebpjk3VT4q8CspenOlkEB8X2ujRlmEWQRRtw8wP5\nbj0ddaf5v+ZxkywH9TDX5xehV0GJqBgMzK8Qvv0kKdLgAeQVUvi+OUVawB3ykTFn\ndHnZ4Wwe4D3gpeFDAeCC4hC2KSLgjOUpqjP7lM57KjqWKxlTDdzfMDgBrN4auq9C\nuFvGnFEyVOC25Oo3ZyuN0W3tX1gVgRMKLqvivR/9MOHRKgA=\n=qIDG\n-----END PGP MESSAGE-----",
				"fp": "FBC7B9E2A4F9289AC0C1D4843D16CEE4A27381B4"
			}
		],
		"unencrypted_suffix": "_unencrypted",
		"version": "3.6.1"
	}
}

Steps to reproduce:

Use the dev PGP key here (signature FBC7B9E2A4F9289AC0C1D4843D16CEE4A27381B4): https://github.com/mozilla/sops#test-with-the-dev-pgp-key

gpg --import pgp/sops_functional_tests_key.asc

Encrypt the plaintext file:

sops -e --pgp FBC7B9E2A4F9289AC0C1D4843D16CEE4A27381B4 example.plain.json > example.enc.json

Confirm everything looks good. Here you'll see that the escaped line breaks at the first level in the file are still present (\n) and the line breaks inside one the string values of the file is properly double escaped as expected (i.e. \\n).

sops example.enc.json

However, if you attempt to load it as an environment variable, the escaped line breaks in the service account key JSON file are NOT unescaped like they should have been (i.e. these should become actual line breaks).

sops exec-env example.enc.json "cmd.exe"

... and in the new shell...

echo %K8S_SERVICE_ACCOUNT_KEY%

Same thing in git shell (MINGW64); this is also the same output when going through a docker container (e.g. docker-compose with the syntax K8S_SERVICE_ACCOUNT_KEY: "${K8S_SERVICE_ACCOUNT_KEY}" to import environment variables.

sops exec-env example.enc.json "/usr/bin/bash"

... and in the new shell...

echo $K8S_SERVICE_ACCOUNT_KEY

[patricknelson]

p.s. I think this may also affect YAML files as well. Here's what I could quickly slap together. Basically for the life of me couldn't figure out how to get it to properly retain and pass on the env variable when using this syntax:

example.plain.yml

K8S_SERVICE_ACCOUNT_KEY: |-
  {
    "type": "service_account",
    "project_id": "example-project",
    "private_key_id": "abc123",
    "private_key": "-----BEGIN PRIVATE KEY-----\nthis is multiple lines\n-----END PRIVATE KEY-----\n",
    "client_email": "hello@example.com",
    "client_id": "1234567890",
    "auth_uri": "https://accounts.google.com/o/oauth2/auth",
    "token_uri": "https://oauth2.googleapis.com/token",
    "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
    "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/hello%40example.com"
  }

example.enc.yml

K8S_SERVICE_ACCOUNT_KEY: ENC[AES256_GCM,data:Qek7i6YzMb9kAj1CzDopUPDiqooRj0FuSbg2SmaYiG7Qj2LMY+/o5C2MYvxoaHVoNLeEnIKYU9MLJonCYA3ONJ/Ts+oesqwL8iLaHXoCnHHLPGq6q8DKmmxNSGGt2ym6jCB7qmEHTTo0iEppnfK18edXAXjg7O6iEbCp6ROc/zVGK/qx6Tn+OZvO0/53t1K2ylpfGol0NqJUWVr9/feaEQ75OU29mUU9d3ih9ZCTaP3juZiXZ3sDMrmyBJyQ2opHUeds5bjogqeBLicP4JHg1arIobc41tS0B+TDY37vJze6ipoo982CzM3FBPgNd1XuMn0eY/jbxF+HPbMGH6xc2e9Jss+dVD9l3nnBRospkVszEHBqo3sEWoZPbne/JvFWkbT7SOesXDhoVwJTLUtQ8tCLLmFH1DnU89Ne7jLWRtMTCZManENBtrhYAIr+1exV4IdC5ttluESFsfbv0D41kMExwuW+xtobBBSJlqeMZfpvzYim5n9COfOTAEos/SQg01rTOHNDMJuGE8FN/0ndw9ohqoUJZGof8U68sx1T1+jqphsxNYef1860FrdycXxc3SCZ6xH3m1v1GvDadsBsCP5nQXdsqESZu7iqz3WNFTYnN8ShSw3uR95Ps6m0jA60M75ACeWQmagAbRhW6603zY1oG3LmgDRNZCAKD0vU8EbwY1IReDngubnLn9Y5VamYeq8dxoy6nJ8uEcurHEAqngIb2uy/AHOXXEBN+g==,iv:gHoDRW4Bw8wi+TQxtElvvX1u05i46eZuldsjraGQTW0=,tag:1gOQYh3AIY5r4HU9BivJkA==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    lastmodified: '2020-12-10T06:55:32Z'
    mac: ENC[AES256_GCM,data:ngTscrtPqyztg+6DgpDNYgp7xKZnSV1QlYFJ/TZs7pVFkCDe+3I5BXSv2Y9d79OkxZbiTL4kPuj3i2ZHUinKAoBEi91Psoqm54L6KOuCJv43n83Ble3LDgIDpY4HeaLq0NMDrzIkDThg3t47DLF4bcWBV7uW7jHiWsgWGe+hCdI=,iv:g1rz6aQdd5dDjCY45PVRS9hZgF57gTFtNzjAMIcE51M=,tag:/HXuPrDjlZ+rlT0KFXQ0lg==,type:str]
    pgp:
    -   created_at: '2020-12-10T06:55:31Z'
        enc: |-
            -----BEGIN PGP MESSAGE-----

            wcBMAyUpShfNkFB/AQgAAzH4BViXklCyum2d+3WwX5rnpPjTGNCqqDNb6yOpVo/n
            gisQfZdbRGYFWW3FoJe4/MfBPblIZDlvDfObHgGaVBWwpvXL1lPy2Z6sK49G7ZCf
            uIjgnGi8Lhh3miIU7ISTjj30CVq4HG7xgxg1FM8gP9XQqztO7VR4FM99kWDTTBaG
            oY3RtXm3rE7vk5aoRfFBrGxIh5Cr/CvTyctqhYv/03T5Jcds/3sgyjv3l2Me0Y0M
            0/rItBRxxoKrymeE9RTvj2yB1ygu79ontCXmjhkEJIC3Bh6wuW4m2dfRS6rRc8cD
            aj221R+VveVgdi3dq0rhgJ5yhQdPOaTEZrnUuYsXcdLgAeRUwFFbFVxDQgxAynvP
            HZ/t4QJG4IrgNeHE9OAl4gD8SnzgzeXqCxFOW2FghPbvCMl6lgaqcsgWcCKMjVVj
            Rm2N3TpfvuAN5KSQ7mKotGmzQYV5Be4qvVji0xzFuuH0mAA=
            =ptP2
            -----END PGP MESSAGE-----
        fp: FBC7B9E2A4F9289AC0C1D4843D16CEE4A27381B4
    unencrypted_suffix: _unencrypted
    version: 3.6.1

Initially I thought there was an issue with my formatting of multi-line YAML blocks, but it still comes back out fine...

And exec-env produces similar results when loading the YAML as an environment variable in shell:

sops exec-env example.enc.yml "cmd.exe"

[felixfontein]

exec-env essentially calls sops --decrypt --output-type dotenv example.enc.json, which looks like this:

K8S_SERVICE_ACCOUNT_KEY={\n  "type": "service_account",\n  "project_id": "example-project",\n  "private_key_id": "abc123",\n  "private_key": "-----BEGIN PRIVATE KEY-----\nthis is multiple lines\n-----END PRIVATE KEY-----\n",\n  "client_email": "hello@example.com",\n  "client_id": "1234567890",\n  "auth_uri": "https://accounts.google.com/o/oauth2/auth",\n  "token_uri": "https://oauth2.googleapis.com/token",\n  "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",\n  "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/hello%40example.com"\n}\n

This is then split into lines and added to the environment: https://github.com/mozilla/sops/blob/master/cmd/sops/subcommand/exec/exec.go#L86-L96 I know too little about the dotenv format to be able to say whether this is wrong or not, or how to properly fix this.

[mltsy]

I don't think this is related to the linked issue really, but I do think this is probably a bug in the dotenv encoder (which is a really tricky encoder, due to ambiguity in the dotenv format specification). This does seem like a use-case that should be addressed though. It's clearly wrong.

Maybe you could provide an example of a correct dotenv file that does work? (just so someone can use it as a reference for what the output should look like in this case).

[felixfontein]

@mltsy it's an internal use of dotenv inside sops, it's not related to "real" dotenv files. For exec-env, sops decodes the JSON (or YAML) as dotenv and parses the dotenv file.

[mltsy]

@felixfontein Right, but I think there's a bug in how it translates to dotenv format because it's coming out with single slashes in both areas, when it should be coming out with a single slash in the first highlighted case and a double slash in the second highlighted case, right?

[felixfontein]

@mltsy I really don't know the dotenv format (on the basic idea, which doesn't cover escaping stuff and having newlines in values), so I can't really say :)

[patricknelson]

Thanks for the research; actually it looks like the bug definitely occurs well before it gets to the specific block @felixfontein called out. I'm not actually a go developer, so I actually just installed GoLand and started debugging. 😄

I was able to trace it down to this line/commit: 5d32d9a#diff-c9aa3c7e333fa3bc6fa314822b8da1e449389f6bb14b68608e47d048c4997a08R122

The way I see it, there may be two bugs.

proper escaping of full dotenv contents

Related... The dotenv command should ensure that it escapes FULL contents, not just new lines. So in this case, it is injecting \n escape squences when encountering a new line, but yet all the other contents (e.g. existing encoded \n's inside the JSON object) are left untouched, meaning this is now mangling output anyway. So it should be fully escaped such that it's in dotenv format so that when it's evaluated, it can be reversed cleanly. I found that https://github.com/joho/godotenv did a pretty good job of this when I tested it with something like this at (please excuse the hack, as stated above, I'm a n00b in Go).

At https://github.com/mozilla/sops/blob/master/stores/dotenv/store.go#L122-L123, encode like this instead:

// Generate line in dotenv from a single key/value pair, escaping with godotenv lib.
valueMap := make(map[string]string)
valueMap[item.Key.(string)] = item.Value.(string)
line, err := godotenv.Marshal(valueMap)
if err != nil {
	return nil, err
}

For exec-env, the core os/exec package may have a different syntax from dotenv

It looks like this uses os/exec to execute a command. This happens here in ExecWithEnv (via the windows BuildCommand for me). From what I can tell, while it does accept KEY=VALUE syntax in the .Env property, the similarities in formatting could be breaking down after that. I really couldn't find much documentation on how Go's exec package handles environment variables more complex than plain alphanumeric strings, e.g. what about = or "'s? https://golang.org/pkg/os/exec/#example_Command_environment

And, apparently, Go's os/exec package wraps os.StartProcess. However, being new to how Go works, I have no clue how it's passing environment variables. Is it just cloning current variable scope? Does it make sense to call os.SetEnv here? Presumably not since that'd actually change scope at some higher level (at minimum current process scope, at most OS level, no clue).

Either way, I'm betting the syntax here for how KEY=VAL is different from typical dotenv syntax and it'd be better to try using some kind of API that handles it for you, like SetEnv which takes the key/value pair separately.

[patricknelson]

TL;DR: Bottom line is that I think the environment variables (for both exec-eval and dotenv output) should be abstracted away so that:

1. The environment variable name and values are handled separately (so that the value is properly escaped) and...
2. The escaping is done properly depending on the implementation detail (e.g. dotenv may follow a different syntax than os.StartProcess)

[autrilla]

@patricknelson what you've said makes sense to me. I definitely agree that using a "standard" dotenv parser would be good, but that's a bit complicated because there's no dotenv standard. And changing ExecWithEnv to use the actual values instead of passing in the entire line is a good idea as well. It would even let us use it for e.g. YAML files.

[mattiaforc]

Any update on this? Latest release is still affected by this