Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[secure boot] Support rw files allowlist #4585

Merged
merged 9 commits into from
Jun 13, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build_image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ elif [ "$IMAGE_TYPE" = "aboot" ]; then
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_paths.conf; popd
echo "$IMAGE_VERSION" >> .imagehash
zip -g $OUTPUT_ABOOT_IMAGE .imagehash
zip -g $ABOOT_BOOT_IMAGE .imagehash
Expand Down
3 changes: 3 additions & 0 deletions files/Aboot/boot0.j2
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,9 @@ write_boot_configs() {
fi
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=y" >> /tmp/append
Copy link
Contributor

Choose a reason for hiding this comment

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

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


mkdir -p "$image_path"
cat /tmp/append > $cmdline_image
[ -s ${target_path}/machine.conf ] || write_machine_config
Expand Down
28 changes: 28 additions & 0 deletions files/image_config/secureboot/whitelist_paths.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
home/.*
var/core/.*
var/log/.*
etc/group
etc/gshadow
etc/hostname
etc/hosts
etc/machine-id
etc/network/interfaces
etc/nsswitch.conf
etc/pam.d/common-auth-sonic
etc/pam.d/sshd
etc/pam.d/login
etc/passwd
etc/rsyslog.conf
etc/shadow
etc/sonic/acl.json
etc/sonic/config_db.json
etc/sonic/minigraph.xml
etc/sonic/snmp.yml
etc/sonic/updategraph.conf
etc/ssh/ssh_host_rsa_key.pub
etc/ssh/ssh_host_rsa_key
etc/subgid
etc/subuid
etc/tacplus_nss.conf
etc/tacplus_user
lib/systemd/system/serial-getty@.service
yxieca marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 11 additions & 0 deletions files/image_config/secureboot/whitelist_paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Configuration Guide
It is the patterns of the relative paths in /host/image-{{hash}}/rw folder.
The patterns will not be used if the Sonic Secure Boot feature is not enabled.
The files that are not in the whitelist will be removed when the Sonic System cold reboot.

### Example to whitelist all the files in a folder
home/.*

### Example to whitelist a file
etc/nsswitch.conf

28 changes: 28 additions & 0 deletions files/initramfs-tools/union-mount.j2
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,38 @@ set_tmpfs_log_partition_size()
[ $maxsize -le $varlogsize ] && varlogsize=$maxsize
}

whitelist_rw_folder()
Copy link
Collaborator

@qiluo-msft qiluo-msft May 13, 2020

Choose a reason for hiding this comment

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

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

{
image_dir=$1
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

whitelist_file=${rootmnt}/host/$image_dir/whitelist_paths.conf

# Return if the whitelist file does not exist
if ! test -f "${whitelist_file}"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

return
fi

# Return if the secure_boot_enable option is not set
if ! (cat /proc/cmdline | grep -i -q "secure_boot_enable=[y1]"); then
return
fi

rw_dir=${rootmnt}/host/$image_dir/rw

# 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
Copy link
Collaborator

@qiluo-msft qiluo-msft May 26, 2020

Choose a reason for hiding this comment

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

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.


# Find the files in the rw folder, and remove the files not in the whitelist
find ${rw_dir} -type f | grep -v -f $whitelist_pattern_file | xargs /bin/rm -f
rm -f $whitelist_pattern_file
}

## Mount the overlay file system: rw layer over squashfs
image_dir=$(cat /proc/cmdline | sed -e 's/.*loop=\(\S*\)\/.*/\1/')
mkdir -p ${rootmnt}/host/$image_dir/rw
mkdir -p ${rootmnt}/host/$image_dir/work
## Whitelist rw folder
whitelist_rw_folder "$image_dir"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to remove_not_whitelist_files

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 2, 2020

Choose a reason for hiding this comment

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

$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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

mount -n -o lowerdir=${rootmnt},upperdir=${rootmnt}/host/$image_dir/rw,workdir=${rootmnt}/host/$image_dir/work -t overlay root-overlay ${rootmnt}
## Check if the root block device is still there
[ -b ${ROOT} ] || mdev -s
Expand Down