Fix TFTP merge conflict

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.

Fix TFTP merge conflict

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>

show more ...

Add https parsing

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/Upd

Add https parsing

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>

show more ...

Delete IPv4 default gateway when deleting an IPv4 static address

The Redfish schema for creating static IPv4 addresses requires the IP
address, the netmask, and a gateway IP address. There's an issu

Delete IPv4 default gateway when deleting an IPv4 static address

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>

show more ...

Implement client certificate schemas

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

Implement client certificate schemas

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>

show more ...

Fix regression in CrashDump

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

Fix regression in CrashDump

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>

show more ...

Bring consistency to config options

The configuration options that exist in bmcweb are an amalgimation of
CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms
and meson options usi

Bring consistency to config options

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>

show more ...

Remove One chassis option

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: I013e824806e72bc608ae4383ce4b

Remove One chassis option

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>

show more ...

Consolidate Vm implementations

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 (nb

Consolidate Vm implementations

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>

show more ...

Use actual language for registries list

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

Use actual language for registries list

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>

show more ...

Move to process v2

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
redu

Move to process v2

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>

show more ...

AccountService: Add HTTPBasicAuth support

This commit adds HTTPBasicAuth Get/Patch support

Tested By:
Redfish service validator passes.
```
curl -k --user "root:0penBmc" -H "Content-Type: applicat

AccountService: Add HTTPBasicAuth support

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>

show more ...

Handle error code properly

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. Thi

Handle error code properly

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>

show more ...

Add missing headers

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 the

Add missing headers

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>

show more ...

Refactor tftp parser

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

Refactor tftp parser

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>

show more ...

Break out DoTftpUpdate

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 lo

Break out DoTftpUpdate

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>

show more ...

Refactor UpdateService

Similar to other refactorings we've been doing, make UpdateService call
into methods instead of inline lambdas.

Tested: Redfish service validator passes. Structural changes

Refactor UpdateService

Similar to other refactorings we've been doing, make UpdateService call
into methods instead of inline lambdas.

Tested: Redfish service validator passes. Structural changes only.

Change-Id: I96b6db5e14fa0f7d357fb0faf63d0457b7963581
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix NTP set race condition

There's currently a problem with phosphor-timesyncd, where enabling NTP
doesn't immediately reflect in the system status on return[1]. To say
it another way, NTP is not e

Fix NTP set race condition

There's currently a problem with phosphor-timesyncd, where enabling NTP
doesn't immediately reflect in the system status on return[1]. To say
it another way, NTP is not enabled/disabled atomically, which leads to
the following problem.

// Disable NTP
PATCH /redfish/v1/Managers/bmc/NetworkProtocol
{"NTP":{"ProtocolEnabled": false}}
// Set the time manually
PATCH /redfish/v1/Managers/bmc {"DateTime": "<timestring"}

Doing this in rapid succession leads to a 500 error, which is obviously
a bug. In the prior commit, this error was changed to a
PropertyValueConflict error, which is still incorrect, but at least
informative of what's going on. REST APIs are intended to have CRUD
compliance. The response should not be returned until the value has
been accepted, and not doing that can lead to problems.

This commit changes the backend to use systemd directly, rather than
routing through phosphor-settings, to avoid this race.

Quite possibly resolves #264 but haven't tested that.

Tested: The above procedure succeeds.

[1] https://github.com/systemd/systemd/pull/11424

Change-Id: I19241e7677d9b6415aff79ac65c474ae71984417
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Stage 2 refactor LDAP parameters

ReadJsonObject isn't required for cases where we don't have a list of
structures, and ideally we should consolidate all fixed readJson calls
in one place (and not ha

Stage 2 refactor LDAP parameters

ReadJsonObject isn't required for cases where we don't have a list of
structures, and ideally we should consolidate all fixed readJson calls
in one place (and not have multi-depth readJson calls).

This commit moves all the calls up, and consolidates all the LDAP patch
params into a single struct that can be moved between the layers, rather
than having the parameters individually.

Tested: Does LDAP work anymore? Could use some help if anyone has test
scripts, otherwise code compiles and this is inspection only, but
similar to other mechanical changes we've made recently

Change-Id: I77c0a8b97d4783fdca875c86d7dace122a0a55d7
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Add type safety for NTP server objects

NTPServers is our last usage of nlohmann::json in a readJson unpack.
The capability and unit tests are left in place for that type in case we
need them in the

Add type safety for NTP server objects

NTPServers is our last usage of nlohmann::json in a readJson unpack.
The capability and unit tests are left in place for that type in case we
need them in the future, but for now, document them as deprecated.

Tested: Redfish service validator passes. Redfish protocol validator
passes most tests (1 known failure in SSE is unrelated to this change).

Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Move to Redfish setProperty call

This commit changes sdbusplus setProperty calls (in various files) to
setDbusProperty method in Redfish namespace that handles all DBus
errors in a consistent manner

Move to Redfish setProperty call

This commit changes sdbusplus setProperty calls (in various files) to
setDbusProperty method in Redfish namespace that handles all DBus
errors in a consistent manner.

It also handles and translates additional DBus errors to Redfish
specific errors in dbus_utils file.

Tested By:
Not tested yet

Change-Id: If440774879413754f4c24f9b6572c3c9fa1fd033
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>

show more ...

Fix nullptr failures for image upload

Several places that call *req.ioService were missing nullptr checks.
Add them, and fix the one case where it might not be filled in.

Tested: With HTTP2 enable

Fix nullptr failures for image upload

Several places that call *req.ioService were missing nullptr checks.
Add them, and fix the one case where it might not be filled in.

Tested: With HTTP2 enabled, the following command succeeds.
```
curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:"
```

Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix post code parsing

If we use string_view with std::from_chars, we can use begin() and end()
directly (because they return pointers) and not have to do silly things
like dereference end(), which,

Fix post code parsing

If we use string_view with std::from_chars, we can use begin() and end()
directly (because they return pointers) and not have to do silly things
like dereference end(), which, while works in practice, is technically
undefined behavior, and some static analyzers complain about it.

Tested: Unit tests pass against both old parsePostCode and new.

Change-Id: Icfdec3b81f4a9c9bed3599571a8bc8779f9bfb98
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Use rvalue reference

Change-Id: Ie1a03bac54183b206bf27e37f1fed804601c8643
Signed-off-by: Ed Tanous <ed@tanous.net>

Remove a copy

Capturing by auto here causes a copy. Found using static analysis.

Change-Id: Ifbb08f9af0cd6eeec1e611c610e7adf53e17665c
Signed-off-by: Ed Tanous <ed@tanous.net>

Add nullptr check

Change-Id: If511f1210cca7bd1da3a8c5152688487d3036e2f
Signed-off-by: Ed Tanous <ed@tanous.net>