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

Documentation: cloak.migrate.ecto mix task ignores labels and is order-dependent #33

Closed
AndroidOatmeal opened this issue May 26, 2022 · 1 comment · Fixed by #39
Closed
Labels
docs Change the documentation

Comments

@AndroidOatmeal
Copy link

AndroidOatmeal commented May 26, 2022

Hi there. First off, thanks so much for the amazing library! It's been extremely useful for our business and maintaining security compliance.

I was looking at the documentation for the mix cloak.migrate.ecto task: https://hexdocs.pm/cloak_ecto/rotate_keys.html#content

In the example, the docs say to simply change the labels for your keys and add a new :default entry:

Then change the :default label to the new key, and demote the existing key to the :retired label.

config :my_app, MyApp.Vault,
  ciphers: [
    default: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V2", key: <<...>>},
    retired: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V1", key: <<...>>}
  ]

This example works as expected. However, the documentation is a bit misleading. It seems that cloak ignores labels like :default and :retired. Instead, it relies on the order of the cipher entries.

For example, consider this configuration:

config :my_app, MyApp.Vault,
  ciphers: [
    retired: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V1", key: <<...>>},
    default: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V2", key: <<...>>}
  ]

The only difference is the order of the entries. When we run mix cloak.migrate.ecto, the keys aren't rotated and remain V1. It seems the migration picks the first cipher entry.

Happy to provide more information if necessary. Thanks!

@danielberkompas
Copy link
Owner

@AndroidOatmeal You are correct. The order of the keys is important because I didn't think it was necessary to dictate the labels. I'll look things over and document this better.

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

Successfully merging a pull request may close this issue.

2 participants