summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
44 hoursFix mounting in proxy mode in virtual mediaHEADmasterBoleslaw Ogonczyk Makowski1-4/+12
Broken by [1], interface and path were incorrect after changing getManagedObjects to getAllProperties. Tested: Mounting in proxy mode works [1]: https://github.com/openbmc/bmcweb/commit/e4b32753a7ce7ca436bf751f390ee01f02b9efd5 Change-Id: I06f36e8fc864fe13200d5d13a12639ebba9d9be1 Signed-off-by: Boleslaw Ogonczyk Makowski <boleslawx.ogonczyk-makowski@intel.com>
3 daysCheck sizeEd Tanous1-3/+4
Static analysis flags that we're ignoring a return value here. Use it. Tested: Code compiles. Change-Id: I2b37286b5a7b549b483ed5669fa0c24a628adc98 Signed-off-by: Ed Tanous <ed@tanous.net>
5 daysMove under exception handlerEd Tanous1-1/+2
Static analysis still sometimes flags that this throws, even through clang-tidy doesn't. Move it under the exception handler. Tested: Logging still works. Change-Id: I67425749b97b0a259746840c7b9a9b4834dfe52e Signed-off-by: Ed Tanous <ed@tanous.net>
5 daysLast fix for inversionEd Tanous1-1/+1
CSRF option got inverted. Fix it. Tested: Code compiles. Change-Id: Ibfc56ef2ce8d065aa7dad836e3d4a5edc5632926 Signed-off-by: Ed Tanous <ed@tanous.net>
8 daysupdateservice: refactor task creationJagpal Singh Gill1-119/+117
Refactor task creation into a separate function so it can be used from different places in code. The new usage of this function will be from start update interface based flow. More details refer to - https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738 https://gerrit.openbmc.org/c/openbmc/docs/+/65739 Tested: Firmware update using curl. Change-Id: I5e8a0ab98f49657178ee733fa4d34fbf40a7b1f3 Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
8 daysupdateservice: refactor parse multi-part formsJagpal Singh Gill1-33/+72
Refactor parsing multi-part forms into a different function for using it in legacy vs start update flows. More more details refer to - https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738 https://gerrit.openbmc.org/c/openbmc/docs/+/65739 Tested: Used curl to test firmware update. Change-Id: I2ec1ec9f4ac04349a1fbd588664f2d51bae827ea Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
8 daysRemove reserveEd Tanous1-5/+0
This is flagging static analysis issues, because we don't reserve a quadraticly sized increment. This is a micro optimization that never made much sense. Tested: Old code. Inspection only. Change-Id: I442628751558616c24123dba66aab448a4c24039 Signed-off-by: Ed Tanous <ed@tanous.net>
9 daysFix lesser used optionsEd Tanous5-75/+72
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we had ifndef instead of ifdef. because these weren't the defaults, these don't show up as failures when testing. Tested: Redfish service validator passes. Inspection primarily. Mechanical change. Change-Id: I3f6915a97eb44d071795aed76476c6bee7e8ed27 Signed-off-by: Ed Tanous <ed@tanous.net>
10 daysFix merge conflict in sessionEd Tanous1-0/+1
9f217c26f58c0a99c18e7cac7b095dcf6068562d had a merge conflict that was resolved incorrectly with 25b54dba775b31021a3a4677eb79e9771bcb97f7 The line returning the cookie session got dropped in that merge conflict. Restore it. Tested: Webui can log in, and cookie auth works again. Ideally in the future we'd have some tests for places like this where we've gone outside the redfish spec. Change-Id: I7740e19dac4d0dae5c5c9b27a5b8699a4751fd6f Signed-off-by: Ed Tanous <ed@tanous.net>
10 daysFix TFTP merge conflictEd Tanous1-5/+6
Static analysis now shows this code as "dead", even though it's not. This is a merge conflict that was handled wrong. Tested: inspection only. Suspect TFTP will now work. Change-Id: I51e52d62c51b251baf4c6ae74b100c1eda95603d Signed-off-by: Ed Tanous <ed@tanous.net>
10 daysstd::remove to std::filesystem::removeEd Tanous1-1/+3
Change-Id: Ie7729554e438f3af9ddb58941297525c50c6b419 Signed-off-by: Ed Tanous <ed@tanous.net>
10 daysCheck return codesEd Tanous1-2/+10
Static analysis finds these two places that we don't check error codes. Check them. Tested: Deprecated code. Inspection only. Change-Id: I92c238c5a4b1f51c5c6855c59f4f943855c4e50f Signed-off-by: Ed Tanous <ed@tanous.net>
11 daysAdd https parsingEd Tanous2-2/+83
This is yet another step in parsing HTTP requests. Tested: ''' curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" \ -X POST https://192.168.7.2/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate \ -d '{ \ "TransferProtocol":"TFTP", \ "ImageURI":"https://192.168.7.1/myfile.bin" \ }' ''' Returns ActionParameterNotSupported TransferProtocol: Omitted ImageURI: https://192.168.7.1/myfile.bin Returns ActionParameterNotSupported TransferProtocol: Omitted ImageURI: 192.168.7.1/myfile.bin Returns ActionParameterValueTypeError TransferProtocol: Bad ImageURI: https:/192.168.7.1/myfile.bin Returns: ActionParameterNotSupported No changes to GET requests, so Redfish Service Validator not necessary. Change-Id: Ibf4b69877031f3b8617412c06d40f2d0d0827ac3 Signed-off-by: Ed Tanous <ed@tanous.net>
12 daysMove logging argsEd Tanous6-14/+32
Args captured by logging functions should be captured by rvalue, and use std::forward to get perfect forwarding. In addition, separate out the various std::out lines. While we're here, also try to optimize a little. We should ideally be writing each log line to the output once, and ideally not use iostreams, which induce a lot of overhead. Similar to spdlog[1] (which at one point this codebase used), construct the string, then call fwrite and fflush once, rather than calling std::cout repeatedly. Now that we don't have a dependency on iostreams anymore, we can remove it from the places where it has snuck in. Tested: Logging still functions as before. Logs present. [1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13 Change-Id: I1dd4739e06eb506d68989a066d122109b71b92cd Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-04obmc_console: Fix some missing headersGunnar Mills1-0/+6
Fix some obvious missed headers. Tested: It builds, no other testing. Change-Id: I8cfd95e16eca38c75dcf76fc974ed384d13c6757 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-05-03Delete IPv4 default gateway when deleting an IPv4 static addressJohnathan Mantey1-49/+117
The Redfish schema for creating static IPv4 addresses requires the IP address, the netmask, and a gateway IP address. There's an issue inherent with this method. A network interface is only permitted a single IPv4 default gateway. If more than one IPv4 static address is assigned to the NIC each entry is processed, and potentially conflicting default gateways may be assigned. The last entry processed assigns the IPv4 default gateway. This behavior will cause unexpected results. It is necessary to prevent assigning mismatched default gateway values. The IPv4 address removal process requires additional work also. The default gateway value is left in place even after the final static IPv4 address is removed. It is necessary to perform an additional action to clear the gateway address. Without explicit removal the network is left in a condition that may prevent IP traffic from being able to be sent from the BMC. This even in the event that the NIC is actively being managed via DHCPv4. Tested: Disabled DHCPv4 on a secondary NIC (eth1) Assigned a static IPv4 address. Inspected the systemd-networkd config file in order to confirm the Gateway entry is added. This is done to be explicitly sure the network.config file has the Gateway entry. Sent a Redfish PATCH command to delete the static IPv4 address. Confirmed that the systemd-networkd config file no longer contained a Gateway entry. This is done to be explicitly sure the network.config file no longer contains the Gateway entry. Created a PATCH containing multiple IPv4 static addresses all with different Gateway values. Confirmed an error is returned when a mismatch occurs in the Gateway values. Assigned a new static address, and then restored DHCPv4. Confirmed that the default gateway entry in the config file is removed. Submitted a delete request for the remaining static IPv4 address that is now orphaned by re-enabling DHCPv4. This removed the static IPv4 address. Change-Id: Ia12cf2a38ba86266ce71dc28475b0d07b7e09ebc Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
2024-05-03Manage Request with shared_ptrJonathan Doman11-109/+121
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use. Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON. Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-03Implement client certificate schemasEd Tanous3-1/+160
The Redfish standard seems to have caught up with some of the OEM schemas and features we already have, namely MutualTLS and Basic Auth disablement. This commit implements most of the GET parameters for which we already have backends. ClientCertificate is pointed to the same resources as TrustStore. Tested: generate_auth_certificates.py succeeds, and shows a certificate in ClientCertificate collection Get AccountService, and ClientAuthentication/Certificates returns expected values. Redfish service validator passes. Change-Id: If18e34e9dfa8f38293fceff288596811afd16d4a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-03updateservice: add start-update meson optionJagpal Singh Gill3-0/+16
Add the meson option for the start-update D-Bus interface feature to be used in UpdateService. More more details refer to - https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738 https://gerrit.openbmc.org/c/openbmc/docs/+/65739 Tested: Build passes. Change-Id: I594ddc0d2df6c032823eaeba9429cf50047d5dcd Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
2024-05-03Increase the file bufferEd Tanous1-1/+5
When we added file buffer, this number was picked arbitrarily. Prior to the file body patch series, files were buffered entirely in ram, regardless of what size they were. While not doing that was an improvement, I suspect that we were overly conservative in the buffer size. Nginx picks a default buffer size somewhere in the 8k - 64k range dependent on what paths the code takes. Using the higher end of that range seems like a better starting point, but generally we have more ram on the bmc than we have users. Increase the buffer to 64K. Tested: Unit tests pass. [1] https://docs.nginx.com/nginx-management-suite/acm/how-to/policies/http-backend-configuration/#buffers Change-Id: Idb472ccae02a8519c0976aab07b45562e327ce9b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-02Fix regression in CrashDumpGunnar Mills1-1/+1
25b54db introduced a bug where CrashDump was not looking at the correct option. Was using BMCWEB_REDFISH_DUMP_LOG instead of the correct BMCWEB_REDFISH_CPU_LOG. This was caught in CI by a system that doesn't have CrashDump enabled but was hitting: 1 failGet errors in /redfish/v1/Systems/system/LogServices/Crashdump Tested: None. Visually inspected and this matches redfish-core/src/redfish.cpp. Change-Id: Ia6e72e5bbeaaa246fbbc5bcb2a525062e63d7d29 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-05-02Fix regression in LogServiceEd Tanous1-1/+1
25b54db introduced an inadvertent inversion in options. Admittedly, this option (redfish-dbus-entries) chose to override URLs instead of creating new ones, which makes it incompatible. Normally we'd require unique URIs for each entry to make this error not possible, and I believe that this is the only instance of us registering two handlers for one url/verb. Tested: Romulus now boots and functions in qemu. GET /redfish/v1 returns results. Change-Id: I6125a5a0242b6cfc54ff19866665227c97f45d0a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-01Bring consistency to config optionsEd Tanous35-640/+646
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options using a config file. This history has led to a lot of different ways to configure code in the codebase itself, which has led to problems, and issues in consistency. ifdef options do no compile time checking of code not within the branch. This is good when you have optional dependencies, but not great when you're trying to ensure both options compile. This commit moves all internal configuration options to: 1. A namespace called bmcweb 2. A naming scheme matching the meson option. hyphens are replaced with underscores, and the option is uppercased. This consistent transform allows matching up option keys with their code counterparts, without naming changes. 3. All options are bool true = enabled, and any options with _ENABLED or _DISABLED postfixes have those postfixes removed. (note, there are still some options with disable in the name, those are left as-is) 4. All options are now constexpr booleans, without an explicit compare. To accomplish this, unfortunately an option list in config/meson.build is required, given that meson doesn't provide a way to dump all options, as is a manual entry in bmcweb_config.h.in, in addition to the meson_options. This obsoletes the map in the main meson.build, which helps some of the complexity. Now that we've done this, we have some rules that will be documented. 1. Runtime behavior changes should be added as a constexpr bool to bmcweb_config.h 2. Options that require optionally pulling in a dependency shall use an ifdef, defined in the primary meson.build. (note, there are no options that currently meet this class, but it's included for completeness.) Note, that this consolidation means that at configure time, all options are printed. This is a good thing and allows direct comparison of configs in log files. Tested: Code compiles Server boots, and shows options configured in the default build. (HTTPS, log level, etc) Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Remove One chassis optionEd Tanous1-2/+0
The One chassis option has been gone for a long time, but this ifdef looks like it got missed. Remove it. Tested: code compiles. Change-Id: I013e824806e72bc608ae4383ce4ba707641aeec6 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Use fadvise to trigger sequential readingEd Tanous1-0/+8
Nginx and other webservers use fadvise to inform the kernel of in-order reading. We should to the same. Tested: Webui loads correctly, no direct performance benefits immediately obvious, but likely would operate better under load. Change-Id: I4acce316c719df7df012cea8cb89237b28932c15 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Fix bad merge conflictEd Tanous1-11/+2
nbd proxy and vm websocket options got reversed in 36c0f2a35e670a4b798b7b42fd18455085e9d9c0 Change them back. Change-Id: I7c54e66f88aee956bd20f2139d110e64998a4ef5 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Break out lambdas into real methodsEd Tanous1-156/+144
Tested: Need help Change-Id: I28cc1626212ec746b5345490ec285706eb386e65 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Consolidate Vm implementationsEd Tanous8-434/+450
As much as the two vm implementations SEEM different, the differences largely lie in how we're getting the nbd proxy socket. One is relying on launching a process (nbd-proxy), the other is getting the fd from dbus. Given [1] exists and is in process, we need to have a plan for getting these two VM implementations into one, once that patchset is complete. This commit: Splits the vm-websocket option into vm-websocket-provider, providing two options, nbd-proxy, and virtual-media (the names of the respective apps). To accomplish this, it moves the contents of nbd-proxy into include/vm-websocket, so we can compare the similarities and start consolidating. The longer term intent is that the nbd-proxy option will be completely removed, and the code deleted. This has the additional advantage that we will no longer require the boost::process dependency, as all info will be available on dbus. As part of this, the nbd proxy websocket is also registered at /vm/0/0, to be backward compatible with the old interfaces. Tested: Code compiles. Need some help here. [1] https://gerrit.openbmc.org/c/openbmc/jsnbd/+/49944 Change-Id: Iedbca169ea40d45a8775f843792b874a248bb594 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-29Break out formattersEd Tanous9-91/+109
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majority of boost::url, and nlohmann::json as includes. This commit breaks out boost and json formatters into their own separate includes. Tested: Code compiles. Logging changes only. Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-29Use actual language for registries listAlexander Paramonov1-2/+2
Use language from header for registries list instead of hardcoded 'en'. Language from header is already being used in some parts of code[1], but is hardcoded sometimes. This commit fixes inconsistency. TESTED: current language in header with Redfish definition is now consistently taken into account. [1] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/70741/1/redfish-core/lib/message_registries.hpp#b214 Change-Id: Ic5e8e5e76d171b1cb18953e5602f09132b222f3b Signed-off-by: Alexander Paramonov <Sasha110397@gmail.com>
2024-04-27Add static webpack etag supportEd Tanous4-15/+90
Webpack (which is what vue uses to compress its HTML) is capable of generating hashes of files when it produces the dist files[1]. This gets generated in the form of <filename>.<hash>.<extension> This commit attempts to detect these patterns, and enable etag caching to speed up webui load times. It detects these patterns, grabs the hash for the file, and returns it in the Etag header[2]. The behavior is implemented such that: If the file has an etag, the etag header is returned. If the request has an If-None-Match header, and that header matches, only 304 is returned. Tested: Tests were run on qemu S7106 bmcweb with default error logging level, and HTTP/2 enabled, along with svg optimization patches. Run scripts/generate_auth_certificate.py to set up TLS certificates. (valid TLS certs are required for HTTP caching to work properly in some browsers). Load the webui. Note that DOM load takes 1.10 seconds, Load takes 1.10 seconds, and all requests return 200 OK. Refresh the GUI. Note that most resources now return 304, and DOM time is reduced to 279 milliseconds and load is reduced to 280 milliseconds. DOM load (which is what the BMC has control over) is decreased by a factor of 3-4X. Setting chrome to "Fast 5g" throttling in the network tab shows a more pronounced difference, 1.28S load time vs 3.96S. BMC also shows 477KB transferred on the wire, versus 2.3KB transferred on the wire. This has the potential to significantly reduce the load on the BMC when the webui refreshes. [1] https://webpack.js.org/guides/caching/ [2] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag Change-Id: I68aa7ef75533506d98e8fce10bb04a494dc49669 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-27Move to process v2Ed Tanous4-28/+40
Boost process v2 brings some significant benefits to our launching of processes[1]. In bmcweb terms: 1. The code is radically simpler, which decreaeses compile times, and reduces the scope for code scanning tools. 2. The code now uses standard asio pipes instead of inventing its own. 3. Separate compilation. Tested: We don't have a lot of unit tests for the virtual media stuff that I can run, but we do have unit tests for credentials pipe, which in this change have been ported over, so the feature works. Unit tests are passing. [1] https://www.boost.org/doc/libs/1_80_0/doc/html/boost_process/v2.html#boost_process.v2.introduction Change-Id: Ia20226819d75ff6e492f8852185f0b73e8f5cf83 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-26Make cookie auth check all headersEd Tanous1-43/+48
Currently, the Cookie auth only checks the first cookie header in a request. This works fine for most things, because a lot of implementations (browsers) seem to either put the Cookie headers in alphabetical order, or put them in the order in which they were stored which in the case of bmcweb, is also alphabetical. Well, http2 blows this up, because cookies could potentially be in any order, given the hpack compression techniques, so there's no promise that Cookie[0] is the Session cookie. This commit reworks the authentication code to call beasts "equal_range" getter, which returns the range of all headers that matched. This allows us to attempt to parse the cookies in whatever order they might have been received. The auth routine only tries to log in the first cookie matching SESSION=, and do not try to handle duplicates, as this might allow attackers to negate the anti brute force measures by testing multiple passwords at once Tested: With http2 enabled, the UI can now log in more consistently, and in addition, the HTML redfish pages function more consistently when using cookie auth. Redfish service validator passes. Change-Id: I3a61a5a654f62096ff19cfbfaf0a10f30a1a3605 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-25Remove setup.cfg file from the repositoryManojkiran Eda1-1/+0
OpenBMC CI has migrated from using `pycodestyle` to `flake8` as part of this[1] commit. Unlike `pycodestyle` , `flake8` does not rely on the presence of setup.cfg file in their root path as a trigger, but it runs on all repositories by default. Hence there is no need of having setup.cfg file, so removing it from the repository. [1]: https://github.com/openbmc/openbmc-build-scripts/commit/c5ad7ff440cfd94fc025efbd45a3859475b18820 Change-Id: I3f38138d0cbf96a3248e2baf2ed59a816a043b68 Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
2024-04-25AccountService: Add HTTPBasicAuth supportRavi Teja2-3/+31
This commit adds HTTPBasicAuth Get/Patch support Tested By: Redfish service validator passes. ``` curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HTTPBasicAuth":"Enabled"}' https://192.168.7.2/redfish/v1/AccountService ``` Succeeds with various values. Enabled: Basic auth succeeds. Disabled: Basic auth no longer works. AccountService reports "Disabled" For HTTPBasicAuth status. Change-Id: Ic417bf3cd4135f05ab34c8613c7fbce953157b03 Signed-off-by: Ravi Teja <raviteja28031990@gmail.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-24Handle error code properlyEd Tanous1-5/+6
As part of the previous patch tests, UpdateService shows this bug, where on a multipart parser failure, the dbus match object gets instantiated, and eventually fails. This leads to mediocre logging, and possibly could leave update service in an undesirable state. Fix the error by moving the conditional up. Tested: Filling a 16MB file with all zeros and sending it now no longer logs that a monitor has been set up, and returns immediately instead of waiting for timeout. ``` dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":[]} ;type=application/json' -H "Expect:" --user "root:0penBmc" -F UpdateFile=@16mb.txt -v ``` Change-Id: I0962d15c624936b4fa199a675123702003dd697b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-24Fix large content error codesEd Tanous1-88/+203
When pushing multi-part payloads, it's quite helpful if the server supports the header field of "Expect: 100-Continue". What this does, is on a large file push, allows the server to possibly reject a request before the payload is actually sent, thereby saving bandwidth, and giving the user more information. Bmcweb, since commit 3909dc82a003893812f598434d6c4558107afa28 by James (merged July 2020) has simply closed the connection if a user attempts to send too much data, thereby making the bmcweb implementation simpler. Unfortunately, to a security tester, this has the appearance on the network as a crash, which will likely then get filed as a "verify this isn't failing" bug. In addition, the default args on curl multipart upload enable the Expect: 100-Continue behavior, so folks testing must've just been disabling that behavior. Bmcweb should just support the right thing here. Unfortunately, closing a connection uncleanly is easy. Closing a connection cleanly is difficult. This requires a pretty large refactor of the http connection class to accomplish. Tested: Create files of various size and try to send them (Note, default body limit is 30 MB) and upload them with an without a username. ``` dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' -F UpdateFile=@32mb.txt -v ``` No Username: 32MB returns < HTTP/1.1 413 Payload Too Large 16MB returns < HTTP/1.1 401 Unauthorized With Username 32MB returns < HTTP/1.1 413 Payload Too Large 16MB returns < HTTP/1.1 400 Bad Request Note, in all cases except the last one, the payload is never sent from curl. Redfish protocol validator fails no new tests (SSE failure still present). Redfish service validator passes. Change-Id: I72bc8bbc49a05555c31dc7209292f846ec411d43 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-24Add new tidy checksEd Tanous1-1/+27
Another clang, another list of checks we can enable. Tested: Clang-tidy passes. Change-Id: Ib3fcd8046ce9f2caf9f9d95571ffa3cc2f56c596 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-23Fix http2 use after free bugEd Tanous1-2/+2
In the below code, we move out of Response, then use it to set unauthorized, which never gets returned to the user. This results in the browser showing an empty 200 ok request, because while the request was propagated rejected, the 401 error code didn't get propagated to the user. Tested: If not logged in on a chrome browser: /redfish/v1 -> Returns the UI /refish/v1/AccountService -> returns a forward to the webui login page. If logged into the webui. /redfish/v1/AccountService now returns the expected HTML redfish representation of the json response. Change-Id: I2c906f818367ebb253b3e6097e6787ba4c215e0a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-23Remove XSS prevention codeEd Tanous6-83/+14
This feature was created for a time before webpack had a built in proxy, and to debug the UI required setting specific flags. The webpack proxy solves this problem in a much better way, by proxying everything. This commit is one piece in the solving a use after free bug. Removing this allows us to no longer have to cache the origin header [1], which is only used in this mode. Tested: Code compiles. [1] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/70850 Change-Id: I01d67006e217c0c9fd2db7526c0ec34b0da068f3 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-23Implement a Content-Security-Policy TODOEd Tanous3-115/+93
This TODO has been in bmcweb for a very long time. Implement it. W3 sets rules for what security policies apply to which content types[1]. Reading through this, essentially CSP should only apply to HTML files. Tested: Unit tests pass. Webui loads properly. Chrome network window Shows headers show up as expected. [1] https://www.w3.org/TR/CSP2/#which-policy-applies Change-Id: I5467d0373832668763c72a66da2a8872e07bfb58 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-20Change ssl stream implementationsEd Tanous8-18/+17
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to be optimized in SSL. bmcweb never writes one or two bytes, we almost always write the full payload of what we received, so there's no reason to take the binary size overhead, and additional boost headers that this implementation requires. Tested: This drops the on-target binary size by 2.6% Redfish service validator passes. Change-Id: Ie1ae6f197f8e5ed70cf4abc6be9b1b382c42d64d Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-19Add missing headersEd Tanous22-21/+37
Most of these were found by breaking every redfish class handler into its own compile unit: When that's done, these missing headers become compile errors. We should just fix them. In addition, this allows us to enable automatic header checking in clang-tidy using misc-header-cleaner. Because the compiler can now "see" all the defines, it no longer tries to remove headers that it thinks are unused. [1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac Tested: Code compiles. Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Update nghttp2 1.60->1.61Ed Tanous2-6/+11
They seem to have gotten rid of the nghttp2_static target, so go back to relying on the "normal" target. Change-Id: Ic44d9ffe5fa2d88f38c018756738197371b0dc89 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Remove ibm locks featureSunitha Harish6-1274/+1
This feature was introduced to manage the operation sync at BMC while multiple clients manage the BMC. This feature scope has gone away and it is not a simple code to maintain as per the growing standards of bmcweb. This commit removes the feature from this repo. Tested by: Locks routes are not available anymore Change-Id: I257225cfb1f43d7d5dadb21a28a2ee5345c5112a Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Clean up BMCWEB_ENABLE_SSLEd Tanous10-130/+77
This macro came originally from CROW_ENABLE_SSL, and was used as a macro to optionally compile without openssl being required. OpenSSL has been pulled into many other dependencies, and has been functionally required to be included for a long time, so there's no reason to hold onto this macro. Remove most uses of the macro, and for the couple functional places the macro is used, transition to a constexpr if to enable the TLS paths. This allows a large simplification of code in some places. Tested: Redfish service validator passes. Change-Id: Iebd46a68e5e417b6031479e24be3c21bef782f4c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Initialize schemas array with explicit sizeMyung Bae2-2/+7
Currently `update_schemas.py` generates a schema list definition like redfish-core/include/schemas.hpp: ``` constexpr std::array schemas { "AccountService", "ActionInfo", ... "OpenBMCAccountService", }; ``` However, if the number of schemas is more than the clang's default max size, CI may fail. The default is `-fbracket-depth=256`. ``` In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/functional:65: [1m/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/array:288:52: [0m [0;1;31mfatal error: instantiating fold expression with 276 arguments exceeded expression nesting limit of 256 288 | -> array<enable_if_t<(is_same_v<_Tp, _Up> && ...), _Tp>, [0m | [0;1;32m ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ [0m [1m../redfish-core/include/schemas.hpp:17:26: [0m [0;1;30mnote: while substituting deduced template arguments into function template '<deduction guide for array>' [with _Tp = const char *, _Up = <const char *, const char *, const char *, const char *, const char *, ... const char *>] [0m 17 | constexpr std::array schemas { [0m | [0;1;32m ^ [0m1 error generated. ``` To avoid the failure, we can set the size explicitly like ``` constexpr std::array<std::string_view,277> schemas { "AccountService", ... ``` Tested: 1) Remove `include_list` so that all possible schemas are to be used 2) Run with the fixed `scripts/update_schemas.py` 3) Compiles successfully Change-Id: Ib68db9fd3be1b6dbe0c4b5cc0e9a4324966d759e Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-04-17Reformat meson filesEd Tanous3-364/+389
Meson has a style guide, we should follow it. This includes: 4 space indents on new scopes. Trailing commas on the final entry in a list/array Tested: Whitespace only. Code compiles. Change-Id: Ib7f96a2bd722b55410818c766c0261f5d44cb84d Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-17Refactor tftp parserEd Tanous2-48/+86
This function in the next patch will be used for more than just TFTP, so rename it to match intent, and refactor to use non-TFTP specific types. Tested: Rename only. Need help on TFTP setups if we need it. Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-17Break out DoTftpUpdateEd Tanous1-34/+44
This refactor of code is in preparation for adding new SimpleUpdate types. Separating out TFTP helps to keep code organized. Tested: Need help here. TFTP isn't enabled a lot. Change-Id: Ifbdd4b73bb0f9c31092d729d1ec3d3f395f680b8 Signed-off-by: Ed Tanous <ed@tanous.net>