[PATCH openbmc] Package whitelist.conf in obmc-phosphor-image

[Patrick Williams]

On Mon, Jun 20, 2016 at 09:10:42AM -0500, OpenBMC Patches wrote:
> From: tomjose <tomjoseph at in.ibm.com>
> 
> IPMI whitelisted commands are added in whitelist.conf
> which would be consumed by ipmid daemon. Only whitelisted
> IPMI commands are executed in restricted mode.
> ---
>  .../recipes-phosphor/host-ipmid/host-ipmid.bb      |  5 ++++-
>  .../host-ipmid/host-ipmid.bbappend                 |  1 +
>  .../host-ipmid/host-ipmid/files/whitelist.conf     | 26 ++++++++++++++++++++++
>  3 files changed, 31 insertions(+), 1 deletion(-)
>  create mode 100644 meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bbappend
>  create mode 100644 meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid/files/whitelist.conf
> 
> diff --git a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb
> index 23728cb..b3dba1a 100644
> --- a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb
> +++ b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb
> @@ -17,7 +17,8 @@ TARGET_CFLAGS   += "-fpic"
>  RDEPENDS_${PN} += "clear-once"
>  RDEPENDS_${PN} += "settings"
>  RDEPENDS_${PN} += "network"
> -SRC_URI += "git://github.com/openbmc/phosphor-host-ipmid"
> +SRC_URI += "git://github.com/openbmc/phosphor-host-ipmid \
> +            file://whitelist.conf"

We need a mechanism to have different files per package that we then
combine together at build time.  'host-ipmid',
'openpower-host-ipmi-oem', and 'ipmi-fru-parser' all of a different
subset of the providers.  If we remove 'openpower-host-ipmi-oem' and
replace it with 'anotherarch-host-ipmi-oem' we likely need a different
set of white-listed commands.

>  
>  do_install() {
>          install -m 0755 -d ${D}${libdir}/host-ipmid
> +        install -m 0755 -d ${D}${datadir}/host-ipmid

If we continue to make this runtime parsing, the directory needs to be
passed into the makefile as part of 'do_compile'.  We should not be
forced to use the directory that is defaulted in the package's makefile.

> diff --git a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bbappend b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bbappend
> new file mode 100644
> index 0000000..b2051b6
> --- /dev/null
> +++ b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bbappend
> @@ -0,0 +1 @@
> +FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}/files:"

We have not taken this approach for other packages.  ${PN} is already
supported as a default SRC_URI location.  Why put the 'files'
subdirectory in this case?  And why have a .bbappend file when the
original package recipe is in the same directory?

> diff --git a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid/files/whitelist.conf b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid/files/whitelist.conf
> new file mode 100644
> index 0000000..20dd429
> --- /dev/null
> +++ b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid/files/whitelist.conf
> @@ -0,0 +1,26 @@
> +<NetFn><Command>

So a non-parsed line becomes a "comment"?

> +0x00:0x02
> +0x00:0x08
> +0x00:0x09
> +0x04:0x2D
> +0x04:0x2F

Would be very useful to have comments on all of these lines.
    0x00:0x02 # Function-Foo : Command-Bar
is much more consumable to humans.

-- 
Patrick Williams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160620/2566f503/attachment.sig>