Remove tcp_stream

tcp_stream has several problems in its current implementation. First,
it takes up a significant amount of binary size, for features that we
don't use. Next, it has some implied g

Remove tcp_stream

tcp_stream has several problems in its current implementation. First,
it takes up a significant amount of binary size, for features that we
don't use. Next, it has some implied guarantees about timeouts that we
erronously rely on, namely that async_connect will timeout
appropriately given the tcp_stream timeout (it doesn't).

We already have a timer present in the ConnectionInfo class anyway, this
commit just makes use of it for ALL timeout operations, rather than just
the connect timeout operations. This flow is roughly analogous to what
we do in http_connection.hpp already, so the pattern is well troden.

This saves 2.8% of the binary size of bmcweb, and solves several race
conditions.


Tested:
Relying on Carson: Aggregated a sub-bmc, and ensured that top level
collections returned correctly under GET /redfish/v1/Chassis

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

show more ...

Add SSL support for http_client (EventService)

This commit adds the initial SSL support for http_client which can be
used for sending asynchronous Events/MetricReports to subscribed Event
Listener s

Add SSL support for http_client (EventService)

This commit adds the initial SSL support for http_client which can be
used for sending asynchronous Events/MetricReports to subscribed Event
Listener servers over secure channel.

Current implementation of http client only works for http protocol.
With current implementation, http client can be configured to work
with secure http (HTTPS). As part of implementation it adds the SSL
handshake mechanism and enforces the peer ceritificate verification.

The http-client uses the cipher suites which are supported by mozilla
browser and as recommended by OWASP. For better security enforcement
its disables the SSLv2, SSLv3, TLSv1, TLSv1.1 as described in below
OWASP cheetsheet.

It is validated with RootCA certificate(PEM) for now. Adding support
for different certificates can be looked in future as need arises.

[1]: https://cheatsheetseries.owasp.org/cheatsheets/TLS_Cipher_String_Cheat_Sheet.html

Tested:
- Created new subscription with SSL destination(https) and confirmed
that events are seen on EventListener side.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "https://<IP>:4000/service/collector/event_logs",
"EventFormatType": "Event",
"DeliveryRetryPolicy": "RetryForever",
"Protocol": "Redfish"
}

- Unit tested the non-SSL connection by disabling the check in code
(Note: EventService blocks all Non-SSL destinations). Verified that
all events are properly shown on EventListener.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "http://<IP>:4001/service/collector/event_logs",
"EventFormatType": "Event",
"Protocol": "Redfish"
}

- Combined above two tests and verified both SSL & Non-SSL work fine in
congention.

- Created subscription with different URI paths on same IP, Port and
protocol and verified that events sent as expected.

Change-Id: I13b2fc942c9ce6c55cd7348aae1e088a3f3d7fd9
Signed-off-by: AppaRao Puli <apparao.puli@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

task payload, initialize jsonValue in constructor

We should do this because it means that variables are only initialized
once. In the simplest case:

struct MyClass{
std::string myString

MyCla

task payload, initialize jsonValue in constructor

We should do this because it means that variables are only initialized
once. In the simplest case:

struct MyClass{
std::string myString

MyClass(){
myString = "foo";

}

}

in the language, myString is constructed twice, once with empty string,
then a second time with "foo". If you do the construction in the
initializer list for the class the construction only happens once.

Now, the above case is contrived, the optimizer can see through it and
likely optimizes this case because std::string is relatively simple, but
for more complex structures, it's possible this generates less and
bettercompiled code, and this is worth having the check for, and making
our existing code correct.

Tested: cppcheck passing.

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

show more ...

Move time utils to be in one place

We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.

This commit mo

Move time utils to be in one place

We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.

This commit moves all the time functions from the crow::utility
namespace into the redfish::time_utils namespace, as well as moves the
unit tests.

No code changes where made to the individual functions, with the
exception of changing the namespace on the unit tests.

Tested: Unit tests pass.

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

show more ...

Simplify fillMessageArgs

The aforementioned function does a lot of reconstruction of strings as
args are filled in. This results in the end of the string being copied
many times (N). Replace the a

Simplify fillMessageArgs

The aforementioned function does a lot of reconstruction of strings as
args are filled in. This results in the end of the string being copied
many times (N). Replace the algorithm with one that builds a new
string, using reserve (which is good practice) and is also capable of
returning errors in the case of bad entries. fillMessageArgs now
returns a string instead of trying to do things in place, which avoids
the initial std::string construction, so we should be net the same here.

Given this new algorithm can now detect failures in parsing (ie, trying
to parse %1 with no arguments) add unit tests for coverage of that, and
modify event manager slightly to handle errors.

Tested: Unit tests pass. Pretty good coverage of this stuff.

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

show more ...

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I46a5eec210002af84239af74a93c830b1d4a13f1

show more ...

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. Thi

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. This commit goes through and updates all usages.

Arguably some of these are incorrect, and instances of common error 13,
but because this is mostly a mechanical it intentionally doesn't try to
handle it.

Tested: Unit tests pass.

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

show more ...

Fix const correctness issues

cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.

Tested: WIP

Signed-off-by: Ed Tanous <edtanous@google.com>
Cha

Fix const correctness issues

cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.

Tested: WIP

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

show more ...

Fix undefined behavior in timestamp parsing

If this algorithm ever processed a string where the + came before the .,
then dot - plus would render a negative, and overflow, causing a crash.

In pract

Fix undefined behavior in timestamp parsing

If this algorithm ever processed a string where the + came before the .,
then dot - plus would render a negative, and overflow, causing a crash.

In practice, this doesn't happen.

Tested: Inspection only at this time. Difficult to trigger error.

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

show more ...

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implic

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implicitly constructed an object.

This commit enables the clang-tidy check.

Tested: Code compiles, passes clang-tidy.

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

show more ...

Add missing Context type to MetricReport

During EventService subscription, user can specify the "Context" type to
identify the node. This "Context" must be sent to event listener for
every Events/Me

Add missing Context type to MetricReport

During EventService subscription, user can specify the "Context" type to
identify the node. This "Context" must be sent to event listener for
every Events/MetricReports. For MetricReports context filed is missing
and its added as part of this commit.

Tested:
- Added MetricReport subscription with "Context" type set and can see
the "Context" data on every metric report.

Change-Id: Idcf4826c8e4cd13ce8c8078b6c6223584669be4c
Signed-off-by: AppaRao Puli <apparao.puli@intel.com>

show more ...

sessions: iwyu

While revisiting change 49039 I saw session.hpp included a few
files that are not used. I removed them and the code still
compiles.

Signed-off-by: Sui Chen <suichen6@gmail.com>
Chang

sessions: iwyu

While revisiting change 49039 I saw session.hpp included a few
files that are not used. I removed them and the code still
compiles.

Signed-off-by: Sui Chen <suichen6@gmail.com>
Change-Id: I97aa2359053ce6102b84af1ef555d881cd35eaba

show more ...

clang: fix extra semicolon

Failed with -Wextra-semi.

Tested: no -Wextra-semi when build with clang++

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ib0c0b3a2b0fcfe0e415987baa18f810e4b

clang: fix extra semicolon

Failed with -Wextra-semi.

Tested: no -Wextra-semi when build with clang++

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ib0c0b3a2b0fcfe0e415987baa18f810e4b19b89f

show more ...

bmcweb: Set Retry Policy Valid Response Codes

Allows individual retry policies to specify what HTTP response codes
are considered valid. Sets functions for the EventService and
Redfish Aggregation

bmcweb: Set Retry Policy Valid Response Codes

Allows individual retry policies to specify what HTTP response codes
are considered valid. Sets functions for the EventService and
Redfish Aggregation retry policies. Those functions expect a
response code and return an error code based on what the response
code is.

This change is needed because EventService only considers 2XX codes
to be valid. Any code outside of that range would trigger a retry
attempt. Redfish Aggregation by design will need to return
errors outside of that range such as 404. It should not retry to
send a message when it receives a 404 from a satellite BMC.

Right now 404 is the only error code that is handled differently
between the services. Going forward, Redfish Aggregation will
likely want to allow other error codes as its functionality is
expanded.

Tested:
Used Redfish-Event-Listener with ssh port forwarding to create 3
subscriptions. I then closed the ssh connection and sent a test
event. Bmcweb made 3 retry attempts for each subscription. At
that point the max retry amount (as defined by EventService) was
reached and bmcweb stop attempting to resend the messages.

There were no errors when the Redfish-Event-Listener was correctly
connected. Test events resulted in messages being sent for each
subscription.

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ifdfaf638d28982ed18998f3ca05280a288e0020a

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

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

show more ...

bmcweb: Remove hardcoded HTTP verbs and headers

Modifies HttpClient so that the HTTP verb and headers can be set
for each individual message sent. Right now those fields are set
when a connection i

bmcweb: Remove hardcoded HTTP verbs and headers

Modifies HttpClient so that the HTTP verb and headers can be set
for each individual message sent. Right now those fields are set
when a connection is first created and then reused by each
message sent using that connection.

Tested:
Launched two Event Listener servers that created 6 and 2
subscriptions. Sending a test event resulted in the servers
receiving 6 requests and 2 requests, respectively.

Change-Id: I8d7e2d54385bc2c403498293820adb584bff8b57
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies.

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.

This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.

Tested:
Ran redfish service validator. No new failures.

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

show more ...

Refactor HttpClient Class

Refactors HttpClient with the following changes:
- Convert class to singleton
- Replace circular buffers with devectors
- Sending queued requests and closing connections ha

Refactor HttpClient Class

Refactors HttpClient with the following changes:
- Convert class to singleton
- Replace circular buffers with devectors
- Sending queued requests and closing connections handled
within their own callback
- Add connection pooling (max size 4)
- HttpClient supports multiple connections to multiple clients
- Retry policies can be set for specific use cases

Also modifies its use in the Subscription class to be compatible
with the refactored code.

It is assumed that a BMC will be able to handle 4 parallel
connections and thus the max pool size is set as 4. The max
number of queued messages was left unchanged at 50. Eventually
we may want to allow tuning of these limits to boost performance.
That would come in a future patch.

Tested:
Launched two Event Listener servers that created 6 and 2
subscriptions. Sending a test event created a connection pool
for each server. 4 and 2 connections were added to each pool,
respectively and were used to send the test request. For the first
pool the 2 extra requests were placed into a queue until
connections became available. After a request completed, its
associated connection was used to send the next request in
the queue. Resending the test event caused those prior connections
to be reused instead of new connections being added to the pools.

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Iba72b3e342cdc05d1fb972e2e9856763a0a1b3c5

show more ...

Remove regex uses in event service and consolidate

As the patch at
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/50994 can attest,
parsing urls with a regex is error prone. We should avoid

Remove regex uses in event service and consolidate

As the patch at
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/50994 can attest,
parsing urls with a regex is error prone. We should avoid it where
possible, and we have boost::urls that implements a full, correct, and
unit tested parser.

Ideally, eventually this helper function would devolve into just the
parse_uri, and setting defaults portion, and we could rely on the
boost::urls::url class to pass into things like http_client.

As a side note, because boost url implements port as a proper type-safe
uint16, some interfaces that previously accepted port by std::string&
needed to be modified, and is included in this patch.

Also, once moved, the branch on the ifdef for HTTP push support was
failing a clang-tidy validation. This is a known limitation of using
ifdefs for our code, and something we've solved with the header file, so
move the http push enabler to the header file.

Also note that given this reorganization, two EXPECT statements are
added to the unit tests for user input behaviors that the old code
previously did not handle properly.

Tested: Unit tests passing

Ran Redfish-Event-Listener, saw subscription create properly:
Subcription is successful for https://192.168.7.2, /redfish/v1/EventService/Subscriptions/2197426973

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

show more ...

EventService: Enhance inotify event filename handling

In kernel inotify, the inotify_event.name is padded to a mutiple of
sizeof(inotify_event) with '\0' and len is the size of char[] name,
not the

EventService: Enhance inotify event filename handling

In kernel inotify, the inotify_event.name is padded to a mutiple of
sizeof(inotify_event) with '\0' and len is the size of char[] name,
not the actual size of name. So constructing the name string with
std::string(name, len) constructs a string with all the '\0's, which
is not equal to the filename. This patch uses std::string(name) so
that the string does not contain these '\0's.

Tested:
Manually create/delete /var/log/redfish, confirmed handler is entered
by log.

Change-Id: Ibaa96dd5c47b0205541a6ee155daa593b2e2114d
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Use url_view for telemetry uris

This change refactor telemetry code to use bmcweb utility function for
uri construction, which is safe and preferred way, instead of string
operations.

Testing done:

Use url_view for telemetry uris

This change refactor telemetry code to use bmcweb utility function for
uri construction, which is safe and preferred way, instead of string
operations.

Testing done:
- Some basic GET operations done on Telemetry, no regression.

Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I6de5d79a078944d398357f27dc0c201c130c4302

show more ...

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

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

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

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

show more ...

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the ro

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the robot.

Tested: Code compiles, inspection only (changes made by robot)

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

show more ...

Enable 3 member function checks

The only changes were to make some functions static, which is
essentially no-op.

Changes were done by the robot.

Tested: Unit tests pass, changes no-op

Signed-off-

Enable 3 member function checks

The only changes were to make some functions static, which is
essentially no-op.

Changes were done by the robot.

Tested: Unit tests pass, changes no-op

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

show more ...

Drop message severity

In the way we store the message registry, we store both Severity and
MessageSeverity. Severity as a field is deprecated, and in every case
in every registry both fields have t

Drop message severity

In the way we store the message registry, we store both Severity and
MessageSeverity. Severity as a field is deprecated, and in every case
in every registry both fields have the same value. We shouldn't
duplicate data in that way. This commit changes the parse_registries.py
script to stop producing the Severity field into the struct. The few
uses we have left are moved over to use MessageRegistry.

Tested:

Redfish service validator shows no errors on the
/redfish/v1/Registries tree. Other errors present that were there
previously and are unchanged.

This saves a trivial amount: about 1kB on our compressed binary size.

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

show more ...