-
Notifications
You must be signed in to change notification settings - Fork 89
Added code to enable Dynamic Key Guest Secure Boot #925
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
base: master
Are you sure you want to change the base?
Conversation
Dynamic Key Guest Secure Boot needs kernel config check, setting platform keys, checking signatures of GRUB and Kernel, added code to enable and disable DKGSB. Signed-off-by: Krishan Gopal Saraswat <[email protected]>
PraveenPenguin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Krishan-Saraswat i have just glance .. i see we have scope of reusability of code .. let me know your thought
| except Exception as e: | ||
| error_msg = f"Failed to execute HMC command on HMC '{command}': {str(e)}" | ||
| log.error(error_msg) | ||
| self.fail(error_msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we achive this without the helper function itself? Or can you please help to understand how it helps? Exception helpful
| "HMC object is not available (self.cv_HMC is None). " | ||
| ) | ||
| log.error(msg) | ||
| self.fail(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better, we should move this to the HMC class .. and this can be implicit check before any hmc cmd call
| """ | ||
| self.ensure_hmc_available() | ||
| command = f"lssyscfg -r lpar -m {system_name} --filter \"lpar_names={host_name}\" | sed s/,/\\\n/g | grep \"secure_boot\\|keystore\"" | ||
| return self.execute_hmc_command(command) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be a utils funciton
| self.execute_hmc_command(command) | ||
| time.sleep(sleep_time) | ||
|
|
||
| def configure_secure_boot(self, host_name, system_name, config_params): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems this is part of hmc class. Can you please check once ...
| # Reset secure boot | ||
| self.reset_secure_boot(host_name, system_name) | ||
| # Disable secure boot | ||
| self.disable_secure_boot(host_name, system_name) No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this end line
| if auth_dir: | ||
| log.info(f"Found authfiles at {auth_dir}; running secvarctl write PK reset_PK_by_PK.auth") | ||
| try: | ||
| cmd = f"bash -c 'cd {shlex.quote(auth_dir)} && secvarctl write PK reset_PK_by_PK.auth'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a new module shlex is required to just quote a normal string. If you have a execute quote and security specific then this makes sense.
| error_msg = f"Failed to disable secure boot: {str(e)}" | ||
| log.error(error_msg) | ||
| self.fail(error_msg) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For enable and disable secure boot why can't we use the functions from static guest secure boot? Have you explored that option?
Dynamic Key Guest Secure Boot needs kernel config check, setting platform keys, checking signatures of GRUB and Kernel, added code to enable and disable DKGSB.