From a2debe57c7b310520318df72c81bfe8c0816a4cb Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 27 Jul 2018 18:09:23 -0700 Subject: [PATCH 1/5] [ADR] Proposal for encoding symmetric cryptography --- docs/architecture/adr-015-symmetric-crypto.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/architecture/adr-015-symmetric-crypto.md diff --git a/docs/architecture/adr-015-symmetric-crypto.md b/docs/architecture/adr-015-symmetric-crypto.md new file mode 100644 index 00000000..e798f849 --- /dev/null +++ b/docs/architecture/adr-015-symmetric-crypto.md @@ -0,0 +1,79 @@ +# ADR 015: Need for symmetric cryptography + +## Context + +We require symmetric ciphers to handle how we encrypt keys in the sdk, +and to potentially encrypt `priv_validator.json` in tendermint. + +Currently we use AEAD's to support symmetric encryption, +which is great since we want data integrity in addition to privacy and authenticity. +We don't currently have a scenario where we want to encrypt without data integrity, +so it is fine to optimize our code to just use AEAD's. +Currently there is not a way to switch out AEAD's easily, this ADR outlines a way +to easily swap these out. + +### How do we encrypt with AEAD's + +AEAD's typically require a nonce in addition to the key. +For the purposes we require symmetric cryptography for, +we need encryption to be stateless. +Because of this we use random nonces. +(Thus the AEAD must support random nonces) + +We currently construct a random nonce, and encrypt the data with it. +The returned value is `nonce || encrypted data`. +The limitation of this is that does not provide a way to identify +which algorithm was used in encryption. +Consequently decryption with multiple algoritms is sub-optimal. +(You have to try them all) + +## Decision + +We should create the following two methods in a new `crypto/encoding/symmetric` package: +```golang +func EncryptSymmetric(aead cipher.AEAD, plaintext []byte) (ciphertext []byte, err error) +func DecryptSymmetric(key []byte, ciphertext []byte) (plaintext []byte, err error) +func RegisterSymmetric(aead cipher.AEAD, algo_name string, NewAead func(key []byte) (cipher.Aead, error)) error +``` + +This allows you to specify the algorithm in encryption, but not have to specify +it in decryption. +This is intended for ease of use in downstream applications, in addition to people +looking at the file directly. +One downside is that for the encrypt function you must have already initialized an AEAD, +but I don't really see this as an issue. + +If there is no error in encryption, EncryptSymmetric will return `algo_name || nonce || aead_ciphertext`. +This requires a mapping from aead type to name. +We can achieve this via reflection. +```golang +func getType(myvar interface{}) string { + if t := reflect.TypeOf(myvar); t.Kind() == reflect.Ptr { + return "*" + t.Elem().Name() + } else { + return t.Name() + } +} +``` +Then we maintain a map from the name returned from `getType(aead)` to `algo_name`. + +In decryption, we read the `algo_name`, and then instantiate a new AEAD with the key. +Then we call the AEAD's decrypt method on the provided nonce/ciphertext. + +`RegisterSymmetric` allows a downstream user to add their own desired AEAD to the symmetric package. + +## Status + +Proposed. + +## Consequences + +### Positive +* Allows us to support new AEAD's, in a way that makes decryption easier +* Allows downstream users to add their own AEAD + +### Negative + +### Neutral +* Caller has to instantiate the AEAD with the private key. +However it forces them to be aware of what signing algorithm they are using, which is a positive. \ No newline at end of file From caef5dcd6953f47f661dbbea6dc5102530d5a643 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Jul 2018 04:14:07 -0700 Subject: [PATCH 2/5] (Squash this) forgot to say that algo_name should be length prefixed --- docs/architecture/adr-015-symmetric-crypto.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/architecture/adr-015-symmetric-crypto.md b/docs/architecture/adr-015-symmetric-crypto.md index e798f849..166d99c0 100644 --- a/docs/architecture/adr-015-symmetric-crypto.md +++ b/docs/architecture/adr-015-symmetric-crypto.md @@ -44,7 +44,10 @@ One downside is that for the encrypt function you must have already initialized but I don't really see this as an issue. If there is no error in encryption, EncryptSymmetric will return `algo_name || nonce || aead_ciphertext`. -This requires a mapping from aead type to name. +`algo_name` should be length prefixed, using standard varuint encoding. +This will be binary data, but thats not a problem considering the nonce and ciphertext are also binary. + +This solution requires a mapping from aead type to name. We can achieve this via reflection. ```golang func getType(myvar interface{}) string { From c03ad56d554a929a59a48dd28667ccefc479bf5c Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Jul 2018 04:23:22 -0700 Subject: [PATCH 3/5] (squash this) Note that this breaks existing keys. --- docs/architecture/adr-015-symmetric-crypto.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/architecture/adr-015-symmetric-crypto.md b/docs/architecture/adr-015-symmetric-crypto.md index 166d99c0..fbaee7a1 100644 --- a/docs/architecture/adr-015-symmetric-crypto.md +++ b/docs/architecture/adr-015-symmetric-crypto.md @@ -65,6 +65,13 @@ Then we call the AEAD's decrypt method on the provided nonce/ciphertext. `RegisterSymmetric` allows a downstream user to add their own desired AEAD to the symmetric package. +## Implementation strategy + +The golang implementation of what is proposed is rather straight forward. +The concern is that we will break existing private keys if we just switch to this. +If this is concerning, we can make a simple script which doesn't require decoding privkeys, +for converting from the old format to the new one. + ## Status Proposed. @@ -76,6 +83,8 @@ Proposed. * Allows downstream users to add their own AEAD ### Negative +* We will have to break all private keys stored on disk. +They can be recovered using seed words, and upgrade scripts are simple. ### Neutral * Caller has to instantiate the AEAD with the private key. From ce9ddc7cd7204a6d966dd20c7a5deef8a3161ebb Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Jul 2018 06:32:54 -0700 Subject: [PATCH 4/5] (squash this) Note not to overwrite aead's. --- docs/architecture/adr-015-symmetric-crypto.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/architecture/adr-015-symmetric-crypto.md b/docs/architecture/adr-015-symmetric-crypto.md index fbaee7a1..7a587d18 100644 --- a/docs/architecture/adr-015-symmetric-crypto.md +++ b/docs/architecture/adr-015-symmetric-crypto.md @@ -64,6 +64,8 @@ In decryption, we read the `algo_name`, and then instantiate a new AEAD with the Then we call the AEAD's decrypt method on the provided nonce/ciphertext. `RegisterSymmetric` allows a downstream user to add their own desired AEAD to the symmetric package. +It will error if the AEAD name is already registered. +This prevents a malicious import from modifying / nullifying an AEAD at runtime. ## Implementation strategy From a040c36dfb262e2d7150289eb1dd896eccdf13cf Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Thu, 2 Aug 2018 10:43:47 -0700 Subject: [PATCH 5/5] (squash this) change adr number, remove redundancy in function names --- ...mmetric-crypto.md => adr-013-symmetric-crypto.md} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename docs/architecture/{adr-015-symmetric-crypto.md => adr-013-symmetric-crypto.md} (85%) diff --git a/docs/architecture/adr-015-symmetric-crypto.md b/docs/architecture/adr-013-symmetric-crypto.md similarity index 85% rename from docs/architecture/adr-015-symmetric-crypto.md rename to docs/architecture/adr-013-symmetric-crypto.md index 7a587d18..00442ab0 100644 --- a/docs/architecture/adr-015-symmetric-crypto.md +++ b/docs/architecture/adr-013-symmetric-crypto.md @@ -1,4 +1,4 @@ -# ADR 015: Need for symmetric cryptography +# ADR 013: Need for symmetric cryptography ## Context @@ -31,9 +31,9 @@ Consequently decryption with multiple algoritms is sub-optimal. We should create the following two methods in a new `crypto/encoding/symmetric` package: ```golang -func EncryptSymmetric(aead cipher.AEAD, plaintext []byte) (ciphertext []byte, err error) -func DecryptSymmetric(key []byte, ciphertext []byte) (plaintext []byte, err error) -func RegisterSymmetric(aead cipher.AEAD, algo_name string, NewAead func(key []byte) (cipher.Aead, error)) error +func Encrypt(aead cipher.AEAD, plaintext []byte) (ciphertext []byte, err error) +func Decrypt(key []byte, ciphertext []byte) (plaintext []byte, err error) +func Register(aead cipher.AEAD, algo_name string, NewAead func(key []byte) (cipher.Aead, error)) error ``` This allows you to specify the algorithm in encryption, but not have to specify @@ -43,7 +43,7 @@ looking at the file directly. One downside is that for the encrypt function you must have already initialized an AEAD, but I don't really see this as an issue. -If there is no error in encryption, EncryptSymmetric will return `algo_name || nonce || aead_ciphertext`. +If there is no error in encryption, Encrypt will return `algo_name || nonce || aead_ciphertext`. `algo_name` should be length prefixed, using standard varuint encoding. This will be binary data, but thats not a problem considering the nonce and ciphertext are also binary. @@ -63,7 +63,7 @@ Then we maintain a map from the name returned from `getType(aead)` to `algo_name In decryption, we read the `algo_name`, and then instantiate a new AEAD with the key. Then we call the AEAD's decrypt method on the provided nonce/ciphertext. -`RegisterSymmetric` allows a downstream user to add their own desired AEAD to the symmetric package. +`Register` allows a downstream user to add their own desired AEAD to the symmetric package. It will error if the AEAD name is already registered. This prevents a malicious import from modifying / nullifying an AEAD at runtime.