Added support for AEAD encryption, which is default in Rails 5.2#15
Added support for AEAD encryption, which is default in Rails 5.2#15cconstantin wants to merge 1 commit into
Conversation
| @doc """ | ||
| Encrypts and signs a message. | ||
| """ | ||
| def encrypt_and_authenticate(message, secret, cipher \\ :aes_gcm) |
There was a problem hiding this comment.
| 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.
@cconstantin can you take a look at this? :)
There was a problem hiding this comment.
@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.
| 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? |
5 participants
No description provided.