<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle17
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:959578651;
mso-list-type:hybrid;
mso-list-template-ids:-849995654 -1215015590 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l0:level1
{mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:.75in;
text-indent:-.25in;
font-family:"Calibri",sans-serif;
mso-fareast-font-family:Calibri;}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:1.25in;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:1.75in;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:2.25in;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:2.75in;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:3.25in;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:3.75in;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:4.25in;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:\F0A7;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:4.75in;
text-indent:-.25in;
font-family:Wingdings;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:#1F497D">I started reviewing some of the code, and I have quite a few thoughts, let’s see if I can convey them without writing a book. The main aspect I’m looking at is the redfish interface, but many of these apply equally
well to the REST and redfish parts. Here are my initial thoughts when looking at this proposal.<br>
<br>
1) extensibility: I see openbmc as a base that many people will want to build upon. To enable that, we need to be able to have easy ways to extend or override the things in the base.<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Compile time extensions vs runtime loadable – open question<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Need to add OEM extensions to individual URLs<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Need to override specific properties in an individual URL<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Need to be able to add entire new URI trees<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Need to extend existing trees with links/references to new trees<o:p></o:p></span></p>
<p class="MsoListParagraph" style="margin-left:.75in;text-indent:-.25in;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="color:#1F497D">Need to dynamically generate the metadata URIs<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">My commentary while reviewing this code is that I don’t immediately see obvious ways of doing any of these extensions that don’t effectively fork the entire codebase and make future maintenance much more difficult.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">2) (redfish) Easier ways of generating JSON and mapping JSON properties to backend data: The initial code to generate JSON I see here is quite simple, but one aspect worries me. It appears that you have a nice
and simple way to create the output JSON data structure, but right now most everything is hard coded. Once you start filling in the data “for real”, we will need to write an ever-growing pile of code to translate from DBUS calls to get the data and poke the
data into the correct spot in the JSON output. In my mind, DBUS is introspectable and JSON is introspectable. It seems like it would be a big win to create a small, generic JSON engine that holds the data and then reads metadata about which calls to make
to update the data.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">3) performance: It looks like you currently make backend calls to satisfy requests for data. This means that more frontend calls will result in higher load on the backend. This means that it would be possible
for frontend load to impact things like power/thermal reaction times without some limits on the frontend webserver. However, the DBUS architecture has a signal based system for generating events that we could leverage. The webserver could keep an internal
copy of all of its JSON output and subscribe to DBUS signals/events to update that internal cache. This way, external web requests do not automatically translate every time into internal DBUS calls. The webserver could easily satisfy most web requests using
data it already has on hand. This would result in much higher performance <o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">4) security: the redfish standard has extension points to enable uploading new permission mappings or dynamically create roles and permissions. It’s not clear to me how we would satisfy these dynamic security
requirements<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Token infrastructure: I haven’t looked at this code yet. In general, though, I really like having token-based security middleware so that we can provide extensions to support OAuth 2.0 and OpenID Connect support.
The main idea here is that we need a way to delegate security decisions to a trusted third party. If we are using standard JWT tokens compatible with OIDC for identity, this makes the transition much easier.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">--<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Michael<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="color:#1F497D"><o:p> </o:p></span></a></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> openbmc [mailto:openbmc-bounces+michael.e.brown=dell.com@lists.ozlabs.org]
<b>On Behalf Of </b>Tanous, Ed<br>
<b>Sent:</b> Monday, December 18, 2017 10:08 PM<br>
<b>To:</b> OpenBMC Maillist <openbmc@lists.ozlabs.org><br>
<b>Subject:</b> Request for comments: C++ embedded webserver<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’m looking for comments on a code review that’s been outstanding. One of the large pushes we’ve made is to attempt to make the web server more efficient, and add capabilities that comprehend long term needs of OpenBmc. One key that wasn’t
made clear in the commit message is that it includes the basic redfish implementation that (we hope) should be extensible to the full redfish specification in the short term future.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><a href="https://gerrit.openbmc-project.xyz/#/c/7786/">https://gerrit.openbmc-project.xyz/#/c/7786/</a><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">We would very much appreciate comments to see if we can move this forward.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The following description is pulled from the readme.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"># OpenBMC webserver #<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">This component attempts to be a "do everything" embedded webserver for openbmc.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">## Capabilities ##<o:p></o:p></p>
<p class="MsoNormal">At this time, the webserver implements a few interfaces:<o:p></o:p></p>
<p class="MsoNormal">+ Authentication middleware that supports cookie and token based authentication, as well as CSRF prevention backed by linux PAM authentication credentials.<o:p></o:p></p>
<p class="MsoNormal">+ An (incomplete) attempt at replicating phosphor-dbus-rest interfaces in C++. Right now, a few of the endpoint definitions work as expected, but there is still a lot of work to be done. The portions of the interface that are functional
are designed to work correctly for phosphor-webui, but may not yet be complete.<o:p></o:p></p>
<p class="MsoNormal">+ Replication of the rest-dbus backend interfaces to allow bmc debug to logged in users.<o:p></o:p></p>
<p class="MsoNormal">+ An initial attempt at a read-only redfish interface. Currently the redfish interface targets ServiceRoot, SessionService, AccountService, Roles, and ManagersService. Some functionality here has been shimmed to make development possible.
For example, there exists only a single user role.<o:p></o:p></p>
<p class="MsoNormal">+ SSL key generation at runtime. If an RSA key and cert pair are not available to the server at runtime, keys are generated using the openssl routines, and written to disk.<o:p></o:p></p>
<p class="MsoNormal">+ Static file hosting. Currently, static files are hosted from the fixed location at /usr/share/www. This is intended to allow loose coupling with yocto projects, and allow overriding static files at build time.<o:p></o:p></p>
<p class="MsoNormal">+ Dbus-monitor over websocket. A generic endpoint that allows UIs to open a websocket and register for notification of events to avoid polling in single page applications. (this interface may be modified in the future due to security
concerns.)<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-Ed<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>