freedomofpress / ansible-role-grsecurity

The documentation and build system for the grsecurity kernel maintained by the Freedom of the Press Foundation for SecureDrop
GNU General Public License v2.0
49 stars 13 forks source link

Fixes support for custom config files in build role #77

Closed conorsch closed 8 years ago

conorsch commented 8 years ago

Simple change: default to None rather than '' for the custom config file var. These changes were introduced in #68, to satisfy #48. Use of the with_first_found looping parameter causes Ansible to misinterpret the empty string default var drastically. Fortunately, the subsequent sanity check in the "Fail if grsecurity options are not enabled" task catches the error and stops execution of the role.

Failure output when using an empty string:

TASK [build-grsec-kernel : Copy baseline grsecurity config template.] **********
task path: /home/conor/freedomofpress/ansible-role-grsecurity/roles/build-grsec-kernel/tasks/configure.yml:2
changed: [grsec-build] => (item=/home/conor/freedomofpress/ansible-role-grsecurity/roles/build-grsec-kernel/tasks) => {"changed": true, "dest": "/home/vagrant/linux/linux-4.7.10/.config/", "item": "/home/conor/freedomofpress/ansible-role-grsecurity/roles/build-grsec-kernel/tasks", "src": "/home/conor/freedomofpress/ansible-role-grsecurity/roles/build-grsec-kernel/tasks"}

As indicated by both the item and src parameters in that output, Ansible tries to copy the entire tasks/ directory from within the role—definitely not what we want! The result on the target host:

vagrant@grsec-build:~/linux/linux-4.7.10$ find -iname '*.yml'
./.config.old/tasks/fetch_linux_kernel_source.yml
./.config.old/tasks/ccache.yml
./.config.old/tasks/main.yml
./.config.old/tasks/gpg_keys.yml
./.config.old/tasks/compile.yml
./.config.old/tasks/prepare_source_directory.yml
./.config.old/tasks/configure.yml
./.config.old/tasks/verify.yml
./.config.old/tasks/packages.yml
./.config.old/tasks/fetch_ubuntu_overlay.yml
./.config.old/tasks/fetch_grsecurity_files.yml

That's so wrong it hurts. By changing the default value to None, rather than an empty string—which Ansible silently expands to the parent directory of the task list—the problem is resolved.

Closes #48 (again).

msheiny commented 8 years ago

Ouchhhhhhhhhhhhhhhhhhhhhhh good find, that is some scary default behavior -- LGTM :+1: to merge