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

Ciphertexts are unauthenticated #37

Open
tarcieri opened this issue Nov 12, 2013 · 6 comments
Open

Ciphertexts are unauthenticated #37

tarcieri opened this issue Nov 12, 2013 · 6 comments
Milestone

Comments

@tarcieri
Copy link

This gem does not use a MAC to verify data integrity. This can be problematic if ciphertexts are malleable by an attacker, i.e. an attacker gains access to the database and can perform chosen ciphertext attacks.

I'd suggest you add something like ActiveSupport::MessageVerifier (which uses HMAC and performs a timing-attack resistant MAC comparison) to ensure the ciphertexts are authentic:

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/message_verifier.rb

@spikex
Copy link
Owner

spikex commented Nov 12, 2013

I see how this would be valuable. How do you envision storing the signature? If an attacker can manipulate the data isn't the signature vulnerable as well?

@tarcieri
Copy link
Author

The MAC can be appended or prepended to the ciphertext. It would not be possible for an attacker to compute a valid MAC without knowing the key.

@spikex
Copy link
Owner

spikex commented Nov 12, 2013

So roughly along the lines of:

# Encrypt and Sign

ciphertext = public_key.public_encrypt(plaintext,@padding)
digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@digest).new,
                                 @hmac_secret,
                                 plaintext)
ciphertext += "--#{digest}" # Unsafe assumption

# Decrypt and verify

ciphertext, digest = ciphertext.split("--")

plaintext = private_key.private_decrypt(ciphertext,@padding)

d = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@digest).new,
                                     @hmac_secret,
                                     plaintext)

raise InvalidSignature unless constant_time_comparison(d,digest)

@tarcieri
Copy link
Author

If you compute the HMAC of the plaintext, you're doing encrypt-and-MAC. This was used by SSH, and is vulnerable to chosen ciphertext attacks. See e.g.:

http://eprint.iacr.org/2002/078.pdf

What you'd want to do here is encrypt-then-MAC ala ActiveSupport::MessageEncryptor, where you encrypt the plaintext first, then MAC the ciphertext.

@spikex
Copy link
Owner

spikex commented Nov 12, 2013

Got it, identical plaintext generates identical MACs and badness ensues. So (again roughly):

# Encrypt and Sign

ciphertext = public_key.public_encrypt(plaintext,@padding)
digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@digest).new,
                                 @hmac_secret,
                                 ciphertext)
ciphertext += "--#{digest}" # Unsafe assumption

# [...]

@tarcieri
Copy link
Author

If you're doing RSA encryption exclusively, you should be fine with OAEP. A MAC is needed for symmetric encryption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants