| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In the past, we've tried to erradicate the use of
nlohmann::json(initiatlizer_list<...>) because it bloats binary sizes,
as every type is given a new nlohmann constructor.
This commit hunts down the last few places where we call this. There is
still 2 remaining in openbmc_dbus_rest after this, but those are variant
accesses that are difficult to triage, and considering it's a less used
api, they're left as is.
Tested: WIP
Change-Id: Iaac24584bb78bb238da69010b511c1d598bd38bc
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Static analysis flags that we're not checking certs here. Make it
optional, and default to verification.
Tested: No backend available. Commands parse on command line, and fail
in redfish.
Change-Id: I7558c55b5f1e1695844b986b5587561d1391f245
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
It was reported that ipv6 URLS failed to parse now. This test seems
useful to have.
Tested: Unit tests pass.
Change-Id: I21f1b11039c1b35dff5c85ecab784dd3cd6228dc
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Add the start update D-Bus interface based flow for multi-form content
path. This involves mapping the TargetURI to the corresponding
serviceName and objectPath which hosts the specific D-Bus interface.
As per discussion with Redfish community both ResourceURI and
FirmwareInventory Redfish URI can be used as TargetURI. Current
implementation already allows /redfish/v1/Managers/<bmc>, hence support
for this specific ResourceURI has been preserved. New implementation
adds FirmwareInventory Redfish URI for TargetURI as default option.
https://redfishforum.com/thread/1054.
For more details on design refer to -
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738
https://gerrit.openbmc.org/c/openbmc/docs/+/65739
Tested: Redfish Validator and Build passes.
multipart form data update request with Resource URI as target
```
curl -k -H "X-Auth-Token: $token" -H "Content-Type:multipart/form-data" \
-X POST -F UpdateParameters="{\"Targets\":[\"/redfish/v1/Managers/bmc\"],\"@Redfish.OperationApplyTime\":\"Immediate\"};type=application/json" \
-F "UpdateFile=@obmc-phosphor-image-romulus-20240425222313.static.mtd.all.tar;type=application/octet-stream" \
https://${bmc}/redfish/v1/UpdateService/update
{
"@odata.id": "/redfish/v1/TaskService/Tasks/0",
"@odata.type": "#Task.v1_4_3.Task",
"Id": "0",
"TaskState": "Running",
"TaskStatus": "OK"
}
```
multipart form data update request with Firmware Inventory URI as target
```
curl -k -H "X-Auth-Token: $token" -H "Content-Type:multipart/form-data" \
-X POST -F UpdateParameters="{\"Targets\":[\"/redfish/v1/Managers/bmc\"],\"@Redfish.OperationApplyTime\":\"Immediate\"};type=application/json" \
-F "UpdateFile=@obmc-phosphor-image-romulus-20240509003505.static.mtd.all.tar;type=application/octet-stream" \
https://${bmc}/redfish/v1/UpdateService/update
{
"@odata.id": "/redfish/v1/TaskService/Tasks/1",
"@odata.type": "#Task.v1_4_3.Task",
"Id": "1",
"TaskState": "Running",
"TaskStatus": "OK"
}
```
Change-Id: Id46de79d3af8834630a793678a6fc0e859295afe
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
|
|
Response::openFd was added recently to allow handlers to pass in a file
descriptor to be used to read. This worked great for files, but had
some trouble with unix sockets. First, unix sockets have no known
length that we can get. They are fed by another client until that
client decides to stop sending data and sends an EOF. HTTP in general
needs to set the Content-Length header before starting a reply, so the
previous code just passes an error back.
HTTP has a concept of HTTP chunking, where a payload might not have a
known size, but can still be downloaded in chunks. Beast has handling
for this that we can enable that just deals with this at the protocol
layer silently. This patch enables that.
In addition, a unix socket very likely might not have data and will
block on the read call. Blocking in an async reactor is bad, and
especially bad when you don't know how large a payload is to be
expected, so it's possible those bytes will never come. This commit
sets all FDs into O_NONBLOCK[1] mode when they're sent to a response,
then handles the subsequent EWOULDBLOCK and EAGAIN messages when beast
propagates them to the http connection class. When these messages are
received, the doWrite loop is simply re-executed directly, attempting to
read from the socket again. For "slow" unix sockets, this very likely
results in some wasted cycles where we read 0 bytes from the socket, so
shouldn't be used for eventing purposes, given that bmcweb is
essentially in a spin loop while waiting for data, but given that this
is generally used for handling chunking of large payloads being
generated, and while spinning, other reactor operations can still
progress, this seems like a reasonable compromise.
[1] https://www.gnu.org/software/libc/manual/html_node/Open_002dtime-Flags.html
Tested:
The next patch in this series includes an example of explicitly adding a
unix socket as a response target, using the CredentialsPipe that bmcweb
already has. When this handler is present, curl shows the response
data, including the newlines (when dumped to a file)
```
curl -vvvv -k --user "root:0penBmc" https://192.168.7.2/testpipe -o output.txt
```
Loading the webui works as expected, logging in produces the overview
page as expected, and network console shows no failed requests.
Redfish service validator passes.
Change-Id: I8bd8586ae138f5b55033b78df95c798aa1d014db
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In the initial implementation of metadata indexing the bmc knew at
compile time what schemas it could potentially publish. bmcweb took the
approach of adding all schemas of all versions to the $metadata
resource. Since that was made, two major changes have happened.
First, Redfish has added significantly more versions of each schema, as
well as significantly more schemas to the point where the metadata index
is now 213KB. While this file compresses fairly well, the size is
obvious from the large amount of time that redfish service validator
takes to parse the schemas, compared to actually acquiring BMC redfish
resources.
Second, aggregation was added, where an aggregated Redfish service might
implement any number of schemas, including OEM ones.
In an effort to fix this, this patch takes the compile-time algorithm in
update_schemas.py, and moves it into bmcweb itself, parsing the files on
disk as needed on demand. This has some immediate benefits; First, is
that now schemas can be potentially installed from anywhere, not only
from within the bmcweb build, and they will be resolved at runtime.
Second, patches that want to add support for a given schema need to only
symlink the schema into the correct folder, without needing to rerun
update_schemas.py. This saves time in review.
Finally, this opens to door to reducing the schema versions present in
the metadata to the unique set of only what this bmcweb instance, and
its aggregated BMCs expose.
Tested: Redfish service validator passes. Need A/B checking to verify
the file is byte for byte the same.
GET /redfish/v1/$metadata returns what looks like sane results, with a
correct content-type.
Unit tests require the use of TemporaryFileHandle, so that class is
moved into a more general folder, outside of test/http.
Change-Id: I326159099c6b6c4056023b2e173c5f074ed88ce1
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
For OpenBMC owned schemas, let prettier handle the formatting for us,
given that OpenBMC owns them.
Change-Id: If9558dae8a34fed72b926c2dd95fcff887c8119c
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Now that we have the schemas moved, add symlinks back to the schemas
that we use, so that they will be installed correctly.
The meson option of follow_symlinks: true is added to suppress a
warning about potential change in behavior in meson in the future.
Change-Id: Ie24536ca04038d8137818c201d9411b95361b14f
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
We should check in all the DMTF schemas, even if we don't use them, such
that adding or removing them is simpler, and we can keep an accounting
of changes to overall DMTF schemas, not mixed in with our selections.
Change-Id: I893215ac68829bce8e8e1cf1a9e5499a32abe119
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Reorganize the existing schemas into folders under redfish core.
The existing schema system has some problems:
1. It's hard to add new schemas
2. We have to rerun the script any time we want to change what schemas
we use.
3. Adding schemas optionally takes effort
In an effort to combat this, this patchset moves all the existing
schemas into folders that represent their namespace names
dmtf/csdl represents the CSDL that dmtf publishes
oem/openbmc represents the CSDL that OpenBMC publishes
In theory, this means that in the future we can relax OEM_SCHEMAS.md,
and allow folks to possibly implement their own schemas in a way that
doesn't have to effect all other systems.
This also has the advantage of not requiring changes to
update_schemas.py when we want to add, remove, or modify what version of
a schema we use. "current" schemas are just symlinks, so they can be
updated using git, and not necessarily have merge conflicts with one
another.
Tested: Redfish service validator passes.
Change-Id: I6d4a130bba4cb874ef00a06ed579cc67f53dc7ae
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Removing the IPv4 default gateway doesn't work correctly when only a
single static address has been assigned. This is expected to be the
common mode of operation, and needs to work correctly.
When more than one static address is managed it's necessary to
preserve the existing gateway. If any address is left unmodified,
added, or is modified the gateway must be preserved.
Tested:
Turned off DHCPv4, and assigned a single static address
Sent a PATCH null to delete the address.
Confirmed the default gateway got cleared.
Assigned two static addresses.
Sent a PATCH {}, null
Sent PATCH null
Confirmed expected default gateway handling
Assigned two static addresses.
Sent a PATCH null, {}
Sent PATCH null
Confirmed expected default gateway handling
Change-Id: I85c4a0533f9468b424602aeb636b8f4f218a9a13
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
|
|
When multiple SSE sockets are present, this code has a potential to
crash, given the vector is being modified in this loop.
Tested:
Reproduction of this is not reliable. Inspection only.
Change-Id: I66d4b55b5c0ad46d41d34dd6558b50c5f6fcfc12
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
These includes seem to have snuck in. In theory nothing in redfish
should be taking a #include in anything in openbmc-rest.
Tested: Code compiles
Change-Id: Ifec2a9b18f296870f67b15f98fc44c67050e9e28
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In the early days of bmcweb, we made two pretty critical assumptions;
First, is that a given platform would only have a single BMC instance
(represented as "bmc") and a single host instance (represented as
"system").
Second we assumed that, given that Redfish suggests against hardcoding
URIs in client implementation and leaves them freeform, clients would
code to the standard.
Our own webui-vue hardcodes Redfish URIs [1], and the documentation is
littered with examples of hardcoded curl examples of hardcoding these
URIs. That bug was filed in 2020, and the issue has only gotten worse
over time.
This patchset is an attempt to give a target that we can start solving
these issues, without trying to boil the ocean and fix all clients in
parallel.
This commit adds the meson options
redfish-manager-uri-name
and
redfish-system-uri-name
These are used to control the "name" that bmcweb places in the fixed
locations in the ManagerCollection and ComputerSystemCollection schemas.
Note, managers is added, but is not currently testable. It will be
iterated on over time.
Tested:
Changed the URL options to "edsbmc" and "edssystem" in meson options.
Redfish service validator passes.
URLs appear changed when walking the tree.
[1] https://github.com/openbmc/webui-vue/issues/43
Change-Id: I4b44685067051512bd065da8c2e3db68ae5ce23a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Ideally, we should only have to declare meson options in one place, in
the meson_options.txt, and have all files generate from that list. This
patchset gets us one step closer, where we have to list options in 2
places, whereas previously it was 3.
Tested:
Code compiles. Manual inspection of generated bmcweb_config.h shows
only minor differences in generated file.
Change-Id: I4a4f863c95463e3cdf8b629de5a0a73f74cf001e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
When a user's password is changed, existing Redfish sessions for that
user, created with the old password, continue to work.
As per OWASP session management, "The session ID must be renewed or
regenerated by the web application after any privilege level change
within the associated user session... Common scenarios to consider
include; password changes, permission changes, or switching from a
regular user role to an administrator role within the web application."
[1] https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
This commit removes existing user sessions when the user's password is
changed. This commit leaves the current session in place though a new
removeSessionsByUsernameExceptSession().
This commit doesn't completely get us fully to what owasp says but is a
start.
Tested:
Create some users:
```
curl -k -v -X POST -H "Content-Type: application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/ -d \
'{"UserName":"testadminuser","Password":"<password>","RoleId":"Administrator","Enabled":true}'
```
Using basic auth was able to update own password and another user's
password.
Using token auth, verified the current session did not get deleted but
other sessions from that user did.
```
curl -k -H "Content-Type: application/json" -X POST -D headers.txt \
https://${bmc}/redfish/v1/SessionService/Sessions -d \
'{"UserName":"testadminuser", "Password":"<password>"}'
```
```
curl -k -v -X PATCH -H "X-Auth-Token: $token" \
-H "Content-Type:application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/testadminuser \
-d '{"Password":"<password>"}'
```
Verified when changing another user's password all sessions were
dropped.
Change-Id: I4de60b84964a6b29c021dc3a2bece9ed4bc09eac
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
|
|
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>
|
|
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>
|
|
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>
|
|
CSRF option got inverted. Fix it.
Tested: Code compiles.
Change-Id: Ibfc56ef2ce8d065aa7dad836e3d4a5edc5632926
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
Change-Id: Ie7729554e438f3af9ddb58941297525c50c6b419
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
Fix some obvious missed headers.
Tested: It builds, no other testing.
Change-Id: I8cfd95e16eca38c75dcf76fc974ed384d13c6757
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
nbd proxy and vm websocket options got reversed in
36c0f2a35e670a4b798b7b42fd18455085e9d9c0
Change them back.
Change-Id: I7c54e66f88aee956bd20f2139d110e64998a4ef5
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Tested: Need help
Change-Id: I28cc1626212ec746b5345490ec285706eb386e65
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
