[Ipmitool-devel] Code Review - ID: 2871903 - ipmitool user priv incorrectly sets Link Auth

[Zdenek Styblik]

Hello all,

there is a bug in 'lib/ipmi_user.c', resp. # ipmitool user set priv
<user_id> <priv_level> <channel>; which turns off Link
Authorization(details are in the ticket). Originally, it was proposed
to extend 'set priv' for '[link_auth]' parameter, however I find it to
be pointless since this already is covered by 'channel' sub-command.
Then I thought it could be solved by querying BMC for user access and
re-use this information. But I've noticed IPMI specification allows to
disable changes to link/ipmi/callback in Set User Access Command,
p.320 IPMIv2.0 PDF, by setting the 7th bit to 0. However, by doing OR
0x90h this bit is set to 1(I guess because we want to turn on IPMI
messaging).

Now, the question and proposed fix. What I don't understand is why
ipmi_user_set_userpriv() implicitly sets IPMI messaging on since this
is, again, covered by 'channel' sub-command. Is there a reason to do
so? Because I see none.
Anyway, the fix is either to:
~~~
- msg_data[0] |= 0x90; /* enable ipmi messaging */
~~~
or query BMC for user access info and re-use it all the way. I think
the latter is stupid since changes to link/ipmi/callback are 1]
available via 'channel' sub-command 2] changes to link/ipmi/callback
can be ignored by BMC just fine.

Thanks,
Z.

[Zdenek Styblik]

On Fri, Mar 29, 2013 at 9:07 PM, Zdenek Styblik

Post by Zdenek Styblik
Hello all,
there is a bug in 'lib/ipmi_user.c', resp. # ipmitool user set priv
<user_id> <priv_level> <channel>; which turns off Link
Authorization(details are in the ticket). Originally, it was proposed
to extend 'set priv' for '[link_auth]' parameter, however I find it to
be pointless since this already is covered by 'channel' sub-command.
Then I thought it could be solved by querying BMC for user access and
re-use this information. But I've noticed IPMI specification allows to
disable changes to link/ipmi/callback in Set User Access Command,
p.320 IPMIv2.0 PDF, by setting the 7th bit to 0. However, by doing OR
0x90h this bit is set to 1(I guess because we want to turn on IPMI
messaging).
Now, the question and proposed fix. What I don't understand is why
ipmi_user_set_userpriv() implicitly sets IPMI messaging on since this
is, again, covered by 'channel' sub-command. Is there a reason to do
so? Because I see none.
~~~
- msg_data[0] |= 0x90; /* enable ipmi messaging */
~~~
or query BMC for user access info and re-use it all the way. I think
the latter is stupid since changes to link/ipmi/callback are 1]
available via 'channel' sub-command 2] changes to link/ipmi/callback
can be ignored by BMC just fine.
Thanks,
Z.

Ok. Please, could somebody try the proposed fix? It seems it doesn't
work for me and whole request is ignored instead. Diff is attached.

How to reproduce current problem:
1] create 'test' user on BMC
# ipmitool user list 1
4 test true true true OPERATOR
2] # ipmitool user priv 4 4 1
3] # ipmitool user list 1
4 test true false true OPERATOR

What to test after applying the patch:
# ipmitool user priv 4 4 1
# ipmitool user list 1 | grep test
# ipmitool user priv 4 3 1
# ipmitool user list 1 | grep test

But it doesn't change privilege level for me anymore when I apply the
diff. I think the whole request is silently ignored. I also think this
is IPMI stack implementation bug, not IPMI specification bug. Either
that, or my interpretation of IPMI specification is wrong. I really
can't decide which is it.


IPMIv2.0 spec:
1st byte, 7th bit
* 0b - do not change any of the following bits in this byte
* 1b - enable changing the following bits in this byte

and there is callback[6], link auth[5], ipmi messaging[4], channel[3:0].

Thanks,
Z.