| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
doAccept does essentially the same code in two ways.
boost::beast::lowest_layer is used elsewhere to deduplicate this code.
Use it here as well.
Tested:
curl -vvvv --insecure -u root:0penBmc "https://192.168.7.2:443/redfish/v1"
succeeds.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Idfb0cd8f62ffbc09d6e248c677c24ea1abcb7a5b
|
|
The original crow timeout system had a timer queue setup for handling
many thousands of connections at a time efficiently. The most common
use cases for the bmc involve a handful of connections, so this code
doesn't help us much.
These days, boost asio also implements a very similar timer queue
https://www.boost.org/doc/libs/1_72_0/boost/asio/detail/timer_queue.hpp
internally, so the only thing we're loosing here is the "fuzzy"
coalescing of timeout actions, for which it's tough to say if anyone
will even notice.
This commit implements a timer system that's self contained within each
connection, using steady_timer. This is much more "normal" and how most
of the beast examples implement timers.
Tested:
Minimal touch testing to ensure that things work, but more testing is
required, probably using sloworis to ensure that our timeouts are no
longer issues.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I19156411ce46adff6c88ad97ee8f6af8c858fe3c
|
|
I missed that getDateTimeOffsetNow is extracting the last 5 chars
for DateTimeOffset. So this patch changes the offset to the original
"+00:00" one.
Tested:
1. unit tests
2. Redfish Validator Tests: no errors found on DateTime or
DateTimeLocalOffset.
```
DateTime 1970-01-01T00:13:27+00:00 date Yes PASS
DateTimeLocalOffset +00:00 string Yes PASS
```
All other errors are not related.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I24977c476f18c88515d759e278ec56e5cbb73b3a
|
|
The existing codes cast uint64_t into time_t which is int32_t in
most 32-bit systems. It results overflow if the timestamp is larger
than INT_MAX.
time_t will be 64 bits in future releases of glibc. See
https://sourceware.org/bugzilla/show_bug.cgi?id=28182.
This change workarounds the year 2038 problem via boost's ptime.
std::chrono doesn't help since it is still 32 bits.
Tested on QEMU.
Example output for certificate:
{
"Name": "HTTPS Certificate",
"Subject": null,
"ValidNotAfter": "2106-01-28T20:40:31Z",
"ValidNotBefore": "2106-02-06T18:28:16Z"
}
Previously, the format is like "1969-12-31T12:00:00+00:00". Note
that the ending "+00:00" is the time zone, not ms.
Tested the schema on QEMU. No new Redfish Service Validator errors.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8ef0bee3d724184d96253c23f3919447828d3f82
|
|
Current HTTP server creates an IPv6 acceptor to accept both IPv4 and
IPv6 connections. In this way, IPv4 address will be presented as IPv6
address in IPv4-mapped format. This patch converts it back to IPv4.
Tested:
Verified the ClientOriginIP in Session is shown in native IPv4 format
instead of IPv4-mapped IPv6 format.
Change-Id: Icd51260b2d4572d52f5c670128b7f07f6b5e6912
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
Ed changed the code from req.emplace(parser->release()) to
req.reset() in line 728 of the http/http_connection.hpp file in this
patch https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/47122.
So req cannot be used in doReadHeaders(). These codes are not necessary,
so I choose to delete them.
Tested:
1. Keep these codes and set bmcweb-logging=enabled, then log in to
bmcweb using webui. Bmcweb will core-dump.
2. Delete these codes, bmcweb works normally.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I1875a3fe4fa1d03656631435508b3876d8a42e54
|
|
- Initialized boost::circular_buffer_space_optimized<std::string> which
was not initialized. It prevented any event from being send.
- Removed line 'parser->skip(true)' which cause all received responses
to be interpreted as errors. It was triggering retry which resulted in
sensing same event multiple times.
Tested:
POST redfish/v1/EventService/Subscriptions, body:
{
"Destination": "https://127.0.0.1:4042/",
"Protocol": "Redfish",
"EventFormatType": "MetricReport"
}
{
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The resource has been created successfully",
"MessageArgs": [],
"MessageId": "Base.1.8.1.Created",
"MessageSeverity": "OK",
"Resolution": "None"
}
]
}
POST redfish/v1/TelemetryService/MetricReportDefinitions, body:
{
"Id": "TestReport",
"Metrics": [
{
"MetricId": "TestMetric",
"MetricProperties": [
"/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius"
]
}
],
"MetricReportDefinitionType": "OnRequest",
"ReportActions": [
"RedfishEvent",
"LogToMetricReportsCollection"
]
}
{
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The resource has been created successfully",
"MessageArgs": [],
"MessageId": "Base.1.8.1.Created",
"MessageSeverity": "OK",
"Resolution": "None"
}
]
}
GET redfish/v1/TelemetryService/MetricReports/TestReport
{
"@odata.id": "/redfish/v1/TelemetryService/MetricReports/TestReport",
"@odata.type": "#MetricReport.v1_3_0.MetricReport",
"Id": "TestReport",
"MetricReportDefinition": {
"@odata.id": "/redfish/v1/TelemetryService/MetricReportDefinitions/TestReport"
},
"MetricValues": [
{
"MetricId": "TestMetric",
"MetricProperty": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius",
"MetricValue": "-8388608000.000000",
"Timestamp": "1970-04-12T02:28:53+00:00"
}
],
"Name": "TestReport",
"Timestamp": "1970-04-12T06:08:17+00:00"
}
EVENT RECEIVED
{
"@odata.id": "/redfish/v1/TelemetryService/MetricReports/TestReport",
"@odata.type": "#MetricReport.v1_3_0.MetricReport",
"Id": "TestReport",
"MetricReportDefinition": {
"@odata.id": "/redfish/v1/TelemetryService/MetricReportDefinitions/TestReport"
},
"MetricValues": [
{
"MetricId": "TestMetric",
"MetricProperty": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius",
"MetricValue": "-8388608000.000000",
"Timestamp": "1970-04-12T02:28:53+00:00"
}
],
"Name": "TestReport",
"Timestamp": "1970-04-12T06:08:17+00:00"
}
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I4912853c3b59593e7032424d0b48aca7a36889b3
|
|
This code was in the codebase previously, and at some point got removed
to make copies. Considering the previous commits to this one, making
copies of the request object is a bad idea, and should be avoided.
Therefore, this commit deletes the copy constructor for the Request
object, making request copies fail to compile.
It should be noted, it does leave the move constructor, which I don't
think it's really used, but as a rule isn't an anti-pattern.
Tested:
Code compiles. No functional change.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib38ed79a7c60340fb00f922f29265a3c3c7beca8
|
|
Implemented the disable-auth option.
This patch also removed the pam option which never worked.
Tested:
With disable-auth,
```
~# wget -qO- http://localhost/redfish/v1/Systems/
{
"@odata.id": "/redfish/v1/Systems",
"@odata.type": "#ComputerSystemCollection.ComputerSystemCollection",
"Members": [
{
"@odata.id": "/redfish/v1/Systems/system"
}
],
"Members@odata.count": 1,
"Name": "Computer System Collection"
}
```
Without disable-auth,
```
~# wget -qO- http://localhost/redfish/
{
"v1": "/redfish/v1/"
}
~# wget -qO- http://localhost/redfish/v1/Systems/system
wget: server returned error: HTTP/1.1 401 Unauthorized
```
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I88e4e6fa6ed71096bc866b42b9af283645a65988
|
|
Custom http headers provided by the subscriber was not passed on
to the http client request header
This commit passes the http headers to the Subscriber constructor
Tested by:
Subscribe to events with custom headers
Verify the event request packet sending the custom header to the
subscriber
Verified persistency of the subscription
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: I9a4f59b6e9477fae96b380565885f4ac51e2a628
|
|
These logs are reported in good paths (at least web interfaces are
working fine when they are logged). Convert them to warnings so they do
not show up as false errors when debugging bmcweb.
Here's the log we were getting:
Oct 08 19:20:48 p10bmc bmcweb[15348]: (2021-10-08 19:20:48) [ERROR "http_connection.hpp":537] 0x14bd360 Error while reading: end of stream
Oct 08 19:20:48 p10bmc bmcweb[15348]: (2021-10-08 19:20:48) [ERROR "http_connection.hpp":531] 0x14910b0 async_read_header 308 Bytes
Oct 08 19:20:48 p10bmc bmcweb[15348]: (2021-10-08 19:20:48) [ERROR "authorization.hpp":292] authHeader=
These code paths, and why they are not real errors, is not totally clear
to me. Hoping for some discussion on it in this review.
Tested:
- Verified these no longer show up when doing basic system management
with just error traces enabled in bmcweb
Change-Id: If357aeb93ff28ef02e135a888524e057cb188871
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
|
|
This reverts commit 91995f3272010875e1559397e98ca93354066a0e.
Seeing bumps fail.
https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/48864
Please fix, test, and resubmit.
Change-Id: Id539fe66d5a093caf8f22a393f7af7b58ead5247
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This reverts commit 0f3d3a01aed4040ef73a977a958ecdf4f68111f6.
Seeing bumps fail.
Change-Id: Ida7b1bae48abbed2e00a5259e8f94b64168d4788
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This change, moving the openHandler back to only supporting websocket
disconnects and not 404s.Because AsyncResp is removed from openHandler.
Tested:
Opened KVM in webui-vue and it works.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I90811f4ab91ad41cb298877f76252dce80932b2b
|
|
These modifications are from WIP:Redfish:Query parameters:Only
(https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/47474). And they
will be used in Redfish:Query Parameters:Only.
(https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/38952)
The code changed the completion handle to accept Res to be able to
recall handle with a new Response object.
AsyncResp owns a new res, so there is no need to pass in a res.
Tested:
1.Basic and Token auth both still work.
2.Use scripts/websocket_test.py to test websockets. It is still work
correctly.
python3 websocket_test.py --host 127.0.0.1:2443
This modification is a public part, so you can use any URL to test
this function. The response is the same as before.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I570e32fb47a9a90fe111fcd1f4054060cd21def3
|
|
The newest version of clang correctly recognizes that all values of this
enumeration are already covered by distinct cases, and that this
is unneeded. Considering the default case does nothing, it can just be
removed entirely.
Tested:
Code compiles in clang.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I15333be16affda91d1a733e5d97aaa028a424efd
|
|
For variables that aren't sent through the constructor, it's less code
and cleaner to initialize them inline as part of the struct.
Tested:
Per Appu:
"Did basic check and its initialized to default as before."
Per Sunitha:
"Tested the basic event notification scenario with this commit. Works
fine"
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I60e096686eb328edba51b26f27c3c879dae25a84
|
|
This patchset builds on
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/45761 and moves
the header generation into the constructor, rather than attempting to
repurpose addHeaders().
Tested:
Per Appu:
"Did basic check and its initialized to default as before."
Per Sunitha:
"Tested the basic event notification scenario with this commit. Works
fine"
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/46703/2
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia9e8b89574c7e0137f0d93f08378e45c2fcf5376
|
|
While we're here testing things anyway, it seems like a good time to do
a simple method rename following the projects new naming guidelines.
This seems like an easy thing to do while we're here anyway and might
short circuit a harder discussion later.
Tested:
Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie5ed1c38d7dec13f875efae4acc22f4fd582f9b9
|
|
In the current model, URLs get parsed twice, once to satisfy the
authenticate method, and once again later to satisfy the handle() call.
This commit deduplicates the parsing. This is wasteful, and as of the
previous commit, unnecessary.
Specifically, it moves the actual parsing into the request object, and
adds a target() method to explicitly set a url. This deduplicates the
code that was in http_connection, and centralizes it in request, where
it should really belong.
Tested:
curl --insecure "https://192.168.7.2/redfish/v1"
Returns the redfish v1 resource
curl --insecure "https://192.168.7.2/redfish/v1/Systems"
Returns 401 unauthorized
curl --insecure --user root:0penBmc "https://192.168.7.2/redfish/v1/Systems"
returns the SystemsCollection
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie7ee2d9a9a51bf21c03793b35730e7a0ca82623a
|
|
Removes includes that are now unused.
Tested:
Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7ad7ce71c4238c58d2d2b6030282143f775c5423
|
|
This commit moves the internal data structures to use
boost::beast::http::fields as its internal data structure. fields is a
hyper-optimized map implementation for http headers, and has a lot of
nice escaping properties. It is what boost::beast::http::request uses
under the covers, so this has some niceties in reducing the amount of
code, and means we can completely remove the headers structure, and
simply rely on req. When this conversion was done, now the type safety
of the incoming data needs to have better checking, as loading into the
keys has new requirements (like values must be strings), so that type
conversion code for to and from json was added, and the POST and PATCH
handler updated to put into the new structure.
Tested:
curl -vvvv --insecure -u root:0penBmc
"https://192.168.7.2:443/redfish/v1/EventService/Subscriptions" -X POST
-d
"{\"Destination\":\"http://192.168.7.2:443/\",\"Context\":\"Public\",\"Protocol\":\"Redfish\",\"HttpHeaders\":[{\"Foo\":\"Bar\"}]}"
returned 200.
Tested various "bad" headers, and observed the correct type errors.
Issued: systemctl restart bmcweb.
Subscription restored properly verified with.
GET https://localhost:8001/redfish/v1/EventService/Subscriptions/183211400
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I331f65e1a3960f1812c9baac27dbdcb1d54f112c
|
|
This commit attempts to split up the authenticate method to make it
easier to audit, and to simplify some duplicated URL parsing code.
First, some history:
authenticate used to be token authentication middleware, then it got
promoted into http connection, because of security concerns (we needed
to effectively rate limit unauthenticated users). Then we got rid of
middlewares entirely, then we rearranged the ownership of request such
that it owns all its data and inits later in the cycle. This has caused
a mess, so lets try to clean it up and make the connection class
simpler.
This commit specifically breaks up authenticate into two parts, the
first, which is the same as the old authenticate, is responsible for
actually authenticating the user, and no longer carries the
authorization credentials and allowlist with it. The allowlist, as well
as actually returning 401 is now moved into handle, where it makes more
sense, as the request is complete, and we can immediately invoke the
action, instead of having to set the isCompleted flag and wait until
later.
Because of this again, now the only calls to completeRequest are called
from handle(), which means we can remove the extra "if (req.completed)"
check we formerly had to do for authenticate, continuing to make
authenticate less of a special case.
The only possible negative to this patch, is now any allowlisted
endpoints still have to call through the authenticate code, whereas
previously they could take a fast path. This code runs all requests
against authenticate, regardless of their allowlist status. In theory,
this makes this slower, in practice, It seems to be an unmeasurable
impact.
Tested:
curl --insecure "https://192.168.7.2/redfish/v1"
Returns the redfish v1 resource
curl --insecure "https://192.168.7.2/redfish/v1/Systems"
Returns 401 unauthorized
curl --insecure --user root:0penBmc "https://192.168.7.2/redfish/v1/Systems"
returns the SystemsCollection
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic9c686b8da7bb6c03b9c113a6493f0e071b5bc77
|
|
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I93925bf34b4fec181a56d6524cbe9c6182a16b1f
|
|
Update to the latest version of boost::uri
The newest version of boost uri makes some breaking changes that we need
to account for. At the same time, we take the opportunity to move to
the error code based parse methods that don't rely on exceptions.
The biggest changes are:
The standalone build is no longer present. A discussion with the
boost::url maintainers shows that our best option is to do a simple
copy of the headers, and compile boost/url/src.hpp in a separate file.
This is intended to allow people to pull the library in "standalone" and
not have to rely on the build machinery in boost-url, which we don't
really need. Interestingly, this file doesn't have a newline at the
end, which clang correctly flags. OpenBMC doesn't really need that
warning, as we rely on clang-format to do that, so we add
-Wno-newline-eof clang to get the code to compile there.
All url parsers are moved to the parse_uri, or parse_relative_uri
equivalents. This slightly tightens the requirements around what URLs
are accepted, but in no ways that should break anything. (Ie,
"/redfish/v1" is no longer accepted for a virtual media endpoint.
boost::urls::url_view::params_type has been renamed to
query_params_type, and the relevant methods have been updated.
Because of the missing standalone mode, we now need to use
boost::string_view which doesn't implicitly construct from
std::string_view. Some discussion on the boost list shows that this is
coming soon, so that cruft can eventually be cleaned up, but for now we
need the construction.
Tested:
Loaded in qemu, and ran some URLs (/redfish/v1 and /redfish/v1/Chassis)
to ensure that the url handler functions as intended.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5843776d4ec01b4d92af2ee3a9cf1ebb1d920ae7
|
|
When the event send/receive is failed, the bmcweb does not handle
the failure to tear-down the complete connection and start a fresh
The keep-alive header from the event listener is read to update
the connection states, so that the connection will be kept alive
or closed as per the subscriber's specifications
Updated the connection state machine to handle retry logic properly.
Avoided multiple simultaneous async calls which crashes the bmcweb. So
added few "InProgress" flags which protects simultaneous async calls.
Changed buffer type from flat_buffer to flat_static_buffer and
imposed an upper limit on total size avoiding heap allocations.
Also changed the requestDataQueue from std::queue to
circular_buffer_space_optimized which allocates memory as needed
and dynamically controls size.
Used boost http response parser as parser for producing the response
message. Set the parser skip option to handle the empty response message
from listening server. On reception of response, the response code in
the header is checked to determine success/failure and trigger retry
in the case of failure.
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
- Update the listner to change the keep-alive to true/false and
observe the http-client connection states at bmcweb
- Changed listener client to return non success HTTP status code
and observed retry logic gets trigrred in http-client.
- Gave wrong fqdn and observed async resolve failure and retry logc.
- Stopped listener after connect and verified timeouts on http-client
side.
Change-Id: Ibb45691f139916ba2954da37beda9d4f91c7cef3
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
|
|
This commit enhances some of the parameters used at the http client
1. Added the buffer limit to requestDataQueue
This is to control the message length of an event
2. Moved the flat_buffer to a flat_static_buffer
This is to limit the maximum size to the requestDataQueue
3. Changed requestDataQueue from a queue to circular buffer
This is to make the requestDataQueue space optimized
Tested by:
- Subscribe for the events at BMC using DMTF event listener
- Generate an event and see the same is received at listener's console
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: I563b7f8d24e5cd5e12db324b2e02974328ebd955
|
|
Because http connection can possibly fail on a bad host header, we need
to fill in the request object earlier in the flow. This has the
potential to lead to use after free bugs, although in practice, we have
null checks, so it's a lower likelihood.
Tested:
Ran
redfishtool -S Always -A Session -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2 raw GET /redfish/v1/Managers/bmc
and verified it returned a 200 and the managers data structure.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I789d50f67dff14b769ccefacf55530b0b7c5c7cd
|
|
The previous commit moved userSession to possibly be a per-request
structure. Previously, it was only a per-connection structure, so there
wasn't explicit clearing of it in between requests. This can lead to
problems where a user remains authorized despite explicitly logging
themselves out.
Tested:
redfishtool -S Always -A Session -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2 raw GET /redfish/v1/Managers/bmc
Succeeded
Then a subsequent:
curl -vvvv --insecure "https://192.168.7.2/redfish/v1/Managers/bmc"
Failed with 401
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5844406bd6bed628e5851d8c2f29af875adbaaff
|
|
The code here is fairly complex, and can be simplified without actually
changing any of the logic. The largest thing that this patchset
changes, is that two levels of scope are removed by preferring to handle
the errors and return, rather than waiting until the end of the function
to handle the errors. This is more cleanup that can be done because of
the removal of middlewares.
Tested:
Tested using curl on both an endpoint that requires auth (ie
redfish/v1/Systems) and an endpoint that didn't (ie redfish/v1) as well
as with and without basic auth. All combinations gave the expected
result.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I72ea5276b8e8758823e41412457ae0c4ecca1aab
|
|
Both of these variables are leftover from when middlewares existed in
this codebase, and are essentially unused. The only use is to buffer
cleanupTempSession, which in practice, doesn't require the variable, and
can be run in all cases.
Because this code is "cleaning up" the basic auth session that got
created when the request started, in theory, if the request failed, it's
possible we didn't create a session, but that can happen through the
golden path too, if we access an unprotected resource, or access with
non basic auth, so there's no reason to hide this behind the "did the
middlewares fail" check. This got stuck under the check because of the
way the middlewares used to be ordered, to keep "identical" code paths.
Tested:
curl -vvvv --insecure --auth root:0penBmc"https://192.168.7.2:443/redfish/v1/Systems"
Succeeded with and without the --auth flag. This tests basic auth,
which is the only thing that could've been potentially changed as part
of this commit.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d2500fd2abaab3b23abed51a9a9f55e8d171b76
|
|
req is being created later, in the connection life cycle. req was
holding many important values when it was passed to authenticate, so the
authenticate call had to be refactored to includes all the data req was
holding.
Also uses of req before handle have been changed to direct calls to
boot::parse
Tested:
Made a request that did not require authentication
$ curl -vvvv --insecure "https://192.168.7.2:18080/redfish/v1"
Got correct service root
Made a unauthenticated request (Chassis)
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X GET https://192.168.7.2:18080/redfish/v1/Chassis
Unauthenticated
Made a log-in request
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST https://192.168.7.2:18080/login -d "{\"data\": [ \"root\", \"0penBmc\" ] }"
Made (same) Chassis request
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X GET https://192.168.7.2:18080/redfish/v1/Chassis
Tested the websockets using scripts/websocket_test.py
Websockets continued to work after this change.
Followed the mTLS instructions here https://github.com/openbmc/docs/blob/master/security/TLS-configuration.md
mTLS continues to work after this change.
Change-Id: I78f78063be0331be00b66349d5d184847add1708
Signed-off-by: John Edward Broadbent <jebr@google.com>
|
|
The Connection object used to set the response object public member.
However, it is cleaner when public interfaces are used.
Change-Id: Ib16950f174106e5fd22aad874f09f31704283ad1
Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
This commit moves the mtls code into its own method, to ensure that it's
self contained, and not done as one large thing in the connection
constructor.
Tested:
Ran instructions at
https://github.com/openbmc/docs/blob/master/security/TLS-configuration.md
Verified that the final call to SessionService in those instructions
succeeded.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I170723b0e9368e625412fc895e48f796cc54b9ce
|
|
This commit is for, enable the timeout setting with suggested
values for server(300sec) role. Server's keep_alive_pings is
true by default, so time to server role is become 300/2 sec.
It is used for to check server/client communication status
in every 150sec.
There is a timeout option to websocket stream, which is created
by boost library .By default, timeouts on websocket streams are
disabled. The way to turn them on is to set the suggested timeout
settings on the stream.
Idle Timeout is configured as 300sec to server role by default.
We can adjust the timeout value which we want exactly by changing
the idle_timeout.
Depends on keep_alive_pings , the suggested value of the idle
timeout will be consider. Keep_alive_pings of server role is
set to `true` by default in boost library. So idle_timeout of
server role is consider as 300/2 sec by default.
https://www.boost.org/doc/libs/master/libs/beast/doc/html/beast/using_websocket/timeouts.html
Tested:
Redirected Virtual media. connection has established successfully.
Media instance gets redirected to host machine. Disconnected the
client network. Keepalive idle timeout for server role will found
stream miss communication status in next 150sec cycle, and virtual
media websocket will be stop gracefully.
Existing behaviour:
900sec (15 minutes) will be taken for closing the websocket
connection due to network disconnection.
Signed-off-by: Dhines Kumar E <dhineskumare@ami.com>
Change-Id: Id68d6a5c9b0139b1c70f80e9265a3e4d96e2ee87
|
|
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
|
|
This commit adds the support for "DateTimeLocalOffset" property under
"/redfish/v1/Managers/bmc/" Redfish URI.
And it also adds the support for "DateTime" & "DateTimeLocalOffset"
properties under "/redfish/v1/Systems/system/LogServices/<id>/" &
"/redfish/v1/Managers/bmc/LogServices/<id>/" Redfish URI's.
These properties shows the current Date, Time & the UTC offset that the
current DateTime property value contains.
Tested:
- Redfish Validator Test passed.
curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X GET https://${bmc}/redfish/v1/Managers/bmc/
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_11_0.Manager",
"Actions": {
"#Manager.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Managers/bmc/ResetActionInfo",
"target": "/redfish/v1/Managers/bmc/Actions/Manager.Reset"
},
"#Manager.ResetToDefaults": {
"ResetType@Redfish.AllowableValues": [
"ResetAll"
],
"target": "/redfish/v1/Managers/bmc/Actions/Manager.ResetToDefaults"
}
},
"DateTime": "2021-06-04T12:18:28+00:00",
"DateTimeLocalOffset": "+00:00",
"Description": "Baseboard Management Controller",
"EthernetInterfaces": {
"@odata.id": "/redfish/v1/Managers/bmc/EthernetInterfaces"
},
"FirmwareVersion": "2.11.0-dev-114-gc1989599d",
"GraphicalConsole": {
"ConnectTypesSupported": [
"KVMIP"
],
"MaxConcurrentSessions": 4,
"ServiceEnabled": true
},
"Id": "bmc",
"LastResetTime": "2021-06-04T12:07:02+00:00",
"Links": {
"ActiveSoftwareImage": {
"@odata.id": "/redfish/v1/UpdateService/FirmwareInventory/419c86fb"
},
"ManagerForServers": [
{
"@odata.id": "/redfish/v1/Systems/system"
}
],
"ManagerForServers@odata.count": 1,
"SoftwareImages": [
{
"@odata.id": "/redfish/v1/UpdateService/FirmwareInventory/419c86fb"
}
],
"SoftwareImages@odata.count": 1
},
"LogServices": {
"@odata.id": "/redfish/v1/Managers/bmc/LogServices"
},
"ManagerType": "BMC",
"Model": "OpenBmc",
"Name": "OpenBmc Manager",
"NetworkProtocol": {
"@odata.id": "/redfish/v1/Managers/bmc/NetworkProtocol"
},
"Oem": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem",
"@odata.type": "#OemManager.Oem",
"OpenBmc": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc",
"@odata.type": "#OemManager.OpenBmc",
"Certificates": {
"@odata.id": "/redfish/v1/Managers/bmc/Truststore/Certificates"
}
}
},
"PowerState": "On",
"SerialConsole": {
"ConnectTypesSupported": [
"IPMI",
"SSH"
],
"MaxConcurrentSessions": 15,
"ServiceEnabled": true
},
"ServiceEntryPointUUID": "1832ebbb-0b54-44e9-90d7-b49108f6863c",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
},
"UUID": "7fe3d13d-4ae7-4a4f-add1-2d60308124b4"
}
curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X GET https://${bmc}/redfish/v1/Systems/system/LogServices/EventLog/
{
"@odata.id": "/redfish/v1/Systems/system/LogServices/EventLog",
"@odata.type": "#LogService.v1_1_0.LogService",
"Actions": {
"#LogService.ClearLog": {
"target": "/redfish/v1/Systems/system/LogServices/EventLog/Actions/LogService.ClearLog"
}
},
"DateTime": "2021-06-04T12:11:10+00:00",
"DateTimeLocalOffset": "+00:00",
"Description": "System Event Log Service",
"Entries": {
"@odata.id": "/redfish/v1/Systems/system/LogServices/EventLog/Entries"
},
"Id": "EventLog",
"Name": "Event Log Service",
"OverWritePolicy": "WrapsWhenFull"
}
Signed-off-by: Tejas Patil <tejaspp@ami.com>
Change-Id: I416d13ae11e236cf4552f817a9bd69b48f9b5afb
|
|
The ASN1 free will slowly leak memory for incorrect mutual auth
connections because if the certificate does not match the requirements
the function will return without freeing the usage string.
Tested: curl --cert client-cert.pem --key client-key.pem --cacert \
CA-cert.pem https://${bmc}/redfish/v1/SessionService/Sessions
Change-Id: I4c335d3cd151187c7a10e7e668d1556c11389039
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>
|
|
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
|
|
https://jenkins.openbmc.org/job/ci-openbmc/3949/distro=ubuntu,label=docker-builder,target=tiogapass/consoleText
This build seems to be failing with an error |
../git/http/http_response.hpp:23:10: error: 'optional' in namespace
'std' does not name a template type | 23 |
std::optional<response_type> stringResponse;
This should fix it by including the relevant headers.
Tested:
Code builds. CI error only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ifba15559a73d823d791de1d508e136a3c44e6cd1
|
|
The ip address is cleared out after req.emplace inside doWrite
function which leads to problem with ip address identification.
Fetch ip address on all handle requests instead of fetching on
connection's starts only.
Tested:
- no problems in filling of Request ip field with debug bmcweb build
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
Change-Id: Icc846285b987702a8db582434296d0d1b7f90b27
|
|
In the begining, bmcweb had its own webui checked in as source. Largely
conceived of clay, and built by someone that doesn't understand UI
development (me), it was eventually superceeded by phosphor-webui. When
we did that, we created a bug where bmcweb was expecting a UI to always
be installed, and when it wasn't resolved into an infinite recursive
redirect as it tried to find the login page.
This patchset fixes that, by adding a connection between the
authorization class, and the webassets class, for bmcweb to detect at
runtime whether or not the UI is installed, and change behavior in that
case.
Along the way, we got a circular #include, so some includes needed to be
rearranged slightly.
This patchset will change no behavior when the UI is installed. Login
failures will continue to redirect to /, to hit the login page.
If the UI is not installed, and there is no / route, BMCWEB will return
the plaintext UNAUTHORIZED if you attempt to open the webui from the
browser without having a webui installed and without having credentials.
Tested:
Launched in a build without webui-vue, and observed "UNAUTHORIZED" when
I connected through chrome.
Also launched in a build with webui-vue installed with:
IMAGE_INSTALL_append = "webui-vue"
And loaded the webui in chrome, and logged in successfully.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iac9b83ba9e80d434479685b082d547847cdfe309
|
|
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
|
|
When the session is created using /login, the ClientOriginIPAddress
is mapped to the clientId parameter which displayed the clientIP
instead of the of clientId.
The similar problem is observed with auth methods other than sessions
created using the SessionService resource
This commit swaps the clientId and clientIp parameters passed to
generateUserSession API, so that the optional clientId is
passed as the last parameter
Tested by :
1. Create session using Redfish command
POST https://${bmc}/login -d '{"username": <>,"password": <>}'
POST https://${bmc}/redfish/v1/SessionService/Sessions
-d '{"username": <>,"password": <>}'
2. Open the GUI session to check the clientId is not displaying the
ClientOriginIPAddress
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I6cee3de963c489e690d2ad0bb09ba78dca39e4f9
|
|
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
|
|
The router has an old sanity check in it to verify that nodes are
simple. This is no longer the case, as we can have multiple,
overlapping routes between different handlers, so non-simple root nodes
are allowed.
The commit here broke a couple things.
0260d9d6b252d5fef81a51d4797e27a6893827f4
First, when that route gets injected, the root node is no longer simple,
as the first root in the trie can be a complex node. This should be ok,
and this commit comments out the check.
Also, because the meson node for the option was loaded directly into
set10, instead of the boolean equivalent, the XSS feature always gets
enabled, regardless of whether or not that's what the user wanted. The
fix to this was to simply include a .enabled(), which correctly calls
the bool.
Tested:
Built with insecure-disable-xss set, and observed crash was removed.
Tried several routes including /redfish/v1 and observed them working.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib9fb55a61796ddbda65b7ee5d2803a5cbd2ae75f
|
|
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
|
|
config.h is a generic filename, unprefixed by any sort of name, that
other dependencies could use. Namely, nghttp2 uses an identical
filename, which can cause issues with getting the right one. This
commit renames that file to bmcweb_config.h to disambiguate it from
generic config.h files.
Tested:
Compiled bmcweb and observed compile time params get applied. There are
no defaults on any of this stuff, so there's no way to silently miss the
config file.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9a3e73c37161fa438c5612344dfb01f1f19aff2c
|
|
New versions of beast allow completely removing the per-message deflate
functionality from the binary, thus saving space. Considering we never
used it, it seems worthwhile to remove from the build entirely.
This should have no impact on any external interface.
https://www.boost.org/doc/libs/1_75_0/libs/beast/doc/html/beast/using_websocket.html
Tested:
Build before and after, ~31k of pre-compression binary space saved when
this patchset is included.
Also ran scripts/websocket_test.py
python3 websocket_test.py --host 192.168.7.2
CPU 67.56
Memory 5.95
and saw sensor values stream correctly.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3d8e5febea2446eb4894a840f7fe7ef9cdf6995b
|
|
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>
|
