Skip to content
This repository has been archived by the owner on Nov 17, 2020. It is now read-only.

alt version initial commit #15

Merged
merged 17 commits into from
Jun 10, 2016
Merged

alt version initial commit #15

merged 17 commits into from
Jun 10, 2016

Conversation

fitz123
Copy link
Contributor

@fitz123 fitz123 commented May 21, 2016

No description provided.

mysql_datadir: '/var/lib/mysql'
mysql_hardening_hardening_conf: '/etc/mysql/conf.d/hardening.cnf'
mysql_root_password: "{{ lookup('env','mysql_root_password') }}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about it, but don't know more secure way to pass values like that

Copy link
Member

Choose a reason for hiding this comment

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

We could use a separate variables-file that only contains the password and encrypt this file with vault. Or something along these lines.

@fitz123
Copy link
Contributor Author

fitz123 commented May 21, 2016

Checks are failed because of "python-mysqldb" package installation and probably because of requirement of env variable

@@ -0,0 +1,18 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

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

everything here is copy-paste from old main.yml excluding last part with sql importing

@rndmh3ro
Copy link
Member

Thanks for these improvements. I'll test them locally this weekend and see if anything fails.

@fitz123
Copy link
Contributor Author

fitz123 commented May 21, 2016

I've tested it on one on my mariadb instances and at first view - it looks fine. Gonna test on galera cluster soon and fix if something will fail. So basically "mysql_secure_installation.yml" I've been used quite a long time already and never had issues with it, but "mysql_hardening_options" dict includes a lot of variables I've never used and not sure how it can affect servers, so gonna test them as well, but slowly

@fitz123
Copy link
Contributor Author

fitz123 commented May 21, 2016

And it'd be great if you can find a way to fix travis checks)

@rndmh3ro
Copy link
Member

And it'd be great if you can find a way to fix travis checks)

The problem is that the package-module is only supported from ansible 2.0 on. So we're going to have to stick to yum and apt tasks for now. See here: https://github.com/dev-sec/ansible-os-hardening/blob/master/tasks/pam.yml#L9-L15 on how to do it.

@@ -1,4 +1,4 @@
mysql_hardening_enabled: yes
mysql_hardening_enabled: no
Copy link
Member

Choose a reason for hiding this comment

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

I think disabling the role by default is counter-intuitive.
If I would use this role, I'd expect that it would work out-of-the-box (like os and ssh hardening), not that I should still have to activate it. Because that's already what I'm doing when I'm including this role into one of my playbooks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Up to you, but usually roles are deactivated by default to not accidentally fail builds. For os and ssh that's different, probably because you can successfully execute it on almost build, but mysql have to be executed on mysql-enabled instances only. My use case: I'm using 1 playbook for almost everything, and having group-specific variables that control which roles are enabled (except roles like os/ssh-hardening which can be safelly enabled for all (at least my) instances, so they're enabled by default, though it's an exclusion). We can do the opposite, if you still want. though I think it's better not to leave it disabled and force user to manually activate it

@rndmh3ro
Copy link
Member

I don't understand the distinction between configure.yml and mysql_secure_installation.yml. Why are these both separated?

And why does mysql_secure_installation.yml only runs, when mysql_root_password != ''?
The secure installation should run even if the root-password is set. For example when some admin set a root-password but did not delete the test-database. Or when he forgot to remove the test users?

when: ansible_distribution == 'Debian' or ansible_distribution == 'Ubuntu'

- name: Install python-mysqldb for Ansible
yum: name=python-mysqldb state=present
Copy link
Member

Choose a reason for hiding this comment

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

for rhel that's MySQL-python.

@rndmh3ro
Copy link
Member

I tested the role successfully on a fresh centos 6.5 system and it works great!
I'm going to test it on ubuntu, debian and oracle systems next week.
If this is sucsessful and we can work on the remaining comments, this could be done by next week.

@rndmh3ro
Copy link
Member

I'll update the travis file, too, because right now there isn't much testing going on at all.

@fitz123
Copy link
Contributor Author

fitz123 commented May 22, 2016

Could you also test on ubuntu, as far as I know for apt module to work "python-apt" package is required. I have it installed in all templates since I start using ansible, so not super easy to test)

@fitz123
Copy link
Contributor Author

fitz123 commented May 22, 2016

I don't understand the distinction between configure.yml and mysql_secure_installation.yml. Why are these both separated?

I separated them mostly logically/functionally, configure.yml responses for configuration hardening, and mysql_secure_installation.yml almost copies mysql-secure-installation script's functionality (https://dev.mysql.com/doc/refman/5.7/en/mysql-secure-installation.html)

And why does mysql_secure_installation.yml only runs, when mysql_root_password != ''?
The secure installation should run even if the root-password is set. For example when some admin set a root-password but did not delete the test-database. Or when he forgot to remove the test users?

It does (at least I guess it's) opposite, it doesn't run if you didn't pass root password as a parameter, because mysql_db module have to have credentials for "import" to work, ether passed as parameters or in .my.cnf.
But you have a point, if one doesn't want to set root password at all. So we need to have such an option into defaults, and check somehow if real root password is '', and user don't want to set it, just run the rest of tasks. Currently I'm assuming that setting root password is mandatory

@@ -1,10 +1,13 @@
mysql_hardening_enabled: no
Copy link
Member

Choose a reason for hiding this comment

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

We should set this to "yes", because my expectation is that a role just works when I include it into my playbook. Also I've never seen another role you have to "enable" with an additional variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@rndmh3ro
Copy link
Member

rndmh3ro commented May 29, 2016

Currently I'm assuming that setting root password is mandatory

Yes and we should force this, because if no password is set, than you have a huge gaping security hole, no matter if the installation is secured otherwise.

What I'd like to see:

  • make the role just work without having to set another variable
  • make the root password mandatory
  • please rebase against master
  • make a variable for mysql-user home, so the .my.cnf is in the correct location.

@fitz123
Copy link
Contributor Author

fitz123 commented May 31, 2016

@rndmh3ro,
Sorry, but I not completely get the idea of 1st 2 points. It sounds controversial for me.
Currently I set default mysql root password and show notification during execution about it

I can change behaviour to ether:

  • do not set root password (comment variable out), make variable 'mandatory' and fail if variable is not defined. It means role will fail by default. Doesn't sound good
  • do not set root password and do not fail. Show notification about it, try run the rest of tasks without root password (probably user already has .my.cnf, or want to keep empty password). This is better way to go than previous approach, but I'd prefer to stick to current approach (setting default password) if there're no arguments against

@fitz123
Copy link
Contributor Author

fitz123 commented Jun 5, 2016

any other comments?

@rndmh3ro
Copy link
Member

rndmh3ro commented Jun 5, 2016

Everything's fine now!
I'm in the process of testing the role right now, could take some time though.

@rndmh3ro
Copy link
Member

So I tested the role:

  • Ubuntu 12.04
  • Ubuntu 14.04
  • Ubuntu 16.04
  • Oracle 6
  • Centos 6
  • Centos 7
  • Debian 7
  • Debian 8

There is a minor issue with RedHat that I will fix after merging this.
Also Oracle is a pain in the ass to test.

@rndmh3ro rndmh3ro merged commit 6627aa6 into dev-sec:master Jun 10, 2016
rndmh3ro pushed a commit that referenced this pull request Jun 16, 2016
alt version initial commit
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants