Skip to content

Conversation

@DanScharon
Copy link

replace password with unix socket authentication for root user. Make database_root_password optional and use unix_socket as default auth plugin. Ensure that the tasks run for both Debian and RedHat based distros

… database_root_password optional and uses unix_socket as default auth plugin. Ensures that the tasks run for both Debian and RedHat based distros
@DanScharon
Copy link
Author

follow-up of #4

Copy link
Contributor

@wsmirnow wsmirnow left a comment

Choose a reason for hiding this comment

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

Hi @DanScharon, I'm sorry this review took a while.

I'm not sure of

  • enabling unix socket authentication plugin for root user will also
    • disable password based login for root
    • only unix root user is allowed to run administrative tasks on DB server
  • if this role should really take care about restricting access for root user,
    • yes we did it in the previous version but is it the way to go?
    • Maybe this feature should be configurable?
      • what should the configuration looks like? Root Unix Auth: [true | false]?

What du you think about?


- name: Wait for MariaDB start
ansible.builtin.wait_for:
path: "{{ '/run/mysqld/mysqld.sock' if ansible_os_family == 'Debian' else '/var/lib/mysql/mysql.sock' }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this to be consistent

Suggested change
path: "{{ mariadb_socket_path }}"

host: "%"
priv: "{{ database_name }}.*:ALL,GRANT"
login_user: "{{ database_root_user }}"
login_password: "{{ database_root_password }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Password may be unset. This would handle this case.

Suggested change
login_password: "{{ database_root_password }}"
login_password: "{{ database_root_password | default(omit) }}"

encoding: utf8mb4
state: present
login_user: "{{ database_root_user }}"
login_password: "{{ database_root_password }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Password may be unset. This would handle this case.

Suggested change
login_password: "{{ database_root_password }}"
login_password: "{{ database_root_password | default(omit) }}"

host_all: true
state: absent
login_user: "{{ database_root_user }}"
login_password: "{{ database_root_password }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Password may be unset. This would handle this case.

Suggested change
login_password: "{{ database_root_password }}"
login_password: "{{ database_root_password | default(omit) }}"

Comment on lines 98 to +111
- name: Ensure root user can only connect from the local machine
community.mysql.mysql_user:
user: "{{ database_root_user }}"
password: "{{ database_root_password }}"
host: "{{ item }}"
community.mysql.mysql_query:
query:
- DELETE FROM mysql.user WHERE User='root' AND Host NOT IN ('localhost', '127.0.0.1', '::1')
login_user: "{{ database_root_user }}"
login_password: "{{ database_root_password }}"
loop:
- "::1"
- "localhost"
- "{{ ansible_fqdn }}"
no_log: true
login_unix_socket: "{{ mariadb_socket_path }}"

- name: Ensure unix socket as login method for root user
community.mysql.mysql_user:
name: "{{ database_root_user }}"
plugin: unix_socket
check_implicit_admin: true
login_unix_socket: "{{ mariadb_socket_path }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Enabling unix socket plugin for root user authentication will disallow password based authentication (see docs). IMO there is no need to run this query. Isn't it?

Suggested change
- name: Ensure root user can only connect from the local machine
community.mysql.mysql_user:
user: "{{ database_root_user }}"
password: "{{ database_root_password }}"
host: "{{ item }}"
community.mysql.mysql_query:
query:
- DELETE FROM mysql.user WHERE User='root' AND Host NOT IN ('localhost', '127.0.0.1', '::1')
login_user: "{{ database_root_user }}"
login_password: "{{ database_root_password }}"
loop:
- "::1"
- "localhost"
- "{{ ansible_fqdn }}"
no_log: true
login_unix_socket: "{{ mariadb_socket_path }}"
- name: Ensure unix socket as login method for root user
community.mysql.mysql_user:
name: "{{ database_root_user }}"
plugin: unix_socket
check_implicit_admin: true
login_unix_socket: "{{ mariadb_socket_path }}"
- name: Ensure unix socket as login method for root user
community.mysql.mysql_user:
name: "{{ database_root_user }}"
plugin: unix_socket
check_implicit_admin: true
login_unix_socket: "{{ mariadb_socket_path }}"

Comment on lines +8 to +10
# mariadb root user password,
# only used for authentication to a db with an already set root password and disabled unix_socket authentication
database_root_password: '4567'
Copy link
Contributor

Choose a reason for hiding this comment

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

Stop setting default passwords, especially unsecure one!

Suggested change
# mariadb root user password,
# only used for authentication to a db with an already set root password and disabled unix_socket authentication
database_root_password: '4567'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants