From 6e12dcfd740e93736711213fd3cd580517d5b084 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 4 Dec 2018 16:02:44 +0100 Subject: [PATCH 1/5] security hardening --- src/lib/bootloader/boot_record_backup.rb | 7 ++++--- src/lib/bootloader/grub2pwd.rb | 12 ++++-------- src/modules/BootSupportCheck.rb | 2 +- test/grub2pwd_test.rb | 10 +++------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/lib/bootloader/boot_record_backup.rb b/src/lib/bootloader/boot_record_backup.rb index 5dfd7e203..49ecbf682 100644 --- a/src/lib/bootloader/boot_record_backup.rb +++ b/src/lib/bootloader/boot_record_backup.rb @@ -1,5 +1,6 @@ require "yast" require "date" +require "shellwords" module Bootloader # Responsibility of class is to manage backup of MBR, respective PBR of disk, @@ -35,7 +36,7 @@ def initialize(device) # Backup is stored in /var/lib/YaST2/backup_boot_sectors, in logs # directory and if it is MBR of primary disk, then also in /boot/backup_mbr def write - Yast::SCR.Execute(BASH_PATH, "mkdir -p #{MAIN_BACKUP_DIR}") + Yast::SCR.Execute(BASH_PATH, "/usr/sbin/mkdir -p #{MAIN_BACKUP_DIR.shellescape}") if exists? rotate @@ -88,7 +89,7 @@ def formated_file_ctime(filename) def copy_br(device, target_path, bs: 512) Yast::SCR.Execute( BASH_PATH, - "/bin/dd if=#{device} of=#{target_path} bs=#{bs} count=1 2>&1" + "/bin/dd if=#{device.shellescape} of=#{target_path.shellescape} bs=#{bs.to_s.shellescape} count=1 2>&1" ) end @@ -114,7 +115,7 @@ def rotate Yast::SCR.Execute( BASH_PATH, format("/bin/mv %{path} %{path}-%{date}", - path: device_file_path, date: change_date) + path: device_file_path.shellescape, date: change_date.shellescape) ) end end diff --git a/src/lib/bootloader/grub2pwd.rb b/src/lib/bootloader/grub2pwd.rb index fd6b09f95..803b88694 100644 --- a/src/lib/bootloader/grub2pwd.rb +++ b/src/lib/bootloader/grub2pwd.rb @@ -1,4 +1,5 @@ require "yast" +require "shellwords" Yast.import "Stage" @@ -120,21 +121,16 @@ def disable return unless used_on_target? # operate on target as we have to remove password during installation from target grub2 - Yast::SCR.Execute(Yast::Path.new(".target.bash"), "rm '#{PWD_ENCRYPTION_FILE}'") + Yast::SCR.Execute(Yast::Path.new(".target.bash"), "rm '#{PWD_ENCRYPTION_FILE.shellescape}'") end def encrypt(password) Yast.import "String" quoted_password = Yast::String.Quote(password) - result = Yast::WFM.Execute(YAST_BASH_PATH, - "echo '#{quoted_password}\n#{quoted_password}\n' | LANG=C grub2-mkpasswd-pbkdf2") + result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2", env: { "LANG" => "C"}, stdin: "test\ntest\n", stdout: :capture) - if result["exit"] != 0 - raise "Failed to create encrypted password for grub2. Command output: #{result["stderr"]}" - end - - pwd_line = result["stdout"].split("\n").grep(/password is/).first + pwd_line = result.split("\n").grep(/password is/).first if !pwd_line raise "grub2-mkpasswd output do not contain encrypted password. Output: #{result["stdout"]}" end diff --git a/src/modules/BootSupportCheck.rb b/src/modules/BootSupportCheck.rb index 225dcf7f0..449d11bde 100644 --- a/src/modules/BootSupportCheck.rb +++ b/src/modules/BootSupportCheck.rb @@ -125,7 +125,7 @@ def check_gpt_reserved_partition # Check if EFI is needed def efi? - cmd = "modprobe efivars 2>/dev/null" + cmd = "/usr/sbin/modprobe efivars 2>/dev/null" SCR.Execute(path(".target.bash_output"), cmd) FileUtils.Exists("/sys/firmware/efi/systab") end diff --git a/test/grub2pwd_test.rb b/test/grub2pwd_test.rb index dd344be1f..5001bd501 100644 --- a/test/grub2pwd_test.rb +++ b/test/grub2pwd_test.rb @@ -214,13 +214,9 @@ def mock_file_presence(exists) PBKDF2 hash of your password is #{ENCRYPTED_PASSWORD} EOF - expect(Yast::WFM).to receive(:Execute) - .with(kind_of(Yast::Path), /grub2-mkpasswd/) - .and_return( - "exit" => 0, - "stderr" => "", - "stdout" => success_stdout - ) + expect(Yast::Execute).to receive(:locally) + .with(/grub2-mkpasswd/, anything) + .and_return(success_stdout) subject.password = "really strong password" expect(subject.instance_variable_get(:@encrypted_password)).to eq ENCRYPTED_PASSWORD From b61d70c75b53564265bab81405497d9008d60829 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 4 Dec 2018 16:20:12 +0100 Subject: [PATCH 2/5] changes --- package/yast2-bootloader.changes | 9 +++++++++ package/yast2-bootloader.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/package/yast2-bootloader.changes b/package/yast2-bootloader.changes index 4310aa2f4..fa5b674a1 100644 --- a/package/yast2-bootloader.changes +++ b/package/yast2-bootloader.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Tue Dec 4 15:14:47 UTC 2018 - jreidinger@suse.com + +- always use absolute path to binaries (bsc#1118291) +- escape properly shell arguments (bsc#1118291) +- do not show grub2 password in list of processes when encrypting + (bsc#1118291) +- 4.1.13 + ------------------------------------------------------------------- Sat Nov 24 00:39:54 UTC 2018 - Stasiek Michalski diff --git a/package/yast2-bootloader.spec b/package/yast2-bootloader.spec index a44b238fb..48cd7495f 100644 --- a/package/yast2-bootloader.spec +++ b/package/yast2-bootloader.spec @@ -17,7 +17,7 @@ Name: yast2-bootloader -Version: 4.1.12 +Version: 4.1.13 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From f8de939ec82ab3888ebb9c14461c84b5b1efff93 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 4 Dec 2018 16:21:48 +0100 Subject: [PATCH 3/5] fix accidental testing data --- src/lib/bootloader/grub2pwd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/bootloader/grub2pwd.rb b/src/lib/bootloader/grub2pwd.rb index 803b88694..85e65cc98 100644 --- a/src/lib/bootloader/grub2pwd.rb +++ b/src/lib/bootloader/grub2pwd.rb @@ -128,7 +128,7 @@ def encrypt(password) Yast.import "String" quoted_password = Yast::String.Quote(password) - result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2", env: { "LANG" => "C"}, stdin: "test\ntest\n", stdout: :capture) + result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2", env: { "LANG" => "C"}, stdin: "#{password}\n#{password}\n", stdout: :capture) pwd_line = result.split("\n").grep(/password is/).first if !pwd_line From e7d94881e24280fe40169d82fade8743fc9f35f5 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 4 Dec 2018 16:41:08 +0100 Subject: [PATCH 4/5] make rubocop happy --- src/lib/bootloader/boot_record_backup.rb | 3 ++- src/lib/bootloader/grub2pwd.rb | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/lib/bootloader/boot_record_backup.rb b/src/lib/bootloader/boot_record_backup.rb index 49ecbf682..0f4981be2 100644 --- a/src/lib/bootloader/boot_record_backup.rb +++ b/src/lib/bootloader/boot_record_backup.rb @@ -89,7 +89,8 @@ def formated_file_ctime(filename) def copy_br(device, target_path, bs: 512) Yast::SCR.Execute( BASH_PATH, - "/bin/dd if=#{device.shellescape} of=#{target_path.shellescape} bs=#{bs.to_s.shellescape} count=1 2>&1" + "/bin/dd if=#{device.shellescape} of=#{target_path.shellescape} " \ + "bs=#{bs.to_s.shellescape} count=1 2>&1" ) end diff --git a/src/lib/bootloader/grub2pwd.rb b/src/lib/bootloader/grub2pwd.rb index 85e65cc98..23f817780 100644 --- a/src/lib/bootloader/grub2pwd.rb +++ b/src/lib/bootloader/grub2pwd.rb @@ -125,10 +125,10 @@ def disable end def encrypt(password) - Yast.import "String" - - quoted_password = Yast::String.Quote(password) - result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2", env: { "LANG" => "C"}, stdin: "#{password}\n#{password}\n", stdout: :capture) + result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2", + env: { "LANG" => "C" }, + stdin: "#{password}\n#{password}\n", + stdout: :capture) pwd_line = result.split("\n").grep(/password is/).first if !pwd_line From 62ef4eaa314ee36de30b4ba04d387e63cd2dc01c Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 4 Dec 2018 16:53:31 +0100 Subject: [PATCH 5/5] fix using output --- src/lib/bootloader/grub2pwd.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/bootloader/grub2pwd.rb b/src/lib/bootloader/grub2pwd.rb index 23f817780..d65171d8e 100644 --- a/src/lib/bootloader/grub2pwd.rb +++ b/src/lib/bootloader/grub2pwd.rb @@ -132,12 +132,12 @@ def encrypt(password) pwd_line = result.split("\n").grep(/password is/).first if !pwd_line - raise "grub2-mkpasswd output do not contain encrypted password. Output: #{result["stdout"]}" + raise "grub2-mkpasswd output do not contain encrypted password. Output: #{result}" end ret = pwd_line[/^.*password is\s*(\S+)/, 1] if !ret - raise "grub2-mkpasswd output do not contain encrypted password. Output: #{result["stdout"]}" + raise "grub2-mkpasswd output do not contain encrypted password. Output: #{result}" end ret