1# OpenBMC Webserver Development 2 3## Guidelines 4 5### Performance targets 6 7As OpenBMC is intended to be deployed on an embedded system, care should be 8taken to avoid expensive constructs, and memory usage. In general, our 9performance and metric targets are: 10 11- Binaries and static files should take up < 1MB of filesystem size 12- Memory usage should remain below 10MB at all times 13- Application startup time should be less than 1 second on target hardware 14 (AST2500) 15 16### Asynchronous programming 17 18Care should be taken to ensure that all code is written to be asynchronous in 19nature, to avoid blocking methods from stopping the processing of other tasks. 20At this time the webserver uses boost::asio for it async framework. Threads 21should be avoided if possible, and instead use async tasks within boost::asio. 22 23### Secure coding guidelines 24 25Secure coding practices should be followed in all places in the webserver 26 27In general, this means: 28 29- All buffer boundaries must be checked before indexing or using values 30- All pointers and iterators must be checked for null before dereferencing 31- All input from outside the application is considered untrusted, and should be 32 escaped, authorized and filtered accordingly. This includes files in the 33 filesystem. 34- All error statuses are checked and accounted for in control flow. 35- Where applicable, noexcept methods should be preferred to methods that use 36 exceptions 37- Explicitly bounded types should be preferred over implicitly bounded types 38 (like std::array<int, size> as opposed to int[size]) 39- no use of 40 [Banned functions](https://github.com/intel/safestringlib/wiki/SDL-List-of-Banned-Functions "Banned function list") 41 42### Error handling 43 44Error handling should be constructed in such a way that all possible errors 45return valid HTTP responses. The following HTTP codes will be used commonly 46 47- 200 OK - Request was properly handled 48- 201 Created - Resource was created 49- 401 Unauthorized - Request didn't posses the necessary authentication 50- 403 Forbidden - Request was authenticated, but did not have the necessary 51 permissions to accomplish the requested task 52- 404 Not found - The url was not found 53- 500 Internal error - Something has broken within the OpenBMC web server, and 54 should be filed as a bug 55 56Where possible, 307 and 308 redirects should be avoided, as they introduce the 57possibility for subtle security bugs. 58 59### Startup times 60 61Given that the most common target of OpenBMC is an ARM11 processor, care needs 62to be taken to ensure startup times are low. In general this means: 63 64- Minimizing the number of files read from disk at startup. Unless a feature is 65 explicitly intended to be runtime configurable, its logic should be "baked in" 66 to the application at compile time. For cases where the implementation is 67 configurable at runtime, the default values should be included in application 68 code to minimize the use of nonvolatile storage. 69- Avoid excessive memory usage and mallocs at startup. 70 71### Compiler features 72 73- At this point in time, the webserver sets a number of security flags in 74 compile time options to prevent misuse. The specific flags and what 75 optimization levels they are enabled at are documented in the CMakeLists.txt 76 file. 77- Exceptions are currently enabled for webserver builds, but their use is 78 discouraged. Long term, the intent is to disable exceptions, so any use of 79 them for explicit control flow will likely be rejected in code review. Any use 80 of exceptions should be cases where the program can be reasonably expected to 81 crash if the exception occurs, as this will be the future behavior once 82 exceptions are disabled. 83- Run time type information is disabled 84- Link time optimization is enabled 85 86### Authentication 87 88The webserver shall provide the following authentication mechanisms. 89 90- Basic authentication 91- Cookie authentication 92- Token authentication 93 94There shall be connection between the authentication mechanism used and 95resources that are available over it. The webserver shall employ an 96authentication scheme that is in line with the rest of OpenBMC, and allows users 97and privileges to be provisioned from other interfaces. 98 99### Web security 100 101The OpenBMC webserver shall follow the latest OWASP recommendations for 102authentication, session management, and security. 103 104### Performance 105 106The performance priorities for the OpenBMC webserver are (in order): 107 1081. Code is readable and clear 1092. Code follows secure guidelines 1103. Code is performant, and does not unnecessarily abstract concepts at the 111 expense of performance 1124. Code does not employ constructs which require continuous system resources, 113 unless required to meet performance targets. (example: caching sensor values 114 which are expected to change regularly) 115 116### Abstraction/interfacing 117 118In general, the OpenBMC webserver is built using the data driven design. 119Abstraction and Interface guarantees should be used when multiple 120implementations exist, but for implementations where only a single 121implementation exists, prefer to make the code correct and clean rather than 122implement a concrete interface. 123 124### phosphor webui 125 126The webserver should be capable of hosting phosphor-webui, and implementing the 127required flows to host the application. In general, all access methods should be 128available to the webui. 129 130### Redfish 131 132bmcweb's Redfish implementation, including Redfish OEM Resources, shall conform 133to the Redfish specification. Please keep bmcweb's 134[Redfish support document](https://github.com/openbmc/bmcweb/blob/master/Redfish.md) 135updated. OEM schemas should conform and be developed in line with the rules in 136[OEM SCHEMAS](https://github.com/openbmc/bmcweb/blob/master/OEM_SCHEMAS.md). 137 138### Common errors 139 140A number of examples of common errors are captured in the common errors doc. It 141is recommended that developers read and understand all of them before starting 142any openbmc development. 143[Common Errors](https://github.com/openbmc/bmcweb/blob/master/COMMON_ERRORS.md). 144 145### Commit messages 146 147Project commit message formatting should be obeyed 148[link](https://github.com/openbmc/docs/blob/master/CONTRIBUTING.md#formatting-commit-messages) 149 150Commit messages should answer the following questions: 151 152- Why are the changes useful? Given that bmcweb is a user-facing daemon, commits 153 adding new functionality should include statements about how the commit in 154 question is useful to the user. 155 156- What changes would a user expect to see? This includes new parameters, new 157 resources, and new or changed properties. Any route changes should be 158 explicitly called out. 159 160- Are there compatibility concerns? Is this change backward compatible for 161 clients? If not, what commit would be broken, and how old is it? Have clients 162 been warned? (ideally on the mailing list) link the discussion. 163 164Commit messages should be line wrapped 50/72. 165 166### Compatibility 167 168> Don't make your users mad - 169> [Greg K-H](https://git.sr.ht/~gregkh/presentation-application_summit/tree/main/keep_users_happy.pdf) 170 171The kernel has very similar rules around compatibility that we should aspire to 172follow in the footsteps of. 173 174To that end, bmcweb will do its' best to insulate clients from breaking api 175changes. Being explicit about this ensures that clients can upgrade their 176OpenBMC version without issue, and resolves a significant bottleneck in getting 177security patches deployed to users. Any change that's visible to a user is 178potentially a breaking change, but requiring _all_ visible changes to be 179configurable would increase the software complexity, therefore bmcweb makes 180exceptions for things which a client is reasonably expected to code against: 181 182- New items added to a collection 183- Changes in UID for hypermedia resources (In line with Redfish spec) 184- New properties added to a resource 185- New versions of a given schema 186 187Special note: Code exists in bmcweb that is missing upstream backends to make it 188function. Given that compatibility requires the ability to use and test the 189feature in question, changes to these methods, including outright removal, does 190not constitute a breaking change. 191 192Security: There may be cases where maintainers make explicit breaking changes in 193the best interest of security; In these rare cases, the maintainers and 194contributors will endeavor to avoid breaking clients as much as is technically 195possible, but as with all security, impact will need to be weighed against the 196security impact of not making changes, and judgement calls will be made, with 197options to allow providing the old behavior. 198 199## clang-tidy 200 201clang-tidy is a tool that can be used to identify coding style violations, bad 202design patterns, and bug prone constructs. The checks are implemented in the 203.clang-tidy file in the root of bmcweb, and are expected to be passing. 204[openbmc-build-scripts](https://github.com/openbmc/openbmc-build-scripts/blob/master/run-unit-test-docker.sh) 205implements clang-tidy checks and is the recommended way to run these checks 206