-
Notifications
You must be signed in to change notification settings - Fork 28
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
Added support for AEAD encryption, which is default in Rails 5.2 #15
base: master
Are you sure you want to change the base?
Conversation
@doc """ | ||
Encrypts and signs a message. | ||
""" | ||
def encrypt_and_authenticate(message, secret, cipher \\ :aes_gcm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def encrypt_and_authenticate(message, secret, cipher \\ :aes_gcm) | |
def encrypt_and_authenticate(message, secret, cipher \\ :aes_256_gcm) |
Rails seems to use AES 256 GCM: https://github.com/rails/rails/pull/28132/files#diff-744c15344fa1f284281b429673de936cR231
which seems to be a cypher type in :crypto:
https://github.com/erlang/otp/blob/master/lib/crypto/src/crypto.erl#L495
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cconstantin can you take a look at this? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gstokkink @4xposed what this is missing is a passing test for rails 5.2. I got stuck on that and never had time to get back and complete. Any chance you can contribute that?
cookie, | ||
derive(conn, key, key_opts), | ||
derive(conn, opts.signing_salt, key_opts) | ||
derive(conn, opts.authenticated_encryption_salt, key_opts |> Keyword.put(:key_digest, :sha1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
derive(conn, opts.authenticated_encryption_salt, key_opts |> Keyword.put(:key_digest, :sha1)) | |
derive(conn, opts.authenticated_encryption_salt, key_opts |> Keyword.put(:digest, :sha)) |
I've tried use this branch with real Rails cookie. I've discovered that secret is generated differently. It was because Plug.Crypto.KeyGenerator
uses key :digest
instead of :key_digest
and because sha1
algorithm has to be represented as just :sha
(without the "1").
Not sure if it changed recently.
I am on Erlang 23.2.5
If a different key was intentional, it is not reflected in the derive
function.
Hi @cconstantin, any status on this? Best |
@mgom @4xposed @tomekowal @gstokkink sorry all, I've been away from the Elixir/rails space for a bit now, and I have no quick way to validate the PR. What's the consensus in this group, is this PR good to merge and release, after accepting the suggestions? Probably as a major release, due to breaking changes introduced by the new defaults. |
With the proposed changes, it worked for a project I was briefly developing. However, I don't have access to it anymore so I also don't have an easy way to confirm. Do others have a fresh setup using this branch? |
No description provided.