Support stable unencrypted values without underscore

[berlincount]

What are you trying to accomplish?

Some workflows involve using ejson encrypt secrets.json to encrypt secrets, and often involve variable names used by upstream software. Sometimes, non-confidential variables are mixed into related ejson files.

This can lead to values being encrypted that would be helpful to stay in plaintext, or at least somewhat readable.

$ cat test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "this should stay very secret",
    "DEBUG": "true"
}
$ ejson encrypt test.ejson
Wrote 385 bytes to test.ejson.
$ cat test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "EJ[1:5Ho+m7CaRbCRxZOaaIzrSilnsLfYbE3UVvL2cFYuHXw=:EJcu+MzTv0N+SED/Rsd4GLgdI04DZ2ny:sAidrmCCI9ZwLJIrSMWzsjB0yiYb7xPJolcVdipYDLvqxZMYjYvrIatWC+U=]",
    "DEBUG": "EJ[1:5Ho+m7CaRbCRxZOaaIzrSilnsLfYbE3UVvL2cFYuHXw=:uw6kbrA8ePIUNTZX51+iPrlUI4MUHXnF:Q3A5N48LpJt1xvq+QQS++8pZvTM=]"
}
$ ejson decrypt test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "this should stay very secret",
    "DEBUG": "true"
}

While ejson without this PR supports not encrypting variable names starting with an underscore, this doesn't necessarily help when we don't have full control over the variable name.

With the PR, stably unencrypted values become possible:

$ cat test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "this should stay very secret",
    "DEBUG": "EJ[0:true]"
}
$ ejson encrypt test.ejson
Wrote 283 bytes to test.ejson.
$ cat test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "EJ[1:hf80/6hYhj/QhORAENO72Y/vl2d++xVhjcbbiNZ+V3k=:kBNHRMOishKxZFqiCjHN+l8xeRdt2brw:m0/5Mmzc07K5MWXhmAcZrfL303dVWcLKjca45ymhf1dTKK2tsdBvZa1kinw=]",
    "DEBUG": "EJ[0:true]"
}
$ ejson decrypt test.ejson
{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "SECRET_KEY": "this should stay very secret",
    "DEBUG": "true"
}

Sadly, introducing "Schema 0" breaks the code structure a bit, as it has to completely ignore any values required for crypto in all other cases. Not sure whether this could be handled better, but this leaves the code reasonably readable.

[methodmissing]

No strong opinions at this point, but deferring to @burke

[paracycle]

@berlincount but these EJ[0:XXX] values do not survive a roundtrip. Supposing you lost the original JSON file that you created the encrypted EJSON file from and now you want to recover it. Up to this point, you could decrypt the EJSON file and change values, etc and encrypt it again. With this change though, that would break the "stable unencrypted value" promise, since those fields will end up being encrypted again.

[berlincount]

@paracycle true. Hm. No idea how to do that while provide true transparency for the values inside, though. Guess this problem would show up during key rotation as well.

You'll not lose any values, though, just the easy readability.

[paracycle]

@berlincount Here is a different idea to provide "stable unencrypted values": We create another well-known underscore key that holds the names of all the top-level keys that SHOULD NOT be encrypted.

For example:

{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "_unencrypted_keys": [ "DEBUG" ],
    "SECRET_KEY": "this should stay very secret",
    "DEBUG": "true"
}

when encrypted becomes:

{
    "_public_key": "63ccf05a9492e68e12eeb1c705888aebdcc0080af7e594fc402beb24cce9d14f",
    "_unencrypted_keys": [ "DEBUG" ],
    "SECRET_KEY": "EJ[1:hf80/6hYhj/QhORAENO72Y/vl2d++xVhjcbbiNZ+V3k=:kBNHRMOishKxZFqiCjHN+l8xeRdt2brw:m0/5Mmzc07K5MWXhmAcZrfL303dVWcLKjca45ymhf1dTKK2tsdBvZa1kinw=]",
    "DEBUG": "true"
}

Does that not serve to solve your problem?

TL;DR: There is nothing another level of abstraction/indirection can't solve 😉

[berlincount]

@paracycle tbh, I'd rather live with the problem (which is merely a noticeable inconvenience for deployment) than complicate the codebase by what's needed to support that.

and being honest - the world has (far) too much hard-to-maintain-code with too much abstractions and indirections already.

[burke]

I don't like this on a conceptual level.

The semantics for when a value should be encrypted are intentionally very simple, and the test for when a field is encrypted is also intentionally simple.

With this change, it's no longer possible to scan a field for "EJ[... in order to check whether it's encrypted. With this scheme, a sufficiently-entropic hex-encoded byteslice would look encrypted while being represented in plaintext.

EJSON is one piece of software where we should strive for as few features as possible.

EJSON simply shouldn't be used as a config file like this IMO. It should contain the secret portion of the environment to be merged into the non-secret configuration.

[berlincount]

Discussed with @burke to some further extent in person :)

[burke]

Thinking on this some more, there are a couple of reasons that I'm uncomfortable with this proposal:

• I don't like that values won't roundtrip. This isn't a huge deal in practice but it is a break with current behaviour.
• The EJ[0: syntax is kind of opaque, I think. It makes sense if you understand the format, but most people just read these values as line noise.

I think I like @paracycle's proposal more, but even it would complicate, for example, ejson-safety-check.

The opinion that originally led me to design it this way is that using environment variables for secrets is a bad idea in the first place, and people should be loading secret values from disk. I still feel that way, but clearly the ship has sailed.

My actual preference for things like this would be, if it is for some reason mandatory that DEBUG=true be unencrypted while doing this (IMO) horrible practice of loading secrets as environment, that the loading process automatically strips leading underscores from variable names.

I wonder if anyone on @Shopify/dev-infra has opinions on this.

[lugray]

I agree with the general premise of keep ejson as simple as possible. A loading process that strips a leading underscore makes a great deal of sense, though for backwards compatibility we'd probably need to have it load two variables - with and without the underscore.

[thepwagner]

Greetings from the future: this is very stale, I'm going to close it.

[berlincount]

Rest in piece, sad little branch :D