| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Fix KVM failed to load images issue introduced by incorrect condition
updated by commit 0260d9d6b252d5fef81a51d4797e27a6893827f4.
Tested:
KVM loaded images successfully
Signed-off-by: Arun P. Mohanan <arun.p.m@linux.intel.com>
Change-Id: Ib753ed1d56ce2e0a9228ca52e36ffab298d21cff
|
|
The http client at bmcweb does not resolve the client's
hostname asynchronously
This commit implements the async_resolve by using systemd resolved.
The async dbus message to resolvd.service is sent when a subscriber
successfully subscribes for events. The method ResolveHostname is
used to resolve the subscriber's hostname
Tested by:
Subscribe for the events at BMC using DMTF event listener
Generate an event and see the same is received at the listener's console
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I3ab8206ac4764cfa025e94c06407524d6ba220e0
|
|
Clang tidy 11 got some really neat checks that do a much better job.
Unfortunately, this, combined with the change in how std::executors has
defined how callbacks should work differently in the past, which we
picked up in 1.73, and now in theory we have recursion in a bunch of our
IO loops that we have to break manually. In practice, this is unlikely
to matter, as there's almost a 0% chance that we go through N thousand
requests without ever starving the IO buffer.
Other changes to make this build include:
1. Adding inline on the appropriate places where declared in a header.
2. Removing an Openssl call that did nothing, as the result was
immediately overwritten.
3. Declaring the subproject dependencies as system dependencies, which
silences the clang-tidy checks for those projects.
Tested:
Code builds again, clang-tidy passes
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic11b1002408e8ac19a17a955e9477cac6e0d7504
|
|
Fixes #178
Every few months, this option breaks because of some combination of
compiler options. I'm hoping that this is a more permenant fix, and
will keep it working forever.
Functionally, this commit changes a couple things.
1. It fixes the regression that snuck into this option, by making the
req variable optional using the c++17 [[maybe_unused]] syntax.
2. It promotes the BMCWEB_INSECURE_DISABLE_XSS_PREVENTION into the
config.h file, and a constexpr variable rather than a #define. This has
the benefit that both the code paths in question will compiled
regardless of whether or not they're used, thus ensuring they stay
buildable forever. The optimization path will still delete the code
later, but we won't have so many one-off build options breaking. We
should move all the other feature driven #ifdefs to this pattern in the
future.
3. As a mechnaical change to #2, this adds a config.h.in, which delcares
the various variables as their respective constexpr types. This allows
the constants to be used in a cleaner way.
As an aside, at some point, DISABLE_XSS_PREVENTION should really move to
a non-persistent runtime option rather than a compile time option. Too
many people get hung up on having to recompile their BMC, and moving it
to runtime under admin credentials is no more a security risk.
As another aside, we should move all the other #ifdef style options to
this pattern. It seems like it would help with keeping all options
buildable, and is definitely more modern than #ifdefs for features,
especially if they don't require #include changes or linker changes.
Tested:
enabled meson option insecure-disable-xss, and verified code builds and
works again.
Change-Id: Id03faa17cffdbabaf4e5b0d46b24bb58b7f44669
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.
This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.
Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0
|
|
Lots of code gets checked in that does this path checking incorrectly.
So much so, that we have it documented in COMMON_ERRORS.md, yet, we
persist. This patchset starts using the new object_path::filename()
method that was added recently to sdbusplus. Overall, it deletes code,
and makes for a much better developer experience.
Tested:
Pulled down several endpoints and verified that filename() method works
properly, and the collections are returned as expected.
curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/AccountService/Accounts
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ief1e0584394fb139678d3453265f7011bc931f3c
|
|
While uploading the ConfigFiles, BMC was only checking if it is
not multipart/form-data. This commit is to change the validation
to check for only allowed content-type: application/octet-stream
Tested by:
Uploaded Configfile with below content-types
1. application/octet-stream - passed
2. application/x-www-form-urlencoded - failed
3. application/json - failed
4. multipart/form-data - failed
5. text/plain - failed
6. application/octet-streamabcd - failed
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: Iedadacd2306f729479ee36afff52e29d8112daf6
|
|
The management_console_rest.hpp uses the crow::Response object to
return the response, which is the old way of returning the response to
the client.
This commit brings the bmcweb::AsyncResp class object for sending
the response to the client instead of the crow::Response object
Tested by :
Performed GET, PATCH, DELETE on the /ibm/v1 resources
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: I5ba01bda68d1e6b4590e910bd187aeb9cd6a149b
|
|
The IBM management console usecase - ConfigFile upload was allowing
to create or modify any file at the BMC when the path url is given
as below.
PUT https://${bmc}/ibm/v1/Host/ConfigFiles/../../../../../<any file under root dir> --data-binary "junk data"
This commit adds validation to the "path" variable after the "ConfigFiles/"
in the url - so that only the ConfigFiles are created or modified.
The filename validation includes:
Restrict the maximum filename length to 20 characters
Restrict the allowed charaters to [A-Za-z0-9-]
The minimum size of the file allowed is 100 bytes
The maximum size of the file allowed is 500KB
Maximum total size of the ConfigFile directory at BMC file system allowed is 10MB
Tested by:
1. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/../../../../../etc/p2 --data-binary "some data"
Bad Request
2. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/../../../etc/p2 --data-binary "some data"
Bad Request
3. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/../etc/p2 --data-binary "some data"
Bad Request
4. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/etc/p2 --data-binary "some data"
{
"Description": "Error while creating the file"
}
5. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/mydir/p2 --data-binary "some data"
{
"Description": "Error while creating the file"
}
6. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/ --data-binary "some data"
Not Found
7. PUT https://${bmc}/ibm/v1/Host/ConfigFiles --data-binary "some data"
Method Not Allowed
8. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2/../p2 --data-binary "some data"
Bad Request
9. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2/p2 --data-binary "some data"
{
"Description": "Error while creating the file"
}
10. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2/../../../p2 --data-binary "some data"
Bad Request
11. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/./../../p2 --data-binary "some data"
Bad Request
12. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/. --data-binary "some data"
Bad Request
13. PUT https://${bmc}/ibm/v1/Host/../ConfigFiles/p2 --data-binary "some data"
Not Found
14. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2 --data-binary "some data"
{
"Description": "File Created"
}
15. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2 --data-binary "some data"
{
"Description": "File Updated"
}
16. PUT https://${bmc}/ibm/v1/Host/ConfigFiles/p2.ext --data-binary "some data"
{
"Description": "File Created"
}
17. Tested sending filename greater than 20 charaters
Bad Request
18. Tested sending filename with special charaters
Bad Request
19. Tested sending filesize less than 100bytes
Bad request
20. Tested sending filesize greater than 500KB
Bad request
21. Tested uploading the file when the directory size is nearly full
Bad request
22. Added unit test for isValidConfigFileName
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: I838d39d5765ddc8701f7e5c533a93eebde021cbf
|
|
camelLower is not a type, camelBack is.
Changes were made automatically with clang-tidy --fix-errors
To be able to apply changes automatically, the only way I've found that
works was to build the version of clang/clang-tidy that yocto has, and
run the fix script within bitbake -c devshell bmcweb. Unfortunately,
yocto has clang-tidy 11, which can apparently find a couple extra errors
in tests we already had enabled. As such, a couple of those are also
included.
Tested:
Ran clang-tidy-11 and got a clean result.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9d1080b67f0342229c2f267160849445c065ca51
|
|
Add meson options to enabled/disabled authentication methods:
- basic-auth : For enable basic authentication, default is enabled
- session-auth : For enable session token authentication, default is
enabled
- xtoken-auth : For enable x-token authentication, default is enabled
- cookie-auth : For enabled cookie authentication, default is enabled
Signed-off-by: Alan Kuo <Alan_Kuo@quantatw.com>
Change-Id: I52e636f2534a14897cb57d35e563ea8841cc68b9
|
|
- Add a hostname listener that will create a self-signed HTTPS
certificate with the appropriate subject when the BMC gets its
hostname assigned via IPMI. The "insecure-disable-ssl" must be
disabled for this feature to take effect.
Note:
- New self-signed certificate subject: C=US, O=OpenBMC, CN=${hostname}
- If the same hostname is assigned, it will not be triggered
- Only the self-signed certificate with Netscape Comment of
"Generated from OpenBMC service" will be replaced
Details about certificate key usage:
- NID_basic_constraints
The CA boolean indicates whether the certified public key may be
used to verify certificate signatures.
Refer to: https://tools.ietf.org/html/rfc5280#section-4.2.1.9
- NID_subject_alt_name
Although the use of the Common Name is existing practice, it is
deprecated and Certification Authorities are encouraged to use the
dNSName instead.
Refer to: https://tools.ietf.org/html/rfc2818#section-3.1
- NID_subject_key_identifier
The subject key identifier extension provides a means of
identifying certificates that contain a particular public key.
Refer to: https://tools.ietf.org/html/rfc5280#section-4.2.1.2
- NID_authority_key_identifier
The authority key identifier extension provides a means of
identifying the public key corresponding to the private key used
to sign a certificate.
Refer to: https://tools.ietf.org/html/rfc5280#section-4.2.1.1
- NID_key_usage
- NID_ext_key_usage
id-kp-serverAuth
-- TLS WWW server authentication
-- Key usage bits that may be consistent: digitalSignature,
-- keyEncipherment or keyAgreement
Refer to: https://tools.ietf.org/html/rfc5280#section-4.2.1.3
Refer to: https://tools.ietf.org/html/rfc5280#section-4.2.1.12
Tested:
- To test and verify the service is functionally working correctly,
we can use `openssl` and `ipmitool` to execute the following
commands:
- Assign BMC hostname
ipmitool -H $IP -I lanplus -U root -P 0penBmc -C 17 dcmi
set_mc_id_string $hostname
- Get BMC server certificate infomation
echo quit | openssl s_client -showcerts -servername $IP -connect
$IP:443
Signed-off-by: Alan Kuo <Alan_Kuo@quantatw.com>
Change-Id: I24aeb4d2fb46ff5f0cc1c6aa65984f46b0e1d3e2
|
|
After establishing the obmc_console socket
communication, If client closes the connection
abruptly, async read/write operation fails with
asio.ssl.stream error. To handle the error, it calls
closeHandler call back function and cleans the session
and socket. Any ongoing async read operation should
be discarded by checking socket handle. Read/Write the
message from stream via async_read_some()/async_write
without checking socket handle, causes the crash.
Added socket handle validation before performing any
read/write operation to avoid crash.
Tested:
- Without fix, when sol connection closes abruptly, at times
saw the crash with below logs.
Nov 13 11:32:51 intel-obmc bmcweb[20169]: doRead error asio.ssl.stream:1
Nov 13 11:32:51 intel-obmc systemd[1]: bmcweb.service: Main process exited, code=dumped, status=11/SEGV
Nov 13 11:32:51 intel-obmc systemd[1]: bmcweb.service: Failed with result 'core-dump'.
- With fix, verified the case and no crashes seen.
Nov 13 12:55:04 intel-obmc bmcweb[24426]: (2020-11-13 12:55:04) [ERROR "websocket.h":207] doRead error asio.ssl.stream:1
Nov 13 12:55:04 intel-obmc bmcweb[24426]: (2020-11-13 12:55:04) [ERROR "obmc_console.hpp":67] doread() - Socket closed
Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
Change-Id: I2afda509ca77a561651a8682e042c45ca7366642
|
|
Before writing bmcweb_persistent_data.json on bmcweb shutdown call
applySessionTimeouts() to ensure no stale sessions are wrote.
To accomplish this had to move applySessionTimeouts to public.
Tested: Stop bmcweb, modify bmcweb_persistent_data.json timeout to
be 30 seconds. Start bmcweb. Verify timeout 30 seconds and
1 session is restored. Wait 1 min. stop bmcweb. Verify
no sessions in bmcweb_persistent_data.json.
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
Change-Id: Idfaf7c144b3bdeb2741b48f603d7213ac1a51f10
|
|
This commit implements the ClientOriginIPAddress property on
the session resource. The IP address is persisted across the reboot
Tested by:
1. Create session
POST https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":<>, "Password":<>}'
2. Check the session gets updated with the ClientOriginIPAddress
GET https://${bmc}/redfish/v1/SessionService/Sessions/<id>
3. Redfish validator passed
4. Create session and reboot the BMC to ensure the IP address is persisted
5. Tested the basic auth populates the clientIp at req
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: Iaa60d0657c991bde4bcf6c86819055c71c92e421
|
|
- 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
|
|
- 3174e4dfd3185c131a07371b4b5a5b40cf0e0bdb commit had broke the
release lock api. This small change has been overlooked in the
commit during the refactoring.
- status is a bool & status2 would be of type RcReleaseLockApi.As part of
refactoring instead of status2 we were returning status(bool) as a parameter
in the pair.
Tested By:
- Functional Lock Testing & openbmc-test-automation passed.
(openbmc-test-automation/openpower/ext_interfaces/test_lock_management.robot)
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: I71334dc863023cd40e9d813a5fa147493f5c3f9f
|
|
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
|
|
cppcheck isn't smart enough to recognize these are c++ headers, not c
headers. Considering we're already inconsistent about our naming, it's
easier to just be consistent, and move the last few files to use .hpp
instead of .h.
Tested:
Code builds, no changes.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40
|
|
fix regression on 5fb91ba400e0482813cf5e1a86fdca17468d0a6a.
Timeout is a global setting, not a per-session setting. This caused
problems with regenerating it, as session restoration doesn't follow the
"best effort" policy we've done before.
This commit:
1. Makes Session::fromJson more robust against extra keys.
2. Disallowed reading in client_id if IBM_Management_console isn't
enabled.
3. Moves timeout to the proper place in the persistent config file.
Resolves https://github.com/openbmc/bmcweb/issues/158
Tested:
Downloaded to bmc, cleared bmcweb_persistent_data.json, then logged in
using webui-vue.
Rebooted BMC.
Reloaded /redfish/v1/SessionService/Sessions/<sessionid> and observed
that all data restored properly. Unclear why, but ClientOriginIPAddress
seems broken, but that seems true prior to this patch.
Data that got returned is included for completeness.
{
"@odata.id": "/redfish/v1/SessionService/Sessions/BKqK5dNfNS",
"@odata.type": "#Session.v1_3_0.Session",
"ClientOriginIPAddress": "",
"Description": "Manager User Session",
"Id": "BKqK5dNfNS",
"Name": "User Session",
"UserName": "root"
}
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I716431fd4775af63715d07973f723caa8cb34259
|
|
(In the voice of the kid from sixth sense) I see string copies...
Apparently there are a lot of places we make unnecessary copies. This
fixes all of them.
Not sure how to split this up into smaller patches, or if it even needs
split up. It seems pretty easy to review to me, because basically every
diff is identical.
Change-Id: I22b4ae4f96f7e4082d2bc701098a04f7bed95369
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
|
|
Now that CI can handle clang-tidy, and a lot of the individual fixes
have landed for the various static analysis checks, lets see how close
we are.
This includes bringing a bunch of the code up to par with the checks
that require. Most of them fall into the category of extraneous else
statements, const correctness problems, or extra copies.
Tested:
CI only. Unit tests pass.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9fbd346560a75fdd3901fa40c57932486275e912
|
|
This commit moves the openssl random number generator into its own file,
so it can be used in EventService, and moves it to its own file. Seeding a
random number generator with time is bad practice in general, so much so
that there's a CERT rule about it as well as a clang-tidy check.
https://clang.llvm.org/extra/clang-tidy/checks/cert-msc51-cpp.html
This doesn't matter much in this case, as we're generating a randomized
int for an ID, but it will matter in other cases, and we'd like to have
the check on to verify that.
Change-Id: I8e6aebb7962d259045ffd558eea22f07f9c23821
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Several pieces of code seems to be using the adl_serializer from
nlohmann. This unfortunately has very undesirable behavior in some
cases, and makes a lot of things really difficult to track back to the
function that did the serialization, which has caused several bugs in
the past with incorrect types.
This patchset removes them, and opts for the inline version of the
nlohmann json serialization.
Tested:
Booted bmcweb, and logged in.
cat bmcweb_persistent_data.json showed persistent data written properly.
Logged into bmc through webui-vue
systemctl restart bmcweb
Then refreshed webui-vue, and didn't get logged out.
Change-Id: I92868629c54d08b37dd1d956f7c2e2a954f9b670
|
|
Lots of bad includes got put in recently, including big things, like
boost/http and beast/core. These are lots of code to parse, and leads
to files including things they didn't mean to.
Tested:
Code compiles
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I38de889fdfd9b23f66a2259bb30bf6584345e77f
|
|
The build with `-DBMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION=OFF` fails
with a message about unused parameters in `performTLSAuth` function.
This commit wraps the `performTLSAuth` call in an ifdef statement to
fix the build.
Change-Id: I08a6f8a8708fd14401894db6c058405154e92aa1
Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
|
|
As part of getting the clang-tidy bugprone tests running, these
conditionals were found to have duplicate entries, or be somewhat
nonsensical.
Tested: clang-tidy now passes with the branch-compliance check set.
Change-Id: Ibec106f3bbd866fc471a1fc56bd8cdd5d44024e0
|
|
- This commit would add the patch support for the session
timeout propery under the sessionservice.
- This commit also brings in support for persistent session
timeout property.
Tested By:
1. Redfish validator passed.
2. PATCH the session time out property using the below command
PATCH -d '{"SessionTimeout": 100}' https://<bmcip>/redfish/v1/SessionService
3. GET on sessionservice should return the value of time out which is
patched by using the above command & also GET on the session service fails
with Unauthorized error post the patched timeout value.
4. And also, the existing sessions that are open for the new timeout value are
also closed.
5. As per the schema , the range of values that are allowed for
session timeout are between 30 sec to 86400 sec, so any value which
is patched out of the range is failed with an appropriate error message.
6. PATCH the session timeout to new value using 2, and them restart the bmcweb
and the GET using 3 should return the new value.
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: Id50eacc5018b7a82371fd37a2ae1e7fb7596ed2b
|
|
Someone needs to double check me here, but I suspect this was not doing
what the author intended with the sizeof call, considering it's no a C
array.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I603b72837e24be0eca6337d0703dc56c47dba1d3
|
|
There was some modernization problems in the IBM console. These are all
minor, and unlikely to cause problems. The issues were:
1. Trivial destructors need to use the = default syntax
2. Several loops can be simplified into range based for loops
3. push_back should not be paired with make_pair. emplace_back should
be used instead.
Change-Id: I71b1d5437249d896a6f95c211e176deb676f985d
|
|
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
|
|
In between the json patch being written, and the json patch being
merged, nlohmann library added binary types:
https://nlohmann.github.io/json/features/binary_values/
Which is non standard, but used for things like cbor.
Add a switch to handle them.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I8599847a063a14c5f489e8347c2c440347d2544d
|
|
The existing JSON to html conversion is quite unfortunate, as it runs
several very expensive regular expressions on an output to properly
invoke the correct behavior, and to escape things like links. This
patchset adjusts the behavior to directly dump the tree to HTML,
skipping the json step entirely.
Most of the code was pulled from the nlohmann::serializer class.
Small side node:
This also resolves the CSP issue with the inline CSS classes that are
currently embedded in the json UI.
Note, in terms of user facing behavior, this finally fixes the CSS
issue, so the div is now centered as designed. Previously it was left
justified.
Tested:
Ran several redfish schemas and compared to old ones. Output appears
the same in the window, and content security policy warnings are gone.
Verified several links works as expected, and verified the behavior of
all base types, as well as empty arrays and empty objects. All appear
to work correctly.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Id9bf6dc33acb1603f009de4cd322e81d83f334be
|
|
Lots of missing inline definitions, a case where a RVO move is not
guaranteed when returning a variant, and removing the header checks,
which means that these types of build errors wont happen in the future.
Tested:
Should be no impact, but could someone from the IBM team grab these
changes and sanity check them?
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Iea0a06b8e744542a7d08e38217718e7a969f2827
|
|
We inherited a "using namespace" crow. Lets fix it.
Tested:
Code compiles. No functional changes.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Id47446150dfb312c5cd84a4b4284fb824eba8021
|
|
IBM management console had a using namespace in it. This is against the
coding standard.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Idfd5eac1a91e82f08139d6913a42a6c882072495
|
|
Details on why this revert is needed are here.
https://lists.ozlabs.org/pipermail/openbmc/2020-August/022478.html
Appu and Ravi still have not commented.
It should be noted, this also causes a memory leak in http connection,
where connections refuse to be freed, because of a bad usage of
shared_from_this.
This code wasn't very well thought through, and needs rearchitected to
not break the unit testability of bmcweb, nor cause memory leaks.
https://github.com/openbmc/bmcweb/blob/218bd4746130aac22366968c8c9a34a929e45a3d/http/http_connection.h#L351
Is the memory leak in question.
Specifically, this reverts:
The /attachment download in LogServices. This needs reimplemented
properly, but is an OEM property, so it shouldn't be a big deal to
revert, and shouldn't break our redfish compliance.
The IpAddress property in SessionService. I have no idea why this was
injected, and it's functionally incorrect. IpAddresses are not related
to a session, and IP addresses can change over the course of a session,
so this property is already broken as written. I suspect the author
really wanted RedfishEvent type logging, but that was too complex, so
they half implemented this.
Redfish SSE properties. This needs to be reimplemented similar to the
patchset here:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/13948
Where the ownership of the HTTP connection does not leave the http
framework. As written, the SSE implementation causes ownership issues,
as there's no clear delineation of the ownership between HttpConnection
and the SSE framework.
Tested:
On current master, running this command:
wget -O- --no-http-keep-alive --no-check-certificate https://{bmc
hostname}:18080/redfish/v1
Which should download the service root, then immediately close and
destroy the connection, prints:
(2020-08-28 16:55:24) [DEBUG "routing.h":1258] Matched rule
'/redfish/v1/' 2 / 4
(2020-08-28 16:55:24) [DEBUG "http_response.h":130] calling completion
handler
(2020-08-28 16:55:24) [DEBUG "http_response.h":133] completion handler
was valid
(2020-08-28 16:55:24) [INFO "http_connection.h":429] Response: 0x1e1ee28
/redfish/v1 200 keepalive=0
(2020-08-28 16:55:24) [DEBUG "timer_queue.h":48] timer add inside:
0x1d3d1a8 7
(2020-08-28 16:55:24) [DEBUG "http_connection.h":751] 0x1e1ee28 timer
added: 0x1d3d1a8 7
(2020-08-28 16:55:24) [DEBUG "http_connection.h":655] 0x1e1ee28 doWrite
(2020-08-28 16:55:24) [DEBUG "http_connection.h":663] 0x1e1ee28
async_write 1555 bytes
(2020-08-28 16:55:24) [DEBUG "http_connection.h":697] 0x1e1ee28 timer
cancelled: 0x1d3d1a8 7
(2020-08-28 16:55:24) [DEBUG "http_connection.h":676] 0x1e1ee28 from
write(1)
Then stops. Note, that the connection was not destroyed, and has
leaked. Once this patchset is added, the connection closes and destroys
properly, and doesn't leak, so it prints the above, but also prints.
(2020-08-28 16:27:10) [DEBUG "http_connection.h":305] 0x1d15c90
Connection closed, total 1
Ran Redfish service validator. Saw one unrelated failure due to UUID,
all other things pass.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I18686037bf58f20389d31facc0d77020274d38a1
|
|
clang-tidy warned on some data structures that, if they throw, the
exceptions can't be caught.
Move these data structures to constexpr equivalents to save some memory.
Tested:
Loaded webui. Worked as intended, and static files loaded properly.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I331ebfc2451f0cc0a82a1b70d325008c9c80401a
|
|
Unused param errors are throwing from this sources by the recent
CMake changes. Looks like CI build didn't catch these errors then.
Tested: Build is verified.
Signed-off-by: Vikram Bodireddy <vikram.bodireddy@linux.intel.com>
Change-Id: I139c01a78babc1c370c0c5de787291726ea42b53
|
|
There were a couple places in code where we still use index based for
loops. Move these to the more modern range based for loops.
Tested: Needs testing. Changes made by clang-tidy.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I30bf6fae6b2540434d5c98900a8f6bd0c8f2be93
|
|
Boost 1.74.0 got released the yesterday and deprecated some more stuff
that we use. This patchset prepares us so we will build for it when
meta-oe picks it up.
Tested:
Code builds under boost 1.74.0
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Icc6c54da6705098fc76e3ee6dbdc6c3b5c57fbda
|
|
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>
|
|
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
|
|
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>
|
|
Free memory during failed case inside
pamFunctionConversation() function.
Tested:
- Pam authentication works as normal.
Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
Change-Id: I81c06a3d674b0806c96e5847cda6f208795bd02c
|
|
This commit implements the broadcast of the messages from one
connected client to other connected clients via BMC. When the
management console creates a subscription on the BMC, they will
be provided with the broadcast message service.
Tested by: (Used https://github.com/DMTF/Redfish-Event-Listener)
1. Create a subscription
POST -D headers.txt https://${bmc}/redfish/v1/EventService/Subscriptions
-d '{"Destination":"https://<host:port>","Protocol":"Redfish"}'
2. Send the message
POST https://${bmc}/ibm/v1/HMC/BroadcastService -d '{"Message":"<msg>"}'
3. Verify the event is generated and posted to the subscriber:
bodydata: {"Message":"<The message from HMC to be forwarded>",
"Name":"Broadcast Event","OriginOfCondition":
"/ibm/v1/HMC/BroadcastService",
"Timestamp":"2020-07-15T12:03:30+00:00"}
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
Change-Id: Ib36b4f25505cf66251adc5aeda282312996c25af
|
|
The commit implements the sending of push style events to the IBM's management client
when a configFile is updated.
Tested-By:
1. Create a subscription by passing "ResourceTypes" as ["IBMConfigFile"]
POST -D headers.txt https://${bmc}/redfish/v1/EventService/Subscriptions
-d '{"Destination" : "https://<host:port>,"ResourceTypes":["IBMConfigFile"],"Protocol":"Redfish"}'
2. Update an existing ConfigFile
PUT https://${bmc}/ibm/v1/Host/ConfigFiles/<filename> --data-binary "@<local_path>"
3. Verify the event is generated and posted to the subscriber as the following example:
bodydata: {
"@odata.type":"#Event.v1_4_0.Event",
"Events":[
{
"EventId":1,
"EventTimestamp":"2020-06-26T08:40:04+00:00",
"EventType":"ResourceChanged",
"MemberId":0,
"Message" :"One or more resource properties have changed.",
"MessageArgs":null,
"MessageId":"ResourceEvent.1.0.3.ResourceChanged",
"OriginOfCondition":"/ibm/v1/Host/ConfigFiles/<filename>",
"MessageSeverity":"OK"
}
],
"Id":1,
"Name":"Event Log"
}
4. Verified the event is sent to the subscriber when the resourceType list is empty.
5. Verified the client subscribes for other resource - not ConfigFile ; then
the event is not sent to the subscriber.
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
Change-Id: I785c2a5a6e4e721cf722e94693db3a832f69fa50
|
|
This implements the sendEvent when the IBM management console
creates the ConfigFile at BMC using the PUT operation on the
url /ibm/v1/Host/ConfigFiles
Tested by: (Used https://github.com/DMTF/Redfish-Event-Listener)
1. Create a subscription by passing "ResourceTypes" as ["IBMConfigFile"]
POST -D headers.txt https://${bmc}/redfish/v1/EventService/Subscriptions
-d '{"Destination" : "https://<host:port>,"ResourceTypes":["IBMConfigFile"],"Protocol":"Redfish"}'
2. Create a ConfigFile
PUT https://${bmc}/ibm/v1/Host/ConfigFiles/<filename> --data-binary "@<local_path>"
3. Verify the event is generated and posted to the subscriber as below example
bodydata: {
"@odata.type":"#Event.v1_4_0.Event",
"Events":[
{
"EventId":1,
"EventTimestamp":"2020-06-26T08:40:04+00:00",
"EventType":"ResourceAdded",
"MemberId":0,
"Message":"The resource has been created successfully.",
"MessageArgs":null,
"MessageId":"ResourceEvent.1.0.3.ResourceCreated",
"OriginOfCondition":"/ibm/v1/Host/ConfigFiles/<filename>",
"MessageSeverity":"OK"
}
],
"Id":1,
"Name":"Event Log"
}
4. Verified the event is sent to the subscriber when the resourceType list is empty.
5. Verified the client subscribes for other resource - not ConfigFile ; then
the event is not sent to the subscriber.
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: Ic9b195266fe2df67a3160197d03d9ac155ef0cd1
|
|
strncpy has range checks, which reduce the possibility of overrunning
the buffer in the case of a bug.
Tested: clang-tidy cert check now passes. Needs functional testing.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I2fab19ca40f97cc0574146883ee19b573285a59c
|
|
We use std::filesystem now, so use that directly instead of having the
using at the top of the file.
Tested: Code compiles. No functional change.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: Iab977f08a2a61dcc9f2c82c705e5bcc55304e81a
|
