alex-luminal
commented
8 years ago As you said, credstash generates a new data encryption key for every secret (and every version of every secret), so it's never actually reusing counter/key combinations, which is why it's safe to use use the same counter every time.
That said, you raise a really good point about having to explain that to auditors, or someone in the future not realizing that they need to not ever reuse that key and doing something risky.
I'm happy to just randomize the ctr and stick that in the record on a future change, or accept a PR that does the same.
Thanks!
First: love your little utility!
Second: it seems that the the code uses the default value (=1) for the initial counter value for CTR mode for each and every key and encrypted message, which is ... kind of ok.
Reusing the same initial counter value with the same key is bad, but credstash seems to use a new unique key for each encrypted secret it stores. That particular usage pattern doesn't introduce any vulnerability for the same counter value.
However, any possible future changes of credstash or other possible applications that would use the encrypted secrets in DDB, which would use the same key to encrypt another secret, would have to change that practice and should use a different initial counter values.
Furthermore, any audit of the code will have security engineers cringe and they will have to spend additional effort to ensure that keys and counter values are not reused together.
Looking at the code, it feels that the changes to initialize the counter with a random value, and to store that value with the meta-data in the DDB-record, would not change the code significantly. This could even be implemented in a backwards compatible way (no record value for the initial counter value would default the value to 1).
Are you open to change the code to accommodate unique random initial counter values per key and per encrypted secret?