Discussion on IPMI provider libraries

Discussion on IPMI provider libraries

[tomjose]

Hello Brendan,

The discussion is with respect to issue 
https://github.com/openbmc/openbmc/issues/430.

The goal of this issue is to share ipmi provider libraries between 
phosphor-host-ipmid
and phosphor-net-ipmid.

The plan is to provide a framework in net-ipmid to register IPMI 
provider API's.
Every IPMI command has a privilege level which would be compared against 
the session's
privilege level before the command is executed. For supporting IPMI on 
LAN channel,
the privilege for each command need to be assigned.

This is the present signature of the callback command API's.

typedef ipmi_ret_t (*ipmid_callback_t)(ipmi_netfn_t, ipmi_cmd_t, 
ipmi_request_t,
                                        ipmi_response_t, 
ipmi_data_len_t, ipmi_context_t);

I plan to add additional parameter for the command privilege level. I 
heard you had interest
around this area to modify callback API to suit your specific needs.

Let me know your thoughts.

Regards,
Tom

Re: Discussion on IPMI provider libraries

[Brendan Higgins]

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

Sharing the provider libraries makes sense; my first area of concern is the
API; I am currently working on a change to the API (see
https://gerrit.openbmc-project.xyz/#/c/841/ for details); I would prefer
you do not make any changes to the current API, but understand if the need
arises before my change is ready.

Could you elaborate on how you plan on enforcing privilege? Having each
provider check privilege level seems like a leaky abstraction to me; I
think it would make more sense to have privilege managed by the host-ipmid
and the net-ipmid.

As far as the actual details concerning phosphor-net-ipmid: I do not have
strong opinions on the matter as Google has no intention of using IPMI over
LAN at this time, but would welcome discussion on the matter nonetheless.

Cheers

[-- Attachment #2: Type: text/html, Size: 1390 bytes --]

Re: Discussion on IPMI provider libraries

[tomjose]

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]



On Tuesday 08 November 2016 01:55 AM, Brendan Higgins wrote:
> Sharing the provider libraries makes sense; my first area of concern 
> is the API; I am currently working on a change to the API (see 
> https://gerrit.openbmc-project.xyz/#/c/841/ 
> <https://gerrit.openbmc-project.xyz/#/c/841/> for details); I would 
> prefer you do not make any changes to the current API, but understand 
> if the need arises before my change is ready.
 From what i have noticed in the patch, there is support for 
ipmid_callback_t handlers  as it is now. So the change in API is to 
accommodate the OEM group ?
So do you have plans to change the callback signatures for the standard 
commands already implemented in host-ipmid?

>
> Could you elaborate on how you plan on enforcing privilege? Having 
> each provider check privilege level seems like a leaky abstraction to 
> me; I think it would make more sense to have privilege managed by the 
> host-ipmid and the net-ipmid.
Table G - Command Number Assignments and Privilege Levels in the IPMI 
specification gives more details on this.

Each command is assigned a privilege level( Callback, User, Operator, 
Admin) which means that the command can be executed only on a session 
with this privilege or higher.
So if a command needs be executed on net-ipmid path, one of the 
attribute needed for net-ipmid is the command's privilege level.
The privilege provided by each command is a registration parameter and 
it is consumed only by net-ipmid.

As part of the same issue, i am separating commands that need to be 
executed from system interface as a separate library.

The provider libraries is now copied into /usr/lib/host-ipmid. The plan 
is to have the /usr/lib/ipmid-providers as the default install location 
for all providers
and then symlink into /usr/lib/host-ipmid and /usr/lib/net-ipmid 
depending on whether the provider library is needed in out-of-band or 
in-band path.
>
> As far as the actual details concerning phosphor-net-ipmid: I do not 
> have strong opinions on the matter as Google has no intention of using 
> IPMI over LAN at this time, but would welcome discussion on the matter 
> nonetheless.
>
> Cheers


[-- Attachment #2: Type: text/html, Size: 3881 bytes --]

Re: Discussion on IPMI provider libraries

[Brendan Higgins]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

>
> From what i have noticed in the patch, there is support for
> ipmid_callback_t handlers  as it is now. So the change in API is to
> accommodate the OEM group ?
> So do you have plans to change the callback signatures for the standard
> commands already implemented in host-ipmid?


The support for ipmid_callback_t is only for backwards compatibility. Part
of the change in the API was to accommodate OEM/Group extensions, yes;
however, the intention is also to provide a cleaner API in general.

The privilege provided by each command is a registration parameter and it
> is consumed only by net-ipmid.


That's fine, but in that case it should not go in the callback; it should
be maintained and enforced by net-ipmid when it looks up a handler.

Cheers

[-- Attachment #2: Type: text/html, Size: 1344 bytes --]

Re: Discussion on IPMI provider libraries

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

On Mon, Nov 14, 2016 at 03:59:08PM -0800, Brendan Higgins wrote:
> > The privilege provided by each command is a registration parameter and it
> > is consumed only by net-ipmid.
> 
> 
> That's fine, but in that case it should not go in the callback; it should
> be maintained and enforced by net-ipmid when it looks up a handler.

Neither net-ipmid nor host-ipmid intrinsically know all of the IPMI
commands that will or may be registered.  This is especially true for
OEM commands where the privilege level is determined by the command.

Are the privilege levels defined by the IPMI spec?  If so, I don't see
anything incorrect about each provider having it.  If not, it is
something that we have defined at build time, correct?  Would it be
acceptable to have multiple symlink locations for net-ipmid providers?
ie. /usr/lib/phosphor-net-ipmid/user/ ,
/usr/lib/phosphor-net-ipmid/admin/ , etc.  I suspect we would need to
break libraries up more because currently a single library provides
commands at different privilege levels.

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Discussion on IPMI provider libraries

[tomjose]

On Tuesday 15 November 2016 04:28 PM, Patrick Williams wrote:
> On Mon, Nov 14, 2016 at 03:59:08PM -0800, Brendan Higgins wrote:
>>> The privilege provided by each command is a registration parameter and it
>>> is consumed only by net-ipmid.
>>
>> That's fine, but in that case it should not go in the callback; it should
>> be maintained and enforced by net-ipmid when it looks up a handler.
> Neither net-ipmid nor host-ipmid intrinsically know all of the IPMI
> commands that will or may be registered.  This is especially true for
> OEM commands where the privilege level is determined by the command.
Agree.
>
> Are the privilege levels defined by the IPMI spec?  If so, I don't see
> anything incorrect about each provider having it.  If not, it is
> something that we have defined at build time, correct?  Would it be
> acceptable to have multiple symlink locations for net-ipmid providers?
> ie. /usr/lib/phosphor-net-ipmid/user/ ,
> /usr/lib/phosphor-net-ipmid/admin/ , etc.  I suspect we would need to
> break libraries up more because currently a single library provides
> commands at different privilege levels.
>
My earlier mail got stripped off when Brendan replied which had details 
about the privilege levels.

The Privilege levels are defined by the IPMI specification. Table G - 
Command Number Assignments and Privilege Levels in the IPMI 
specification gives more details on this.

Every command has a privilege level assigned to the command( Admin, 
User, Operator or Callback) or the command is system interface only.

The plan is to break down the system interface commands into a separate 
library.  The apphandler library in phospor-host-ipmid had support for 
Get Message Flags,
Set BMC Global Enables and Read Event Message Buffer which are System 
Interface commands only. So that net-ipmid would not load the system 
interface commands.
In a similar way if an OEM command need not be loaded for net-ipmid, 
then no symlink would be provided for that library in 
/usr/lib/phosphor-net-ipmid.

I have pushed a patch for separating system interface commands: 
https://gerrit.openbmc-project.xyz/#/c/1020/

I did not completely understand the rationale behind having multiple 
symlink locations for net-ipmid providers. The phosphor-net-ipmid would 
load the provider libraries
at starting and net-ipmid would allow multiple sessions with different 
privilege levels. There is a 'Set Session Privilege Level' command which 
would change the privilege level to a
higher( based on the maximum privilege supported for the user) or lower 
privilege level.

So the suggestion for multiple symlink  locations for net-ipmid 
providers(net-ipmid/user.. net-ipmid/admin) is a way for net-ipmid to 
figure out the privilege level of the command
instead of a registration parameter(privilege level) in the callback 
handler?