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

set a random password for keepalived #263

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

simonfelding
Copy link

Description

set a random password for keepalived instead of having the same pre-shared key.
also switched to authentication headers instead of the password, as it is more secure that way.
set the folder permissions so you can't just read the password with OS access.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Small minor change not affecting the Ansible Role code (GitHub Actions Workflow, Documentation etc.)

How Has This Been Tested?

@MonolithProjects MonolithProjects added the enhancement New feature or request label Oct 26, 2024
@MonolithProjects MonolithProjects self-assigned this Oct 26, 2024
@@ -4,3 +4,4 @@
rke2_method: tar
rke2_service_name: "rke2-{{ rke2_type }}.service"
rke2_restart_needed: false
keepalived_secret: "{{ lookup('community.general.random_string', length=80, base64=True) }}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause that all keepalived nodes will have different password

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if there is a way to cache the value or something so it's the same for all hosts? I guess it could be set as a fact first and then that fact could be used later?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically you could use local facts for storing it. But then you would need solve also things like to have this secret idempotent but still have an option to change it, use it also for new node joining the keepalived cluster... etc. The solution would have to be too much programmatic i guess... And Ansible is not an programming language :-) I think the value should be left up to user.

@@ -50,8 +50,8 @@ vrrp_instance VI_1 {
{% endfor %}
}
authentication {
auth_type PASS
auth_pass rke2servers
auth_type AH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be configurable with PASS as the default. Also in case of AH, it would be good to have an additional option for auth_algo.

Copy link
Author

@simonfelding simonfelding Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no expert here, I just thought the AH algo was better than the PASS algo because the AH algo isn't as sniffable. Why would you want to use the PASS algo?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there are users which are already using this Ansible Role and by changing keepalived configuration the role may cause service interruption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants