[PATCH phosphor-host-ipmid] Support for restricted mode for IPMI commands
Jeremy Kerr
jk at ozlabs.org
Thu Apr 21 20:08:31 AEST 2016
Hi Tom,
> From: tomjose <tomjoseph at in.ibm.com>
Can you use a full name for the commit, and add some detail to the
commit log?
Also, you may want to check out our contributing guidelines:
https://github.com/openbmc/docs/blob/master/contributing.md
> @@ -88,6 +89,12 @@ ipmi_ret_t ipmi_app_set_acpi_power_state(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
> *data_len = 0;
>
> printf("IPMI SET ACPI STATE Ignoring for now\n");
> +
> + if(restricted_mode)
> + {
> + return IPMI_CC_INSUFFICIENT_PRIVILEGE;
> + }
> +
> return rc;
> }
I don't think this is a maintainable method of implementing restricted
mode, for a couple of reasons:
- The checks are scattered throughout the codebase. We would have to
audit every ipmi function to check that restricted mode is
implemented properly.
- If we add new commands, we have no way to ensure the whitelist is
implemented correctly for that command.
We'd be better off implementing the check at a single location, where
the IPMI command is first demultiplexed. This way, we can audit it in a
central location, and have a single list of whitelisted commands.
I'd suggest we have a function, looking something like:
struct {
ipmi_netfn_t netfn;
ipmi_cmd_t cmd;
} ipmi_whitelist[] = {
{ NETFUN_CHASSIS, IPMI_CMD_CHASSIS_CONTROL },
....
}
bool ipmi_command_is_allowed(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
void *data)
{
int i;
if (!restricted_mode)
return true;
for (i = 0; i < ARRAY_SIZE(ipmi_whitelist); i++) {
if (netfn == ipmi_whitelist[i].netfn &&
cmd == ipmi_whitelist[i].cmd)
return true;
}
return false;
}
this would be called from handle_ipmi_command, before we route it to a
handler; if it returns false there, the we return
IPMI_CC_INSUFFICIENT_PRIVILEGE immediately.
This way, we have a whitelist rather than a blacklist, and we can audit
it much more easily.
Regards,
Jeremy
More information about the openbmc
mailing list