Skip to content

Do not disable vfat by default#96

Merged
artem-sidorenko merged 1 commit intodev-sec:masterfrom
rndmh3ro:patch-1
Jul 16, 2018
Merged

Do not disable vfat by default#96
artem-sidorenko merged 1 commit intodev-sec:masterfrom
rndmh3ro:patch-1

Conversation

@rndmh3ro
Copy link
Copy Markdown
Member

@rndmh3ro rndmh3ro commented Jul 1, 2018

On UEFI-systems the boot-partition is FAT by default (see here).

If we disable vfat, these systems become unbootable. This has already bitten some users using ansible-os-hardening (dev-sec/ansible-collection-hardening#162, dev-sec/ansible-collection-hardening#145).

Therefore I propose we do not check for a disabled vfat filesystem as vfat is often used on newer systems.

@atomic111
Copy link
Copy Markdown
Member

@rndmh3ro i understand it, but can we include a uefi option. if the uefi option is set then te vfat test is disabled. opinion?

@artem-sidorenko
Copy link
Copy Markdown
Member

@rndmh3ro @mcgege @atomic111 do we have an easy and reliable way to detect uefi? We can look for a mountpoint, but I do not think this is really reliable and safe. Maybe we should allow vfat per default indeed....

@rndmh3ro
Copy link
Copy Markdown
Member Author

rndmh3ro commented Jul 2, 2018

Easiest way seems to check if /sys/firmware/efi exists:

https://askubuntu.com/a/162896

On UEFI-systems the boot-partition is FAT by default (see [here](https://wiki.archlinux.org/index.php/Unified_Extensible_Firmware_Interface/System_partition)).

If we disable vfat, these systems become unbootable. This has already bitten some users using ansible-os-hardening (dev-sec/ansible-collection-hardening#162, dev-sec/ansible-collection-hardening#145).

Therefore I propose we do not check for a disabled vfat filesystem, if efi is used on these systems
@rndmh3ro
Copy link
Copy Markdown
Member Author

I updated the control to only run when efi is not present on the system.

Copy link
Copy Markdown
Member

@artem-sidorenko artem-sidorenko left a comment

Choose a reason for hiding this comment

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

@rndmh3ro thanks!

@artem-sidorenko artem-sidorenko merged commit 170bb04 into dev-sec:master Jul 16, 2018
@rndmh3ro rndmh3ro deleted the patch-1 branch August 1, 2018 19:35
Comment thread controls/os_spec.rb
its(:content) { should match 'install vfat /bin/true' }
# if efi is active, do not disable vfat. otherwise the system
# won't boot anymore
unless Dir.exist?('/sys/firmware/efi')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rndmh3ro maybe we have a bug here. Dir.exist? gets executed in the local context and not on the remote system. We should do this check via inspec resources

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Uh okay, I did not know this... I'll have to take a look.

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.

3 participants