Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
21 changes: 21 additions & 0 deletions sonic_installer/bootloader/aboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ def _get_image_cmdline(self, image):
with open(os.path.join(image_path, KERNEL_CMDLINE_NAME)) as f:
return f.read()

def _set_image_cmdline(self, image, cmdline):
image_path = self.get_image_path(image)
with open(os.path.join(image_path, KERNEL_CMDLINE_NAME), 'w') as f:
return f.write(cmdline)

def supports_package_migration(self, image):
if is_secureboot():
# NOTE: unsafe until migration can guarantee migration safety
Expand Down Expand Up @@ -205,6 +210,22 @@ def verify_next_image(self):
image_path = os.path.join(self.get_image_path(image), DEFAULT_SWI_IMAGE)
return self._verify_secureboot_image(image_path)

def set_fips(self, image, enable):
if enable:
click.echo('Enabling FIPS...')
fips = "1"
else:
click.echo('Disabling FIPS...')
fips = "0"
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Jul 7, 2022

Choose a reason for hiding this comment

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

Just use "1" if enable else "0" #Closed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thanks.

cmdline = self._get_image_cmdline(image)
cmdline = re.sub(' sonic_fips=[^\s]', '', cmdline) + " sonic_fips=" + fips
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

re.sub

Suggest parsing it instead of manipulate single command string. You may use shlex.split.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is to remove the old sonic-fips option, and then add the new setting, and we do not want to have impact on the other settings.
For example, the options as below:

rw console=tty0     console=ttyS0,  9600n8    quiet intel_idle.max_cstate=0 sonic_fips=0 og_size=4096

After changed, the options with multiple space characters will not be changed

rw console=tty0     console=ttyS0,  9600n8    quiet intel_idle.max_cstate=0 og_size=4096 sonic_fips=1

self._set_image_cmdline(image, cmdline)
click.echo('Done')

def get_fips(self, image):
cmdline = self._get_image_cmdline(image)
return 'sonic_fips=1' in cmdline

def _verify_secureboot_image(self, image_path):
if is_secureboot():
cert = self.getCert(image_path)
Expand Down
8 changes: 8 additions & 0 deletions sonic_installer/bootloader/bootloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ def verify_secureboot_image(self, image_path):
"""verify that the image is secure running image"""
raise NotImplementedError

def set_fips(self, image, enable):
"""set fips"""
raise NotImplementedError

def get_fips(self, image):
"""returns true if fips set"""
raise NotImplementedError

def verify_next_image(self):
"""verify the next image for reboot"""
image = self.get_next_image()
Expand Down
44 changes: 44 additions & 0 deletions sonic_installer/bootloader/grub.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,50 @@ def remove_image(self, image):
run_command('grub-set-default --boot-directory=' + HOST_PATH + ' 0')
click.echo('Image removed')

def get_linux_cmdline(self, image):
cmdline = None
config = open(HOST_PATH + '/grub/grub.cfg', 'r')
old_config = config.read()
menuentry = re.search("menuentry '" + image + "[^}]*}", old_config).group()
config.close()
for line in menuentry.split('\n'):
line = line.strip()
if line.startswith('linux '):
cmdline = line[6:].strip()
break
return cmdline

def set_linux_cmdline(self, image, cmdline):
config = open(HOST_PATH + '/grub/grub.cfg', 'r')
old_config = config.read()
old_menuentry = re.search("menuentry '" + image + "[^}]*}", old_config).group()
config.close()
new_menuentry = old_menuentry
for line in old_menuentry.split('\n'):
line = line.strip()
if line.startswith('linux '):
new_menuentry = old_menuentry.replace(line, "linux " + cmdline)
break
config = open(HOST_PATH + '/grub/grub.cfg', 'w')
config.write(old_config.replace(old_menuentry, new_menuentry))
config.close()

def set_fips(self, image, enable):
if enable:
click.echo('Enabling FIPS...')
fips = "1"
else:
click.echo('Disabling FIPS...')
fips = "0"
cmdline = self.get_linux_cmdline(image)
cmdline = re.sub(' sonic_fips=[^\s]', '', cmdline) + " sonic_fips=" + fips
self.set_linux_cmdline(image, cmdline)
click.echo('Done')

def get_fips(self, image):
cmdline = self.get_linux_cmdline(image)
return 'sonic_fips=1' in cmdline

def platform_in_platforms_asic(self, platform, image_path):
"""
For those images that don't have devices list builtin, 'tar' will have non-zero returncode.
Expand Down
21 changes: 21 additions & 0 deletions sonic_installer/bootloader/uboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import platform
import subprocess
import os
import re

import click

Expand Down Expand Up @@ -81,6 +82,26 @@ def remove_image(self, image):
def verify_image_platform(self, image_path):
return os.path.isfile(image_path)

def set_fips(self, image, enable):
if enable:
click.echo('Enabling FIPS...')
fips = "1"
else:
click.echo('Disabling FIPS...')
fips = "0"
proc = subprocess.Popen("/usr/bin/fw_printenv linuxargs", shell=True, text=True, stdout=subprocess.PIPE)
(out, _) = proc.communicate()
cmdline = out.strip()
cmdline = re.sub('^linuxargs=', '', cmdline)
cmdline = re.sub(' sonic_fips=[^\s]', '', cmdline) + " sonic_fips=" + fips
run_command('/usr/bin/fw_setenv linuxargs ' + cmdline)
click.echo('Done')

def get_fips(self, image):
proc = subprocess.Popen("/usr/bin/fw_printenv linuxargs", shell=True, text=True, stdout=subprocess.PIPE)
(out, _) = proc.communicate()
return 'sonic_fips=1' in out

@classmethod
def detect(cls):
arch = platform.machine()
Expand Down
29 changes: 29 additions & 0 deletions sonic_installer/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,35 @@ def set_next_boot(image):
sys.exit(1)
bootloader.set_next_image(image)

# Set fips for image
@sonic_installer.command('set-fips')
@click.argument('image')
@click.option('--disable-fips', is_flag=True,
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Jul 7, 2022

Choose a reason for hiding this comment

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

disable

prefer positive words, like --enable-fips. And default to true. #Closed

Copy link
Copy Markdown
Collaborator Author

@xumia xumia Jul 7, 2022

Choose a reason for hiding this comment

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

Changed to use --enable-fips/disable-fips.

help="Disable fips")
def set_fips(image):
""" Set fips for the image """
bootloader = get_bootloader()
if image not in bootloader.get_installed_images():
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Jul 7, 2022

Choose a reason for hiding this comment

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

image

Is it convenient to have a default image? #Closed

Copy link
Copy Markdown
Collaborator Author

@xumia xumia Jul 8, 2022

Choose a reason for hiding this comment

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

There is no function to retrieve the default image, if the image is not set, then we change the next image.

echo_and_log('Error: Image does not exist', LOG_ERR)
sys.exit(1)
enable_fips = not disable_fips
bootloader.set_fips(image, enable=enable_fips)
click.echo('Set fips for the image successfully')

# Get fips for image
@sonic_installer.command('get-fips')
@click.argument('image')
def get_fips(image):
""" Get the fips enabled or disabled status for the image """
bootloader = get_bootloader()
if image not in bootloader.get_installed_images():
echo_and_log('Error: Image does not exist', LOG_ERR)
sys.exit(1)
enable = bootloader.get_fips(image)
if enable:
click.echo("Fips is enabled")
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Jul 7, 2022

Choose a reason for hiding this comment

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

Fips

Fips -> FIPS #Closed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thanks.

else:
click.echo("Fips is disabled")

# Uninstall image
@sonic_installer.command('remove')
Expand Down
23 changes: 23 additions & 0 deletions tests/installer_bootloader_aboot_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

# Import test module
import sonic_installer.bootloader.aboot as aboot
import tempfile
import shutil

# Constants
image_dir = f'{aboot.IMAGE_DIR_PREFIX}expeliarmus-{aboot.IMAGE_DIR_PREFIX}abcde'
Expand Down Expand Up @@ -50,3 +52,24 @@ def test_get_next_image(re_search_patch):
# Test convertion image dir to image name
re_search_patch().group = Mock(return_value=image_dir)
assert bootloader.get_next_image() == exp_image

def test_set_fips_aboot():
image = 'test-image'
dirpath = tempfile.mkdtemp()
bootloader = aboot.AbootBootloader()
bootloader.get_image_path = Mock(return_value=dirpath)

# The the default setting
bootloader._set_image_cmdline(image, 'test=1')
assert not bootloader.get_fips(image)

# Test fips enabled
bootloader.set_fips(image, True)
assert bootloader.get_fips(image)

# Test fips disabled
bootloader.set_fips(image, False)
assert not bootloader.get_fips(image)

# Cleanup
shutil.rmtree(dirpath)
26 changes: 26 additions & 0 deletions tests/installer_bootloader_grub_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,29 @@ def test_remove_image(open_patch, run_command_patch, re_search_patch):

args, _ = args_list[0]
assert exp_image_path in args[0]

def test_set_fips_uboot():
cmdline = ""
def mock_get_linux_cmdline(image):
return cmdline

def mock_set_linux_cmdline(image, cmd):
nonlocal cmdline
cmdline = cmd

image = 'test-image'
bootloader = grub.GrubBootloader()

bootloader.get_linux_cmdline = mock_get_linux_cmdline
bootloader.set_linux_cmdline = mock_set_linux_cmdline

# The the default setting
assert not bootloader.get_fips(image)

# Test fips enabled
bootloader.set_fips(image, True)
assert bootloader.get_fips(image)

# Test fips disabled
bootloader.set_fips(image, False)
assert not bootloader.get_fips(image)
36 changes: 36 additions & 0 deletions tests/installer_bootloader_uboot_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
# Import test module
import sonic_installer.bootloader.uboot as uboot

class MockProc():
commandline = "linuxargs="
def communicate():
return commandline, None

def mock_run_command(cmd):
MockProc.commandline = cmd

@patch("sonic_installer.bootloader.uboot.subprocess.call", Mock())
@patch("sonic_installer.bootloader.uboot.run_command")
Expand All @@ -27,3 +34,32 @@ def test_remove_image(run_command_patch):

args, _ = args_list[0]
assert exp_image_path in args[0]

@patch("sonic_installer.bootloader.uboot.subprocess.Popen")
@patch("sonic_installer.bootloader.uboot.run_command")
def test_set_fips_uboot(run_command_patch, popen_patch):
class MockProc():
commandline = "linuxargs"
def communicate(self):
return MockProc.commandline, None

def mock_run_command(cmd):
# Remove leading string "/usr/bin/fw_setenv linuxargs " -- the 29 characters
MockProc.commandline = 'linuxargs=' + cmd[29:]

run_command_patch.side_effect = mock_run_command
popen_patch.return_value = MockProc()

image = 'test-image'
bootloader = uboot.UbootBootloader()

# The the default setting
assert not bootloader.get_fips(image)

# Test fips enabled
bootloader.set_fips(image, True)
assert bootloader.get_fips(image)

# Test fips disabled
bootloader.set_fips(image, False)
assert not bootloader.get_fips(image)