Make code pass clang-tidy-13

clang-tidy-13 appears to have improved its "use brace initialization"
checker to handle more cases, and now correctly fails a couple cases
that we posses. This commit s

Make code pass clang-tidy-13

clang-tidy-13 appears to have improved its "use brace initialization"
checker to handle more cases, and now correctly fails a couple cases
that we posses. This commit simply makes the very mechanical changes to
make this function.

Tested:
Ran previous commits checks to run ninja clang-tidy, and observed that
clang-tidy now passes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I665a7f3e71d25af93703e6b31af0f1eb12a44527

show more ...

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multi

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multiple compile units. It also adds the static keyword
to the privilege registry, and the inline keyword to many methods that
now need it.

clang-format is also updated to version 12 in parse_registies.py, as
that's what CI uses, and what most people have installed.

Tested:
Followed clang-tidy instructions in README.md
"bitbake bmcweb" step now succeeds.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id43b13606754cb37a404799fce155599ac3a3240

show more ...

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.py to also pull down the privilege registry
from DMTF.
The script then generates privilege_registry.hpp, which include const
defines for all the privilege registry entries in the same format that
the Privileges struct accepts. This allows new clients to simply
reference the variable to these privilege structures, instead of having
to manually (ie error pronely) put the privileges in themselves.

This commit updates all the routes.

For the moment, override and OEM schemas are not considered. Today we
don't have any OEM-specific Redfish routes, so the existing ones inherit
their parents schema. Overrides have other issues, and are already
incorrect as Redfish defines them.

Binary size remains unchanged after this patchset.

Tested:
Ran redfish service validator

Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure
that the new privileges constructor didn't cause us to regress the brace
construction initializer.

Checked binary size with:
gzip -c
$BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb
| wc -c
1244048

(tested on previous patchset)

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f

show more ...

Modify entityPrivileges for certificate service

DMTF published new entity privileges for certificate service classes
which modify entity privilege Certificate, CertificateCollection,
CertificateLoca

Modify entityPrivileges for certificate service

DMTF published new entity privileges for certificate service classes
which modify entity privilege Certificate, CertificateCollection,
CertificateLocations, and CertificateService on bmcweb. Modification
restricts a user without "ConfigureManager" from accessing the
CertificateCollection and Certificate scehamas

Redfish is a hypermedia API where the parent URI describes sub-URI.
Thus, restricting sub-URI in a parent-URI data helps to forbidden user
access, stricken the rule. So sub-URI only gets display if a user has
access to that URI.

Restricting the link allows the Redfish Validator to pass.

These impact roles without ConfigureManager, which include operator
and read-only. No access is not impacted since it already did not
have access.

The following are bmcweb user consequences:
1. ReadOnly and Operator role users are no longer able to view
certificates or the certificate collection (LDAP, HTTPS, TrustStore)
2. Operator role users are no longer able to replace the certificates
(LDAP, HTTPS, TrustStore), Install certificates (LDAP, HTTPS,
TrustStore) or delete the Truststore Certificate. HTTPS and LDAP
certificates do not have delete methods.

Resolves openbmc/bmcweb#61

Tested: manually tested on Witherspoon system and run Redfish-Service-
Validator with all roles root, operator, read-only, and No access. Test
pass for root, operator, and read-only roles, And new errors get
introduced for no access role.

Signed-off-by: Abhishek Patel <Abhishek.Patel@ibm.com>
Change-Id: Ibc5eed7db7e224e46f8572df8bcfba2a1ff47644

show more ...

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together.

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together. To handle this, there were two overloads
of the privileges() method, one that took a vector of Privileges, and
one that took an initializer_list of const char*. Unfortunately, this
leads some code in AccountService to pick the wrong overload when it's
called like this
.privileges( {{"ConfigureUsers"}, {"ConfigureManager"},
{"ConfigureSelf"}})

This is supposed to be "User must have ConfigureUsers, or
ConfigureManager, or ConfigureSelf". Currently, because it selects the
wrong overload, it computes to "User must have ConfigureUsers AND
ConfigureManager AND ConfigureSelf.

The double braces are supposed to cause this to form a vector of
Privileges, but it appears that the initializer list gets consumed, and
the single invocation of initializer list is called. Interestingly,
trying to put in a privileges overload of
intializer_list<initializer_list<const char*>> causes the compilation to
fail with an ambiguous call error, which is what I would've expected to
see previously in this case, but alas, I'm only a novice when it comes
to how the C++ standard works in these edge cases. This is likely due
in part to the fact that they were templates of an unused template param
(seemingly copied from the previous method) and SFINAE rules around
templates.

This commit functionally removes one of the privileges overloads, and
adds a second set of braces to every privileges call that previously had
a single set of braces. Previous code will not compile now, which is
IMO a good thing.

This likely popped up in the Node class removal, because the Node class
explicitly constructs a vector of Privilege objects, ensuing it can hit
the right overload

Tested:
Ran Redfish service validator

Tested the specific use case outlined on discord with:
Creating a new user with operator privilege:
```
redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2
AccountService adduser foo mysuperPass1 Operator
```

Then attempting to list accounts:
```
curl -vvvv --insecure --user foo:mysuperPass1
https://192.168.7.2/redfish/v1/AccountService/Accounts/foo
```

Which succeeded and returned the account in question.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495

show more ...

Remove Redfish Node class

Reduces the total number of lines and will allow for easier testing of
the redfish responses.

A main purpose of the node class was to set app.routeDynamic(). However
now a

Remove Redfish Node class

Reduces the total number of lines and will allow for easier testing of
the redfish responses.

A main purpose of the node class was to set app.routeDynamic(). However
now app.routeDynamic can handle the complexity that was once in critical
to node. The macro app.routeDynamic() provides a shorter cleaner
interface to the unerlying app.routeDyanic call. The old pattern set
permissions for 6 interfaces (get, head, patch, put, delete_, and post)
even if only one interface is created. That pattern creates unneeded
code that can be safely removed with no effect.
Unit test for the responses would have to mock the node the class in
order to fully test responses.

see https://github.com/openbmc/bmcweb/issues/181

The following files still need node to be extracted.

virtual_media.hpp
account_service.hpp
redfish_sessions.hpp
ethernet.hpp

The files above use a pattern that is not trivial to address. Often their
responses call an async lambda capturing the inherited class. ie
(https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770
28d258fa/redfish-core/lib/account_service.hpp#L1393)
At a later point I plan to remove node from the files above.

Tested:
I ran the docker unit test with the following command.
WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb
./openbmc-build-scripts/run-unit-test-docker.sh

I ran the validator and this change did not create any issues.
python3 RedfishServiceValidator.py -c config.ini

Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df

show more ...

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended a

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended after the first tree is done populating.
Don't use res.end() anymore.

Tested:
1. Validator passed.

Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8

show more ...

Remove IBM copyright

These aren't needed and are not in all files.
These aren't being updated. Would perfer these go away.

Tested: Not Tested.

Change-Id: I050874d58f86cae138ce2ab8c0c53831aeba5b21

Remove IBM copyright

These aren't needed and are not in all files.
These aren't being updated. Would perfer these go away.

Tested: Not Tested.

Change-Id: I050874d58f86cae138ce2ab8c0c53831aeba5b21
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Remove unnecessary error responses for LDAP certs

Currently, /v1/CertificateService/CertificateLocations and
/v1/AccountService/LDAP/Certificates endpoints assume the presence of
xyz.openbmc_project

Remove unnecessary error responses for LDAP certs

Currently, /v1/CertificateService/CertificateLocations and
/v1/AccountService/LDAP/Certificates endpoints assume the presence of
xyz.openbmc_project.Certs.Manager.Client.Ldap service, and return an
error on D-Bus failures. But this service can be missing if LDAP support
is removed from the build, so we should just return empty responses
instead of errors.

Tested: Passed Redfish service validator.

Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Change-Id: Ib8416e850b52e8ce0f8947017d863cee19f7b2c8

show more ...

Improve loops & fix cpp check warning

- This commit improves certain while loops to range based for loops.

- This commit also fixes the cppcheck warning that mentions about
performance issues whe

Improve loops & fix cpp check warning

- This commit improves certain while loops to range based for loops.

- This commit also fixes the cppcheck warning that mentions about
performance issues when using postfix operators on non-primitive
types.

Tested By:
- A function is unittested.
- GET on both EthernetInterfaces & certificate service
looks good without any issues.

Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: I85420f7bf9af45a97e1a93b916f292c2516f5802

show more ...

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Us

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Using std::move on const variables. This does nothing.
2. Passing big variables (like std::string) by value.
3. Using double quotes on a find call, which constructs an intermediate
string, rather than using the character overload.

Tested
Loaded on system, logged in successfully and pulled down webui-vue. No
new errors.

Walked the Redfish tree a bit, and observed no new problems.

Ran redfish service validator. Got no new failures (although there are
a lot of log service deprecation warnings that we should look at).

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

show more ...

Fix integer constant from 20 to 18

A bad click merged a commit before it was ready (it was +2ed
previously). So far as I'm aware, this was the only change needed.

Happy to revert it if someone see

Fix integer constant from 20 to 18

A bad click merged a commit before it was ready (it was +2ed
previously). So far as I'm aware, this was the only change needed.

Happy to revert it if someone sees need to.

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

show more ...

Use std::array instead of char array

Char arrays are outdated, and not needed in this case.

No functional changes

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

Use std::array instead of char array

Char arrays are outdated, and not needed in this case.

No functional changes

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

show more ...

Fix naming conventions

Lots of code has been checked in that doesn't match the naming
conventions. Lets fix that.

Tested:
Code compiles. Variable/function renames only.

Signed-off-by: Ed Tanous

Fix naming conventions

Lots of code has been checked in that doesn't match the naming
conventions. Lets fix that.

Tested:
Code compiles. Variable/function renames only.

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

show more ...

Enable unused variable warnings and resolve

This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I th

Enable unused variable warnings and resolve

This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I think it was worthwhile. It also cleaned up several unused
variable from old constructs that no longer exist.

Tested:
Built with clang. Code no longer emits warnings.

Downloaded bmcweb to system and pulled up the webui, observed webui
loads and logs in properly.

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

show more ...

Enable clang warnings

This commit enables clang warnings, and fixes all warnings that were
found. Most of these fall into a couple categories:

Variable shadow issues were fixed by renaming variabl

Enable clang warnings

This commit enables clang warnings, and fixes all warnings that were
found. Most of these fall into a couple categories:

Variable shadow issues were fixed by renaming variables

unused parameter warnings were resolved by either checking error codes
that had been ignored, or removing the name of the variable from the
scope.

Other various warnings were fixed in the best way I was able to come up
with.

Note, the redfish Node class is especially insidious, as it causes all
imlementers to have variables for parameters, regardless of whether or
not they are used. Deprecating the Node class is on my list of things
to do, as it adds extra overhead, and in general isn't a useful
abstraction. For now, I have simply fixed all the handlers.

Tested:
Added the current meta-clang meta layer into bblayers.conf, and added
TOOLCHAIN_pn-bmcweb = "clang" to my local.conf

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

show more ...

Remove middlewares

Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token aut

Remove middlewares

Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.

Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.

This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.

Note, this also deletes about 400 lines of hard to understand code.

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

show more ...

Codespell redfish-core spelling fixes

These spelling errors were found using
https://github.com/codespell-project/codespell
Tested: Top commit (along with this) was built and ran against
val

Codespell redfish-core spelling fixes

These spelling errors were found using
https://github.com/codespell-project/codespell
Tested: Top commit (along with this) was built and ran against
validator.
Change-Id: Ic9dce27b1de8567eedf7753164ef564d3aedf8ca
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

clang-format: update to latest from docs repo

This is from openbmc/docs/style/cpp/.clang-format

Other OpenBMC repos are doing the same.

Tested: Built and validator passed.
Change-Id: Ief26c755c9ce

clang-format: update to latest from docs repo

This is from openbmc/docs/style/cpp/.clang-format

Other OpenBMC repos are doing the same.

Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Change the default EC key to secp384r1

prime256v1 is okay for now, but secp384r1 is more future-proof (gives us
a couple more years) and in this case does not really have any
drawbacks.

Tested: Che

Change the default EC key to secp384r1

prime256v1 is okay for now, but secp384r1 is more future-proof (gives us
a couple more years) and in this case does not really have any
drawbacks.

Tested: Checked to see that a new secp384r1 key is generated on first
boot and the generate CSR redfish option works.

Change-Id: I334fc56db3dd55058a4c6780f8966bcc48d8f816
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>

show more ...

certificate_service: Remove odata.context

Redfish made odata.context optional (1.6.0 of DSP0266, Sept 2018).
Redfish has removed odata.context from example payloads in the
specification (1.7.0 of DS

certificate_service: Remove odata.context

Redfish made odata.context optional (1.6.0 of DSP0266, Sept 2018).
Redfish has removed odata.context from example payloads in the
specification (1.7.0 of DSP0266), removed it from the mockups,
and Redfish recommended not using.

The reason for making optional and removing from mockups/examples,
"no one could figure out how to use it and it did not add value".

Don't see value in it for our implementation.

Change-Id: I3d634aa1a58072589e565f2361e010b459bfd3f5
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Add "Retry-After" header for temporarily unavailable messages

Whenever the Redfish response is that a service is temporarily
unavailable, the "Retry-After" header is added with the same
value, so ju

Add "Retry-After" header for temporarily unavailable messages

Whenever the Redfish response is that a service is temporarily
unavailable, the "Retry-After" header is added with the same
value, so just set the header automatically with the response.

Tested:
Confirmed that the "Retry-After" header is set correctly with
the Redfish temporarily unavailable message.

Change-Id: I9c940be94d9d284b9633c5caa2ce71ade76d22d5
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>

show more ...

Fix extracting certificate id

std::strtol() expects null-terminated string. This means that passing
string_view.data() to it may cause undefined behaviour.

Let's fix it by using boost::convert inst

Fix extracting certificate id

std::strtol() expects null-terminated string. This means that passing
string_view.data() to it may cause undefined behaviour.

Let's fix it by using boost::convert instead.

Tested: Manually by sending valid requests and looking for empty
responses.

Change-Id: I319277551b5e85586783afdc8c86e4a7d8db876e
Signed-off-by: Iwona Klimaszewska <iwona.klimaszewska@intel.com>

show more ...

Certificate delete API – middleware

With introducing Mutual-TLS and option to add multiple certificates
there is a need to give user a possibility to remove them, for example
when they expire. This

Certificate delete API – middleware

With introducing Mutual-TLS and option to add multiple certificates
there is a need to give user a possibility to remove them, for example
when they expire. This commit adds implementation of DELETE function
to TLS Certificate node, so each of them can be removed.

Beckend implementation is here:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/25268

Tested with uploaded multiple TLS certificates.
Other certificates remains irremovable as they were so far.

Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com>
Change-Id: I9781c5c79288ec5d080e80e42c63a55e471ddb77
Depends-On: I9dd6fa998e8bd8081fbd13549831bc94a4a7aa54

show more ...

Handling of adding certificates the Redfish way (TrustStore)

Added handling for POSTing certificates the Redfish way (as proper JSON).
Currently it was only possible to add certificate as a RAW cert

Handling of adding certificates the Redfish way (TrustStore)

Added handling for POSTing certificates the Redfish way (as proper JSON).
Currently it was only possible to add certificate as a RAW certificate in
request body. Now user is able to add it as
{
"CertificateType": "PEM",
"CertificateString": "..."
}
as well as previously in RAW form.

Tested:
- Uploading certificates in RAW form
- Uploading certificates in JSON form
- In case of malformend reqeust a propser error message is returnd.

Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com>
Change-Id: Iab563964102b0a1a351cb0bb1ea181643da33480

show more ...