-
Notifications
You must be signed in to change notification settings - Fork 10
Logstash hashing breaks idempotency? #247
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
Comments
The password_hash function uses a random string as salt if no secret string is provided. https://docs.ansible.com/ansible/latest/collections/ansible/builtin/password_hash_filter.html That breaks the logic of Jinja templates and the ansible.builtin.template module, since the password_hash function will always return a hash with a random salt. We could allow to specify a secret sting for the salt, optionally and ommitable, of course. I already tested it. If the salt is specified, the same hash will be returned. However, I don't know how much sense this makes regarding security. That probably makes only sense for idempotency. |
Then, we either should safe the salt somewhere or, maybe even better, just use a default and tell users to change it. And I think, idempotency is a very valid reason for implementing this. |
I'm not sure. The salt should always be random. So simply a default var is not a good idea. I probably would omit in every case, except for the pipelines? |
Why would you want to use a random salt every time? Maybe I missed something but when the salt is sufficiently random and long, it should be enough. Having a new random salt every time will change the template for the user everytime and will lead to the user being re-created or changed with every Ansible run. |
@afeefghannam89 the original code is from you. So ping :-) |
@danopt I can think of two options to make it a bit more robust and not relying on the user to change the variable:
So the first one seems to be a bit more reasonable to me. |
Yes, you are right. It should be globally unique. So every password should have it's own random salt. I didn't get that, that's why I said I'm not sure.
I agree, that's absolutely fine to save the a salted hash to a file. It would be saved in a file anyway. |
Ah, ok. Now I get it. To be honest, I did wonder if I was mistaken and was searching for a situation where having a changing salt would be beneficial. So creating one and saving it might be the best option. I'll adapt the PR. |
@widhalmt unfortunately we can not use SHA-256 without changing the default hashing algorithm by Elasticsearch which is bcrypt see here https://www.elastic.co/guide/en/elasticsearch/reference/current/security-settings.html#password-hashing-settings SHA-256, in particular, benefits a lot from being implemented on a GPU. Thus, if you use SHA-256-crypt, attackers will be more at an advantage than if you use bcrypt, which is hard to implement efficiently in a GPU. |
If you use SHA-256 without changing the default elasticsearch password hashing algorithm bcrypt you will get the same error which Lucinda had here #186 (comment) |
Thanks for the hint. I totally lost that. I'll change it right away. |
Yes, I've read the same. sha256 is a fast hashing algorithm and bcrypt a slow one. Slow hashing algorithms should always be preferred for passwords. |
Using randomly created hashes for our hash algorithm will break idempotency. Besides, it will lead to the `logstash_writer` user template being recreated with every run. I'm not sure if changing the contents every time can't have any more side effects we can circumvent by just using a fixed hash. fixes #247 fixes #251 fixes #244 --------- Co-authored-by: Afeef Ghannam <[email protected]> Co-authored-by: Afeef Ghannam <[email protected]>
The hashing in https://github.com/NETWAYS/ansible-collection-elasticstack/blob/main/roles/logstash/templates/logstash_writer_user.j2#L3 seems to break idempotency. So I had to deactivate it for Molecule in #243 .
This issue is for investigating if there really is a problem with the hashing. e.g. creating different hashes with repeated runs. Maybe this will also help with solving #186
The text was updated successfully, but these errors were encountered: