search

NETWAYS / ansible-collection-elasticstack

A collection to install and manage the Elastic Stack
GNU General Public License v3.0
10 stars 8 forks source link

Logstash hashing breaks idempotency? #247

Closed widhalmt closed 1 year ago

widhalmt commented 1 year ago

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

danopt commented 1 year ago

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.

widhalmt commented 1 year ago

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.

danopt commented 1 year ago

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?

widhalmt commented 1 year ago

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.

widhalmt commented 1 year ago

@afeefghannam89 the original code is from you. So ping :-)

widhalmt commented 1 year ago

@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.

danopt commented 1 year ago

@widhalmt

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.

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.

So the first one seems to be a bit more reasonable to me.

I agree, that's absolutely fine to save the a salted hash to a file. It would be saved in a file anyway.

widhalmt commented 1 year ago

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.

afeefghannam89 commented 1 year ago

@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 Changing this default may cause braking somewhere in elastic. So I would stay using bcrypt and not change it to SHA-256. I think they prefer bcrypt over SHA-256 for the following reason:

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.

afeefghannam89 commented 1 year ago

If you use SHA-256 without changing the default elasticsearch password hashing algorithm bcrypt you will get the same error which Lucinda had here https://github.com/NETWAYS/ansible-collection-elasticstack/pull/186#issuecomment-1614385105

widhalmt commented 1 year ago

Thanks for the hint. I totally lost that. I'll change it right away.

danopt commented 1 year ago

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.

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.