18db6e57bSEd Tanous# OpenBMC Webserver Development 28db6e57bSEd Tanous 3f4f2643aSPatrick Williams## Guidelines 4f4f2643aSPatrick Williams 5f4f2643aSPatrick Williams### Performance targets 6dfa3fdc3SPatrick Williams 78db6e57bSEd TanousAs OpenBMC is intended to be deployed on an embedded system, care should be 88db6e57bSEd Tanoustaken to avoid expensive constructs, and memory usage. In general, our 98db6e57bSEd Tanousperformance and metric targets are: 108db6e57bSEd Tanous 118db6e57bSEd Tanous- Binaries and static files should take up < 1MB of filesystem size 128db6e57bSEd Tanous- Memory usage should remain below 10MB at all times 138db6e57bSEd Tanous- Application startup time should be less than 1 second on target hardware 148db6e57bSEd Tanous (AST2500) 158db6e57bSEd Tanous 16f4f2643aSPatrick Williams### Asynchronous programming 17dfa3fdc3SPatrick Williams 1804e8795dSGunnar MillsCare should be taken to ensure that all code is written to be asynchronous in 19f4f2643aSPatrick Williamsnature, to avoid blocking methods from stopping the processing of other tasks. 20f4f2643aSPatrick WilliamsAt this time the webserver uses boost::asio for it async framework. Threads 21f4f2643aSPatrick Williamsshould be avoided if possible, and instead use async tasks within boost::asio. 228db6e57bSEd Tanous 23f4f2643aSPatrick Williams### Secure coding guidelines 24dfa3fdc3SPatrick Williams 258db6e57bSEd TanousSecure coding practices should be followed in all places in the webserver 268db6e57bSEd Tanous 278db6e57bSEd TanousIn general, this means: 28dfa3fdc3SPatrick Williams 298db6e57bSEd Tanous- All buffer boundaries must be checked before indexing or using values 308db6e57bSEd Tanous- All pointers and iterators must be checked for null before dereferencing 31f4f2643aSPatrick Williams- All input from outside the application is considered untrusted, and should be 32f4f2643aSPatrick Williams escaped, authorized and filtered accordingly. This includes files in the 33dfa3fdc3SPatrick Williams filesystem. 348db6e57bSEd Tanous- All error statuses are checked and accounted for in control flow. 358db6e57bSEd Tanous- Where applicable, noexcept methods should be preferred to methods that use 368db6e57bSEd Tanous exceptions 378db6e57bSEd Tanous- Explicitly bounded types should be preferred over implicitly bounded types 388db6e57bSEd Tanous (like std::array<int, size> as opposed to int[size]) 39dfa3fdc3SPatrick Williams- no use of 40dfa3fdc3SPatrick Williams [Banned functions](https://github.com/intel/safestringlib/wiki/SDL-List-of-Banned-Functions "Banned function list") 418db6e57bSEd Tanous 42f4f2643aSPatrick Williams### Error handling 43dfa3fdc3SPatrick Williams 448db6e57bSEd TanousError handling should be constructed in such a way that all possible errors 458db6e57bSEd Tanousreturn valid HTTP responses. The following HTTP codes will be used commonly 46dfa3fdc3SPatrick Williams 478db6e57bSEd Tanous- 200 OK - Request was properly handled 488db6e57bSEd Tanous- 201 Created - Resource was created 498db6e57bSEd Tanous- 401 Unauthorized - Request didn't posses the necessary authentication 508db6e57bSEd Tanous- 403 Forbidden - Request was authenticated, but did not have the necessary 518db6e57bSEd Tanous permissions to accomplish the requested task 528db6e57bSEd Tanous- 404 Not found - The url was not found 53f4f2643aSPatrick Williams- 500 Internal error - Something has broken within the OpenBMC web server, and 54f4f2643aSPatrick Williams should be filed as a bug 558db6e57bSEd Tanous 56f4f2643aSPatrick WilliamsWhere possible, 307 and 308 redirects should be avoided, as they introduce the 57f4f2643aSPatrick Williamspossibility for subtle security bugs. 588db6e57bSEd Tanous 59f4f2643aSPatrick Williams### Startup times 60dfa3fdc3SPatrick Williams 61f4f2643aSPatrick WilliamsGiven that the most common target of OpenBMC is an ARM11 processor, care needs 62f4f2643aSPatrick Williamsto be taken to ensure startup times are low. In general this means: 638db6e57bSEd Tanous 64f4f2643aSPatrick Williams- Minimizing the number of files read from disk at startup. Unless a feature is 65f4f2643aSPatrick Williams explicitly intended to be runtime configurable, its logic should be "baked in" 66f4f2643aSPatrick Williams to the application at compile time. For cases where the implementation is 67f4f2643aSPatrick Williams configurable at runtime, the default values should be included in application 68f4f2643aSPatrick Williams code to minimize the use of nonvolatile storage. 698db6e57bSEd Tanous- Avoid excessive memory usage and mallocs at startup. 708db6e57bSEd Tanous 71f4f2643aSPatrick Williams### Compiler features 72dfa3fdc3SPatrick Williams 738db6e57bSEd Tanous- At this point in time, the webserver sets a number of security flags in 748db6e57bSEd Tanous compile time options to prevent misuse. The specific flags and what 75f4f2643aSPatrick Williams optimization levels they are enabled at are documented in the CMakeLists.txt 76f4f2643aSPatrick Williams file. 778db6e57bSEd Tanous- Exceptions are currently enabled for webserver builds, but their use is 78dfa3fdc3SPatrick Williams discouraged. Long term, the intent is to disable exceptions, so any use of 79f4f2643aSPatrick Williams them for explicit control flow will likely be rejected in code review. Any use 80f4f2643aSPatrick Williams of exceptions should be cases where the program can be reasonably expected to 81f4f2643aSPatrick Williams crash if the exception occurs, as this will be the future behavior once 82f4f2643aSPatrick Williams exceptions are disabled. 838db6e57bSEd Tanous- Run time type information is disabled 848db6e57bSEd Tanous- Link time optimization is enabled 858db6e57bSEd Tanous 86f4f2643aSPatrick Williams### Authentication 87dfa3fdc3SPatrick Williams 888db6e57bSEd TanousThe webserver shall provide the following authentication mechanisms. 89dfa3fdc3SPatrick Williams 908db6e57bSEd Tanous- Basic authentication 918db6e57bSEd Tanous- Cookie authentication 928db6e57bSEd Tanous- Token authentication 938db6e57bSEd Tanous 948db6e57bSEd TanousThere shall be connection between the authentication mechanism used and 958db6e57bSEd Tanousresources that are available over it. The webserver shall employ an 96f4f2643aSPatrick Williamsauthentication scheme that is in line with the rest of OpenBMC, and allows users 97f4f2643aSPatrick Williamsand privileges to be provisioned from other interfaces. 988db6e57bSEd Tanous 99f4f2643aSPatrick Williams### Web security 100dfa3fdc3SPatrick Williams 1018db6e57bSEd TanousThe OpenBMC webserver shall follow the latest OWASP recommendations for 1028db6e57bSEd Tanousauthentication, session management, and security. 1038db6e57bSEd Tanous 104f4f2643aSPatrick Williams### Performance 105dfa3fdc3SPatrick Williams 1068db6e57bSEd TanousThe performance priorities for the OpenBMC webserver are (in order): 107dfa3fdc3SPatrick Williams 1088db6e57bSEd Tanous1. Code is readable and clear 1098db6e57bSEd Tanous2. Code follows secure guidelines 1108db6e57bSEd Tanous3. Code is performant, and does not unnecessarily abstract concepts at the 1118db6e57bSEd Tanous expense of performance 112dfa3fdc3SPatrick Williams4. Code does not employ constructs which require continuous system resources, 113f4f2643aSPatrick Williams unless required to meet performance targets. (example: caching sensor values 114f4f2643aSPatrick Williams which are expected to change regularly) 1158db6e57bSEd Tanous 116f4f2643aSPatrick Williams### Abstraction/interfacing 117dfa3fdc3SPatrick Williams 1188db6e57bSEd TanousIn general, the OpenBMC webserver is built using the data driven design. 11904e8795dSGunnar MillsAbstraction and Interface guarantees should be used when multiple 1208db6e57bSEd Tanousimplementations exist, but for implementations where only a single 1218db6e57bSEd Tanousimplementation exists, prefer to make the code correct and clean rather than 1228db6e57bSEd Tanousimplement a concrete interface. 1238db6e57bSEd Tanous 124cc519de3SGunnar Mills### webui-vue 125dfa3fdc3SPatrick Williams 126cc519de3SGunnar MillsThe webserver should be capable of hosting webui-vue, and implementing the 127f4f2643aSPatrick Williamsrequired flows to host the application. In general, all access methods should be 128f4f2643aSPatrick Williamsavailable to the webui. 1298db6e57bSEd Tanous 130f4f2643aSPatrick Williams### Redfish 131dfa3fdc3SPatrick Williams 132f4f2643aSPatrick Williamsbmcweb's Redfish implementation, including Redfish OEM Resources, shall conform 133f4f2643aSPatrick Williamsto the Redfish specification. Please keep bmcweb's 134dfa3fdc3SPatrick Williams[Redfish support document](https://github.com/openbmc/bmcweb/blob/master/Redfish.md) 135f4f2643aSPatrick Williamsupdated. OEM schemas should conform and be developed in line with the rules in 1362d5fe9d0SEd Tanous[OEM SCHEMAS](https://github.com/openbmc/bmcweb/blob/master/OEM_SCHEMAS.md). 13740d68ef6SGunnar Mills 138f4f2643aSPatrick Williams### Common errors 139dfa3fdc3SPatrick Williams 140f4f2643aSPatrick WilliamsA number of examples of common errors are captured in the common errors doc. It 141f4f2643aSPatrick Williamsis recommended that developers read and understand all of them before starting 142f4f2643aSPatrick Williamsany openbmc development. 143bafb82b2SEd Tanous[Common Errors](https://github.com/openbmc/bmcweb/blob/master/COMMON_ERRORS.md). 144bafb82b2SEd Tanous 145f4f2643aSPatrick Williams### Commit messages 146f4f2643aSPatrick Williams 14719ace2b2SEd TanousProject commit message formatting should be obeyed 14819ace2b2SEd Tanous[link](https://github.com/openbmc/docs/blob/master/CONTRIBUTING.md#formatting-commit-messages) 14919ace2b2SEd Tanous 15019ace2b2SEd TanousCommit messages should answer the following questions: 151dfa3fdc3SPatrick Williams 152dfa3fdc3SPatrick Williams- Why are the changes useful? Given that bmcweb is a user-facing daemon, commits 153dfa3fdc3SPatrick Williams adding new functionality should include statements about how the commit in 154dfa3fdc3SPatrick Williams question is useful to the user. 15519ace2b2SEd Tanous 15619ace2b2SEd Tanous- What changes would a user expect to see? This includes new parameters, new 15719ace2b2SEd Tanous resources, and new or changed properties. Any route changes should be 15819ace2b2SEd Tanous explicitly called out. 15919ace2b2SEd Tanous 16019ace2b2SEd Tanous- Are there compatibility concerns? Is this change backward compatible for 161dfa3fdc3SPatrick Williams clients? If not, what commit would be broken, and how old is it? Have clients 162dfa3fdc3SPatrick Williams been warned? (ideally on the mailing list) link the discussion. 16319ace2b2SEd Tanous 16419ace2b2SEd TanousCommit messages should be line wrapped 50/72. 16519ace2b2SEd Tanous 166f4f2643aSPatrick Williams### Compatibility 167f4f2643aSPatrick Williams 168f4f2643aSPatrick Williams> Don't make your users mad - 169f4f2643aSPatrick Williams> [Greg K-H](https://git.sr.ht/~gregkh/presentation-application_summit/tree/main/keep_users_happy.pdf) 17019ace2b2SEd Tanous 171dfa3fdc3SPatrick WilliamsThe kernel has very similar rules around compatibility that we should aspire to 172dfa3fdc3SPatrick Williamsfollow in the footsteps of. 17319ace2b2SEd Tanous 17419ace2b2SEd TanousTo that end, bmcweb will do its' best to insulate clients from breaking api 17519ace2b2SEd Tanouschanges. Being explicit about this ensures that clients can upgrade their 176dfa3fdc3SPatrick WilliamsOpenBMC version without issue, and resolves a significant bottleneck in getting 177dfa3fdc3SPatrick Williamssecurity patches deployed to users. Any change that's visible to a user is 178dfa3fdc3SPatrick Williamspotentially a breaking change, but requiring _all_ visible changes to be 179dfa3fdc3SPatrick Williamsconfigurable would increase the software complexity, therefore bmcweb makes 180dfa3fdc3SPatrick Williamsexceptions for things which a client is reasonably expected to code against: 181dfa3fdc3SPatrick Williams 18219ace2b2SEd Tanous- New items added to a collection 18319ace2b2SEd Tanous- Changes in UID for hypermedia resources (In line with Redfish spec) 18419ace2b2SEd Tanous- New properties added to a resource 18519ace2b2SEd Tanous- New versions of a given schema 18619ace2b2SEd Tanous 187dfa3fdc3SPatrick WilliamsSpecial note: Code exists in bmcweb that is missing upstream backends to make it 188dfa3fdc3SPatrick Williamsfunction. Given that compatibility requires the ability to use and test the 189dfa3fdc3SPatrick Williamsfeature in question, changes to these methods, including outright removal, does 190dfa3fdc3SPatrick Williamsnot constitute a breaking change. 19119ace2b2SEd Tanous 192dfa3fdc3SPatrick WilliamsSecurity: There may be cases where maintainers make explicit breaking changes in 193dfa3fdc3SPatrick Williamsthe best interest of security; In these rare cases, the maintainers and 194dfa3fdc3SPatrick Williamscontributors will endeavor to avoid breaking clients as much as is technically 195dfa3fdc3SPatrick Williamspossible, but as with all security, impact will need to be weighed against the 1968ece0e45SEd Tanoussecurity impact of not making changes, and judgment calls will be made, with 197dfa3fdc3SPatrick Williamsoptions to allow providing the old behavior. 1983e6217d7SGunnar Mills 1993cd642a0SGunnar Mills### clang-tidy 200403e0ea3SEd Tanous 201403e0ea3SEd Tanousclang-tidy is a tool that can be used to identify coding style violations, bad 2022c70f800SEd Tanousdesign patterns, and bug prone constructs. The checks are implemented in the 203dfa3fdc3SPatrick Williams.clang-tidy file in the root of bmcweb, and are expected to be passing. 204dfa3fdc3SPatrick Williams[openbmc-build-scripts](https://github.com/openbmc/openbmc-build-scripts/blob/master/run-unit-test-docker.sh) 205c5bb9982SEd Tanousimplements clang-tidy checks and is the recommended way to run these checks 2060e88cb37SGunnar Mills 2070e88cb37SGunnar Mills### Logging Levels 2080e88cb37SGunnar Mills 2090e88cb37SGunnar MillsFive bmcweb logging levels are supported, from least to most severity: 2100e88cb37SGunnar Mills 2110e88cb37SGunnar Mills- debug 2120e88cb37SGunnar Mills- info 2130e88cb37SGunnar Mills- warning 2140e88cb37SGunnar Mills- error 2150e88cb37SGunnar Mills- critical 2160e88cb37SGunnar Mills 2170e88cb37SGunnar MillsAnd their use cases: 2180e88cb37SGunnar Mills 2190e88cb37SGunnar Mills- critical: Something went badly wrong, and we're no longer able to serve 2200e88cb37SGunnar Mills traffic. "critical" should be used when bmcweb encountered an event or entered 2210e88cb37SGunnar Mills a state that caused crucial function to stop working or when bmcweb 2220e88cb37SGunnar Mills encountered a fatal error. 2230e88cb37SGunnar Mills- error: Something went wrong, and we weren't able to give the expected 2240e88cb37SGunnar Mills response. Service is still operational. "error" should be used for unexpected 2250e88cb37SGunnar Mills conditions that prevented bmcweb from fulfilling the request. "error" shall be 2260e88cb37SGunnar Mills used for 5xx errors. 2270e88cb37SGunnar Mills- warning: A condition occurred that is outside the expected flows, but isn't 2280e88cb37SGunnar Mills necessarily an error in the webserver, or might only be an error in certain 2290e88cb37SGunnar Mills scenarios. For example, connection drops or 4xx errors. 2300e88cb37SGunnar Mills- info: Information for the golden path debugging. 2310e88cb37SGunnar Mills- debug: Information that's overly verbose such that it shouldn't be printed in 2320e88cb37SGunnar Mills all debug scenarios, but might be useful in some debug contexts. 2330e88cb37SGunnar Mills 2340e88cb37SGunnar Mills### Enabling logging 2350e88cb37SGunnar Mills 2360e88cb37SGunnar Millsbmcweb by default is compiled with runtime logging disabled, as a performance 237662aa6e3SMyung Baeconsideration. To enable it in a standalone build, add the logging level 2380e88cb37SGunnar Mills 2390e88cb37SGunnar Mills```ascii 240662aa6e3SMyung Bae-Dlogging='debug' 2410e88cb37SGunnar Mills``` 2420e88cb37SGunnar Mills 2430e88cb37SGunnar Millsoption to your configure flags. If building within Yocto, add the following to 2440e88cb37SGunnar Millsyour local.conf. 2450e88cb37SGunnar Mills 2460e88cb37SGunnar Mills```bash 247662aa6e3SMyung BaeEXTRA_OEMESON:pn-bmcweb:append = "-Dbmcweb-logging='debug'" 2480e88cb37SGunnar Mills``` 249bd1299b7SAushim Nagarkatti 250bd1299b7SAushim Nagarkattibmcweb also supports setting logging level at runtime. The bmcweb CLI 251bd1299b7SAushim Nagarkattiapplication can be used to set the daemon's logging level on target. For 252bd1299b7SAushim Nagarkattiexample, to set logging level to INFO: 253bd1299b7SAushim Nagarkatti 254bd1299b7SAushim Nagarkatti```bash 255*a553291fSAushim Nagarkattibmcweb loglevel info 256bd1299b7SAushim Nagarkatti``` 257