[secure boot] Support rw files allowlist by xumia · Pull Request #4585 · sonic-net/sonic-buildimage

xumia · 2020-05-13T00:54:01Z

- Why I did it
The files without signature validation may be tempered when the switch is turned off, it is required to remove the files in the rw folder which will be executed when system starts for the secure boot. we will remove all the executable files and all the file not in the allowlist config file, only allow limited config files to improve the security.

- How I did it
The allowlist config file "files/image_config/secureboot/allowlist_paths.conf" is to control whether the rw files in root file system will be removed or not when the system reboot. The config file will be extracted from the signed and verified image in each reboot.
The Linux kernel will take the process to remove files in the union-mount just before mounting the rw folder.

- How to verify it

- Description for the changelog
Remove the files in rw folder of the overlay file system based on a allowlist config file.

- A picture of a cute animal (not mandatory but encouraged)

qiluo-msft · 2020-05-13T01:02:59Z

files/Aboot/boot0.j2

    fi

+    # setting secure_boot_enable=true when secure boot enabled
+    [ -f /bin/securebootctl ] && securebootctl secureboot -display | grep -i "Secure Boot enable" -q && echo "secure_boot_enable=true" >> /tmp/append


securebootctl secureboot -display | grep -i "Secure Boot enable" -q [](start = 33, length = 67)

Is it possible to test secure boot enabled without grep? #Pending

I will ask Arista about it.

The arista secure boot using the similar way.

qiluo-msft · 2020-05-13T01:08:08Z

files/image_config/secureboot/whitelist

+etc/subuid
+etc/tacplus_nss.conf
+etc/tacplus_user
+lib/systemd/system/[email protected]


Need to double check #Closed

qiluo-msft · 2020-05-13T01:14:42Z

build_image.sh

    sed -i -e "s/%%IMAGE_VERSION%%/$IMAGE_VERSION/g" files/Aboot/boot0
    pushd files/Aboot && zip -g $OLDPWD/$OUTPUT_ABOOT_IMAGE boot0; popd
    pushd files/Aboot && zip -g $OLDPWD/$ABOOT_BOOT_IMAGE boot0; popd
+    pushd files/image_config/secureboot && zip -g $OLDPWD/$OUTPUT_ABOOT_IMAGE whitelist; popd


whitelist [](start = 78, length = 9)

Name it as whitelist_paths.txt ? #Closed

Can we use whitelist_paths? Do we need to remove the .txt when added into the image package?

Changed to whitelist_paths.conf

qiluo-msft · 2020-05-13T01:19:36Z

files/initramfs-tools/union-mount.j2

+  fi
+
+  # Return if the secure_boot_enable option is not set
+  if cat /proc/cmdline | grep -v -q "secure_boot_enable=true"; then


secure_boot_enable=true [](start = 37, length = 23)

The convention is
secure_boot_enable=[0|1]
or
secure_boot_enable=[n|y]
#Closed

qiluo-msft · 2020-05-13T01:28:05Z

files/initramfs-tools/union-mount.j2

+
+  whitelist_log=${rootmnt}/host/$image_dir/whitelist.log
+  rw_dir=${rootmnt}/host/$image_dir/rw
+  whitelist=$(cat ${rootmnt}/host/$image_dir/whitelist | grep -v "^\s*#" | awk '{$1=$1};1')


| grep -v "^\s*#" | awk '{$1=$1};1' [](start = 54, length = 36)

Is it only for parsing comments? If yes, we may remove the complexity. And add a whiltelist_README.md side-by-side to explain usage. #Closed

Offline discussed, move comment out into separate file. It is not a runtime conf file.

In reply to: 424122684 [](ancestors = 424122684)

qiluo-msft · 2020-05-13T01:31:38Z

files/initramfs-tools/union-mount.j2

+      echo $file >> ${whitelist_log}
+      rm -f $file
+    fi
+  done


I am worried about the performance if rw contains many files and deep directories. Since whitelist is short, can we mv folder to a temp one, and copied back one by one.

Not sure the validation of this solution, is there any metadata / hidden files in rw for overlay?

If it working, no need to add /* for directories in whitelist. #Closed

The find command can get hidden files, what is the metadata?

Yes, the performance is not good enough, but it is not relative to the depth of the directories, it is the grep.

The performance issue should be fixed. Tested on the device, reduce from 100 seconds to 1 seconds for 300 files. And checked the changed files on product, it only has about 700 changed files in max.

qiluo-msft · 2020-05-13T01:35:11Z

files/initramfs-tools/union-mount.j2

  [ $maxsize -le $varlogsize ] && varlogsize=$maxsize
 }

+whitelist_rw_folder()


Sh code is not easy to understand, add some comments? #Closed

jleveque · 2020-05-13T20:06:36Z

FYI, I updated the PR title.

qiluo-msft · 2020-05-26T00:38:56Z

files/initramfs-tools/union-mount.j2

+
+  # Set the grep pattern file
+  whitelist_pattern_file=${rootmnt}/host/$image_dir/whitelist_paths.pattern
+  grep -v "^\s*$" ${whitelist_file} | awk -v rw_dir="$rw_dir" '{print rw_dir"/"$0"$"}' > $whitelist_pattern_file


awk [](start = 38, length = 3)

Can we use awk to handle empty line https://stackoverflow.com/a/11687266/2514803
And remove grep. #Closed

lguohan · 2020-05-31T10:58:12Z

please add descriptions for the pr

files/image_config/secureboot/whitelist_paths.conf

yxieca · 2020-05-31T23:01:16Z

files/initramfs-tools/union-mount.j2


+whitelist_rw_folder()
+{
+  image_dir=$1


If it doesn't bother you too much. can you switch to 4 space indentation? Most of our files are 4 space indentations.

yxieca · 2020-05-31T23:05:13Z

files/initramfs-tools/union-mount.j2

+  whitelist_file=${rootmnt}/host/$image_dir/whitelist_paths.conf
+
+  # Return if the whitelist file does not exist
+  if ! test -f "${whitelist_file}"; then


Should this check be moved down or add as a protection of accessing the file?

If secure boot is enabled and this file is missing, should you at least whitelist rw_dir? I understand that is not enough.

The file should not be missing, it is extracted from the image every time in boot0.
If secure boot enabled, the file should always exist, I will improve it.

daall · 2020-06-01T18:55:06Z

files/Aboot/boot0.j2

+    # setting secure_boot_enable=true when secure boot enabled
+    [ -f /bin/securebootctl ] && securebootctl secureboot -display | grep -i "Secure Boot enable" -q && echo "secure_boot_enable=y" >> /tmp/append


Comment and code does not match, is it supposed to be "secure_boot_enable=true" or "secure_boot_enable=y"?

tahmed-dev · 2020-06-01T20:06:39Z

files/initramfs-tools/union-mount.j2

 mkdir -p ${rootmnt}/host/$image_dir/rw
 mkdir -p ${rootmnt}/host/$image_dir/work
+## Whitelist rw folder
+whitelist_rw_folder "$image_dir"


suggest name change to reflect the fact that files are removed.

Changed to remove_not_whitelist_files

qiluo-msft · 2020-06-02T01:13:53Z

files/initramfs-tools/union-mount.j2

 mkdir -p ${rootmnt}/host/$image_dir/rw
 mkdir -p ${rootmnt}/host/$image_dir/work
+## Whitelist rw folder
+whitelist_rw_folder "$image_dir"


$image_dir [](start = 21, length = 10)

If there is any executable files inside rw, let's chmod a-x to change it to non executables.

However, user's home directory should keep as is. #Closed

qiluo-msft

Add new comment

qiluo-msft · 2020-06-09T01:14:58Z

change code and comment/title to use allowlist #Closed

xumia · 2020-06-09T07:30:14Z

Retest mellanox please

qiluo-msft

Please also check with other reviewers.

xumia · 2020-06-13T02:58:32Z

Retest mellanox please

Support rw files whitelist for Sonic Secure Boot

05c5daa

qiluo-msft reviewed May 13, 2020

View reviewed changes

xumia added 2 commits May 13, 2020 06:17

Improve the performance

09f568a

fix bug

1619d51

jleveque added the Enhancement ➕ label May 13, 2020

jleveque changed the title ~~Support rw files whitelist for Sonic Secure Boot~~ [secure boot] Support rw files whitelist May 13, 2020

Move the config description into a md file

38aef30

qiluo-msft reviewed May 26, 2020

View reviewed changes

Change to use a simple way to remove the blank line

90c05df

qiluo-msft requested review from daall, jleveque, lguohan, tahmed-dev and yxieca May 31, 2020 18:35

yxieca reviewed May 31, 2020

View reviewed changes

files/image_config/secureboot/whitelist_paths.conf Show resolved Hide resolved

yxieca reviewed May 31, 2020

View reviewed changes

daall reviewed Jun 1, 2020

View reviewed changes

tahmed-dev reviewed Jun 1, 2020

View reviewed changes

qiluo-msft reviewed Jun 2, 2020

View reviewed changes

qiluo-msft requested changes Jun 2, 2020

View reviewed changes

Support chmod a-x in rw folder

528953b

Change function name

355e1d0

Change some unnecessary words

4bdaf29

xumia changed the title ~~[secure boot] Support rw files whitelist~~ [secure boot] Support rw files allowlist Jun 9, 2020

Change some unnecessary words

c8f7fde

qiluo-msft approved these changes Jun 13, 2020

View reviewed changes

yxieca approved these changes Jun 13, 2020

View reviewed changes

tahmed-dev approved these changes Jun 13, 2020

View reviewed changes

daall approved these changes Jun 13, 2020

View reviewed changes

qiluo-msft merged commit 76a395c into sonic-net:master Jun 13, 2020

BaluAlluru mentioned this pull request Jun 23, 2020

Permission issues with latest SONiC images #4832

Closed

		# setting secure_boot_enable=true when secure boot enabled
		[ -f /bin/securebootctl ] && securebootctl secureboot -display \| grep -i "Secure Boot enable" -q && echo "secure_boot_enable=y" >> /tmp/append

Conversation

xumia commented May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jleveque commented May 13, 2020

Uh oh!

qiluo-msft May 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan commented May 31, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xumia commented May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 13, 2020 •

edited

Loading

qiluo-msft May 26, 2020 •

edited

Loading

qiluo-msft Jun 2, 2020 •

edited

Loading

qiluo-msft commented Jun 9, 2020 •

edited

Loading