Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message decryption relies (in part) on deserialization for validity #4138

Closed
AaronFeickert opened this issue May 25, 2022 · 4 comments
Closed
Assignees
Labels
A-security Area - Security related

Comments

@AaronFeickert
Copy link
Collaborator

AaronFeickert commented May 25, 2022

When a node wishes to determine if it can decrypt an incoming encrypted message, it performs several steps; these include:

  • Generating an ECDH shared secret using the provided ephemeral public key
  • Decrypting the message signature ("origin MAC")
  • Deserializing the signature
  • Deserializing the signature public key
  • Verifying the signature against the message and header
  • Decrypting the message
  • Deserializing the message

If the message was not encrypted for the node:

  • The first two steps will always succeed
  • The next two deserialization steps may succeed (possibly maliciously)
  • Signature verification will (almost certainly) fail

It is possible to alter this construction such that messages not intended for a node will be rejected with less computational effort, and without relying on particular serialization formats.

To do so, the sending node does the following:

  • Uses domain-separated KDFs (recommendation is Blake2b) with ECDH shared-secret input to produce a ChaCha20 key and ChaCha20-Poly1305 key, respectively:
    • key_message = Blake2b(persona="Tari message key", input=[ECDH_shared_secret])
    • key_signature = Blake2b(persona="Tari signature key", input=[ECDH_shared_secret])
  • Encrypts the message using ChaCha20 with key_message
  • Generates the signature as before
  • Encrypts the signature using the ChaCha20-Poly1305 AEAD construction with key_signature

The recipient node does the following:

  • Uses domain-separated KDFs (recommendation is Blake2b) with ECDH shared-secret input to produce a ChaCha20 key and ChaCha20-Poly1305 key, respectively:
    • key_message = Blake2b(persona="Tari message key", input=[ECDH_shared_secret])
    • key_signature = Blake2b(persona="Tari signature key", input=[ECDH_shared_secret])
  • Attempts to decrypt the encrypted signature using the ChaCha20-Poly1305 AEAD construction with key_signature
    • If tag verification fails prior to decryption, the message was not intended for this node, so we abort
  • Deserializes the signature as before
  • Verifies the signature as before
  • Decrypts the message using ChaCha20 with key_message
  • Deserializes the message as before

This approach adds an additional 16-byte ChaCha20-Poly1305 tag to the message structure.

This issue relates to (and in part supersedes) this issue.

@AaronFeickert
Copy link
Collaborator Author

The discretization approach in #4140 can be used here. To do so, assemble an extended message consisting of the original message length (in a platform-independent and fixed-length encoding), the original message, and arbitrary padding up to the next multiple of a globally-fixed base length. This extended message is then used for the encryption and decryption steps above.

@AaronFeickert
Copy link
Collaborator Author

In case it is useful here, it's possible to use fixed nonces for both the message encryption and the signature authenticated encryption, since in honest messages the ephemeral public key used for derived encryption keys is unique. A malicious or malfunctioning sender could reuse an ephemeral key and nonce anyway in a way not detected by the recipient unless it caches previous values, so there doesn't appear to be any particular security benefit to including a random nonce. The fixed nonces can be any value; zero is easy and makes the intent clear; further, the use of proper key derivation between the two encryption operations means the nonces may be identical.

@AaronFeickert
Copy link
Collaborator Author

Note that the new hashing API should be used for the key derivations proposed here.

jorgeantonio21 added a commit to jorgeantonio21/tari that referenced this issue Jul 22, 2022
aviator-app bot pushed a commit that referenced this issue Jul 28, 2022
Description
--- Fix issues #4138, #4333 and #4170.

Motivation and Context
--- Add domain separation and cipher authentication for out bound messaging

How Has This Been Tested?
--- Unit tests.
@hansieodendaal hansieodendaal added the A-security Area - Security related label Aug 15, 2022
@stringhandler stringhandler added this to the Stagenet Freeze milestone Nov 15, 2022
@SWvheerden
Copy link
Collaborator

Fixed in: #4336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-security Area - Security related
Projects
Archived in project
Development

No branches or pull requests

4 participants