[Starlingx-discuss] About configfile patch for redfishtool

Saul Wold sgw at linux.intel.com
Fri Jul 12 15:13:09 UTC 2019 

+1, thanks for following up with Upstream, I am glad they accepted this.

If you plan on extending the configuration file usage, I would recommend 
trying to push it upstream first.  I would also suggest re-factoring 
where and how the config file is parsed, that is outside of the options 
parsing, not changing the format.

Sau!


On 7/12/19 5:40 AM, MacDonald, Eric wrote:
> Awesome !!
> 
>> -----Original Message-----
>> From: Liu, ZhipengS [mailto:zhipengs.liu at intel.com]
>> Sent: Friday, July 12, 2019 1:41 AM
>> To: MacDonald, Eric; Saul Wold
>> Cc: starlingx-discuss at lists.starlingx.io
>> Subject: RE: [Starlingx-discuss] About configfile patch for redfishtool
>> Importance: High
>>
>> Hi Eric and Saul,
>>
>> My patch has already been accepted by upstream and merged.
>> https://github.com/DMTF/Redfishtool/pull/67
>> So, I believe we can remove this patch soon.
>>
>> Thanks!
>> Zhipeng
>>
>> -----Original Message-----
>> From: Liu, ZhipengS [mailto:zhipengs.liu at intel.com]
>> Sent: 2019年7月8日 22:08
>> To: starlingx-discuss at lists.starlingx.io
>> Subject: [Starlingx-discuss] About configfile patch for redfishtool
>>
>> Hi all,
>>
>> Let's discuss this topic here. Your comments are welcome!
>>
>> Thanks!
>> Zhipeng
>>
>> -----Original Message-----
>> From: MacDonald, Eric [mailto:Eric.MacDonald at windriver.com]
>> Sent: 2019年7月8日 19:41
>> To: Saul Wold <sgw at linux.intel.com>; Liu, ZhipengS <zhipengs.liu at intel.com>
>> Cc: Hu, Yong <yong.hu at intel.com>; Rowsell, Brent <Brent.Rowsell at windriver.com>; Eslimi, Dariush
>> <Dariush.Eslimi at windriver.com>; Khalil, Ghada <Ghada.Khalil at windriver.com>; Xie, Cindy
>> <cindy.xie at intel.com>
>> Subject: RE: About configfile patch
>>
>> Saul,
>>
>> Very good points and suggestion. Thank you.
>>
>> Zhipeng,
>>
>> Can you put this out to the general starling-x discussion list as well as Redfish discussion list and keep
>> us informed as to how the redfish community is reacting to the change request.
>>
>> Eric.
>>
>>> -----Original Message-----
>>> From: Saul Wold [mailto:sgw at linux.intel.com]
>>> Sent: Sunday, July 07, 2019 6:42 PM
>>> To: Liu, ZhipengS; MacDonald, Eric
>>> Cc: Hu, Yong
>>> Subject: Re: About configfile patch
>>> Importance: High
>>>
>>>
>>> Hi Zihipeng, Eric:
>>>
>>> I would like to see this move to the general discuss list, I think
>>> it's appropriate for everyone to understand what's going. Thanks for
>>> getting the patch proposed to upstream Redfish.
>>>
>>> I am concerned first with the technical debt and making sure that the
>>> Redfish upstream community is aware of what we are proposing / doing
>>> in StarlingX.  I had another look at this and I now have a better idea
>>> of why it kept being a concern.
>>>
>>> 1) processing the config file itself inside of options processing is
>>> not generally a good idea.  It does not allow for easy parsing and extension
>>>    of the config file's contents.
>>>
>>> 2) I see you using json, this is good, thanks for proposing it to the
>>> Redfish community, they might have an idea to use a different format
>>> for the contents of the config file. This gets the json idea out there
>>> now rather than finding out in 6 month they they decided to use a
>>> different format.
>>>
>>> 3) As I have mentioned before having plain text passwords is never my
>>> favorite way to go, but since we are already down that path with IPMI,
>>> let's keep going, again maybe the RedFish community had thought about
>>> this or this patch proposal will force that discussion.
>>>
>>> My sunday afternoon thoughts.
>>>
>>> Sau!
>>>
>>>
>>> On 7/4/19 7:24 PM, Liu, ZhipengS wrote:
>>>> +Saul and Yong,
>>>>
>>>> Hi Saul,
>>>>
>>>> Below email thread may give you some clarification about your concern.
>>>>
>>>> Zhipeng
>>>>
>>>> *From:* Liu, ZhipengS
>>>> *Sent:* 2019年7月5日 10:20
>>>> *To:* 'MacDonald, Eric' <Eric.MacDonald at windriver.com>
>>>> *Subject:* RE: About configfile patch
>>>>
>>>> I can see password through
>>>>
>>>> ps  –n
>>>>
>>>> Thanks!
>>>>
>>>> Zhipeng
>>>>
>>>> *From:* Liu, ZhipengS
>>>> *Sent:* 2019年7月5日 10:01
>>>> *To:* 'MacDonald, Eric' <Eric.MacDonald at windriver.com
>>>> <mailto:Eric.MacDonald at windriver.com>>
>>>> *Subject:* RE: About configfile patch
>>>>
>>>> Hi Eric,
>>>>
>>>> Thanks for your clarification!
>>>>
>>>> BTW, how to use process listing, could you give me an example? J
>>>>
>>>> Zhipeng
>>>>
>>>> *From:*MacDonald, Eric [mailto:Eric.MacDonald at windriver.com]
>>>> *Sent:* 2019年7月4日20:26
>>>> *To:* Liu, ZhipengS <zhipengs.liu at intel.com
>>>> <mailto:zhipengs.liu at intel.com>>
>>>> *Subject:* RE: About configfile patch
>>>>
>>>> Hi Zhipeng,
>>>>
>>>> See below.
>>>>
>>>> Is Saul’s concern the technical debt of the config patch or the pw
>>>> file in general. Seems the former.
>>>>
>>>> What can  do, should I speak with him ?
>>>>
>>>> Eric.
>>>>
>>>> *From:*Liu, ZhipengS [mailto:zhipengs.liu at intel.com]
>>>> *Sent:* Thursday, July 04, 2019 4:30 AM
>>>> *To:* MacDonald, Eric
>>>> *Subject:* About configfile patch
>>>> *Importance:* High
>>>>
>>>> Hi Eric,
>>>>
>>>> For configfile patch, Saul still have some concern about it and why
>>>> we use a password file
>>>>
>>>> Anyway, I have submitted patch to upstream.
>>>>
>>>> https://github.com/DMTF/Redfishtool/pull/67
>>>>
>>>>   From code, I can see that MTC get bmc_pw through keyring.
>>>>
>>>> */[... Eric ] or barbican now, yes./*
>>>>
>>>> Then we pass the bmc_pw through extra_info to ipmi command thread
>>>>
>>>> */[... Eric ] Yes/*
>>>>
>>>> “The current implementation using IPMITOOL puts the BMC password
>>>> into a short lived root privilege temp file so that it does not show
>>>> up in a process listing.”
>>>>
>>>> Why we have to use temp file instead of showing up in a process
>>>> listing?    The password can be got through process listing?  Not
>>>> clear about this point.
>>>>
>>>> */[... Eric ] If we use the –P <pw> option when invoking ipmitool
>>>> then while that command is active and someone does a process listing
>>>> then they can see the –P <pw> on the process listing. This is a
>>>> security issue because a non-root user can learn the BMC password
>>>> for any host by just doing a process listing on the active
>>>> controller./*
>>>>
>>>> Could you give me more detail information, thanks!
>>>>
>>>>   From below code, it seems we have comment related code.  Does it
>>>> means the file may not be removed right away even with the file open.
>>>>
>>>> So, still not sure which one is much more safe.
>>>>
>>>> */[... Eric ] The temp file is removed in the thread after execution
>>>> completion or timeout./*
>>>>
>>>> */Example code taken from mtcThreads.cpp/*
>>>>
>>>> *//*
>>>>
>>>> *//*
>>>>
>>>> */There is also a garbage collection cleanup audit that ensures
>>>> these temp files do not linger due to ‘say’ a process restart during
>>>> command
>>>> execution./*
>>>>
>>>> */[... Eric ] /**//*
>>>>
>>>> *//*
>>>>
>>>> *
>>>>
>>>> * TODO: fix or figure out why the unlink removes the file right away
>>>> even
>>>>
>>>> *       with the file open.
>>>>
>>>> *
>>>>
>>>> ********************************************************************
>>>> *********/
>>>>
>>>> */[... Eric ] The above comment was added simply because when I was
>>>> coding I didn’t understand why the unlink removes the file right
>>>> away./*
>>>>
>>>> */I think now that it was because the file was not open at the time
>>>> the unlink was executed./*
>>>>
>>>> *//*
>>>>
>>>> */In any case the tmp pw file is still removed with redundancy./*
>>>>
>>>> int hostUtil_mktmpfile ( string hostname, string basename, string &
>>>> filename, string data )
>>>>
>>>> {
>>>>
>>>>       // buffer to hold the temporary file name
>>>>
>>>>       char tempBuff[MAX_FILENAME_LEN];
>>>>
>>>>       int fd = -1;
>>>>
>>>>       memset(tempBuff,0,sizeof(tempBuff));
>>>>
>>>>       if ( basename.empty() || data.empty() )
>>>>
>>>>       {
>>>>
>>>>           slog ("%s called with one or more bad parameters
>>>> (%d:%d)\n",
>>>>
>>>>                     hostname.c_str(), basename.empty(),
>>>> data.empty());
>>>>
>>>>           return (0);
>>>>
>>>>       }
>>>>
>>>>       /* add what mkstemp will make unique */
>>>>
>>>>       basename.append("XXXXXX");
>>>>
>>>>       // Copy the relevant information in the buffers
>>>>
>>>>       snprintf ( &tempBuff[0], MAX_FILENAME_LEN, "%s",
>>>> basename.data());
>>>>
>>>>       // Create the temporary file, this function will
>>>>
>>>>       // replace the 'X's with random letters
>>>>
>>>>       fd = mkstemp(tempBuff);
>>>>
>>>> // Call unlink so that whenever the file is closed or the program
>>>> exits
>>>>
>>>>       // the temporary file is deleted.
>>>>
>>>>       //
>>>>
>>>>       // Note: Unlinking removes the file immediately.
>>>>
>>>>       // Commenting out. Caller must remove file.
>>>>
>>>>       //
>>>>
>>>>       // unlink(tempBuff);
>>>>
>>>> Thanks!
>>>>
>>>> Zhipeng
>>>>
>>>> *From:* MacDonald, Eric [mailto:Eric.MacDonald at windriver.com]
>>>> *Sent:* 2019年7月2日 19:21
>>>> *To:* Liu, ZhipengS <zhipengs.liu at intel.com
>>>> <mailto:zhipengs.liu at intel.com>>
>>>> *Subject:* WolfPass Sensors
>>>>
>>>> Hi Zhipeng,
>>>>
>>>> I've been upgrading the firmware on our set of WolfPass servers.
>>>>
>>>> Even with the upgrade I’ve been having a hard time reading ther
>>>> server sensors through redfish.
>>>>
>>>> Can you send me the command(s) you use and output you see for/when
>>>> dumping the sensors on your WolfPass server ?
>>>>
>>>> I use the following commands on the supermicro but it seems that the
>>>> wolfpass servers don't support this method.
>>>>
>>>> redfishtool -r <BMC IP> -u <bmc username> -p <bmc password> Chassis
>>>> Thermal
>>>>
>>>> redfishtool -r <BMC IP> -u <bmc username> -p <bmc password> Chassis
>>>> Power
>>>>
>>>> Here are the firmware versions I have. I wonder if it’s my SDR version.
>>>> What is yours ?
>>>>
>>>> *WolfPass*
>>>>
>>>>
>>>>
>>>> *BMC FW*
>>>>
>>>>
>>>>
>>>> *ME*
>>>>
>>>>
>>>>
>>>> *SDR*
>>>>
>>>>
>>>>
>>>> *Redfish Version*
>>>>
>>>> WolfPass 1
>>>>
>>>>
>>>>
>>>> 1.93.870cf4f0
>>>>
>>>>
>>>>
>>>> 04.00.04.340
>>>>
>>>>
>>>>
>>>> 1.04
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 2
>>>>
>>>>
>>>>
>>>> 1.93.870cf4f0
>>>>
>>>>
>>>>
>>>> 04.00.04.340
>>>>
>>>>
>>>>
>>>> 1.04
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 3
>>>>
>>>>
>>>>
>>>> 1.93.870cf4f0
>>>>
>>>>
>>>>
>>>> 04.00.04.288
>>>>
>>>>
>>>>
>>>> 1.29
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 4
>>>>
>>>>
>>>>
>>>> 1.29.7d703f59
>>>>
>>>>
>>>>
>>>> 04.00.04.288
>>>>
>>>>
>>>>
>>>> 1.29
>>>>
>>>>
>>>>
>>>> No Redfish Support
>>>>
>>>> WolfPass 5
>>>>
>>>>
>>>>
>>>> 1.29.7d703f59
>>>>
>>>>
>>>>
>>>> 04.00.04.288
>>>>
>>>>
>>>>
>>>> 1.29
>>>>
>>>>
>>>>
>>>> No Redfish Support
>>>>
>>>> WolfPass 6
>>>>
>>>>
>>>>
>>>> 1.29.7d703f59
>>>>
>>>>
>>>>
>>>> 04.00.04.288
>>>>
>>>>
>>>>
>>>> 1.29
>>>>
>>>>
>>>>
>>>> No Redfish Support
>>>>
>>>> WolfPass 7
>>>>
>>>>
>>>>
>>>> 1.29.7d703f59
>>>>
>>>>
>>>>
>>>> 04.00.04.288
>>>>
>>>>
>>>>
>>>> 1.29
>>>>
>>>>
>>>>
>>>> No Redfish Support
>>>>
>>>> WolfPass 8
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 9
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 10
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 11
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 12
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.340
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 13
>>>>
>>>>
>>>>
>>>> 1.93.870cf4f0
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 14
>>>>
>>>>
>>>>
>>>> 1.93.870cf4f0
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 15
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.340
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 16
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> WolfPass 17
>>>>
>>>>
>>>>
>>>> 1.43.660a4315
>>>>
>>>>
>>>>
>>>> 04.00.04.294
>>>>
>>>>
>>>>
>>>> 1.43
>>>>
>>>>
>>>>
>>>>       "RedfishVersion": "1.1.0",
>>>>
>>>> Cheers,
>>>>
>>>> Eric MacDonald, MTS, Engineering, Wind River
>>>>
>>>> direct 613.963.1387  fax: 613.492.7870  skype: eric.r.macdonald
>>>>
>>>> 350 Terry Fox Drive, Suite 200, Kanata, ON K2K 2W5
>>>>
>> _______________________________________________
>> Starlingx-discuss mailing list
>> Starlingx-discuss at lists.starlingx.io
>> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
> _______________________________________________
> Starlingx-discuss mailing list
> Starlingx-discuss at lists.starlingx.io
> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
>

More information about the Starlingx-discuss mailing list