| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Recently, oss-fuzz added support for bmcweb, but did it by checking in
a number of code patches. This commit should get similar coverage by
hooking into the HTTP connection class, and using the unit test
code to inject bytes directly into a stream. This can be improved over
time, but this is a good start.
Change-Id: I20b5d536cff1588a8387f2770c022516e1cd62d0
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
IWYU is not a tool we have seen results from in some time. I very much
suspect that these few comments are not enough to get a clean build.
Clean them up. If we want to turn this tool back on in the future,
this patch can be reverted.
Tested: Comment only change. Review only.
Change-Id: I45ff737800f9d8b1b63db2f482e59f815d7126a2
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Add Response::headerCount() to return the total number of
stored header fields on a response. The implementation counts
the current header entries directly from the underlying Beast
header container.
Added unit coverage to verify the method reports the full
header count after headers are added through the normal
addHeader() path.
Tested with:
meson test -C build http_response_test --print-errorlogs
This helper is used by response unit tests to verify that only
the expected headers are present and that no unexpected headers
are added to the response. See also:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/91848
Change-Id: I28432bb4fc982db44cee0688395d04ac513d6749
Signed-off-by: Joel Pullokaran Jesin <joelpj@ami.com>
|
|
parseStringAsJson() routes through BmcwebSaxParse, which hard-caps a
payload at 500 total JSON values to defend against malicious external
HTTP request bodies.
The most visible symptom is that bmcweb silently loses every persisted
session across a restart once ~50 sessions accumulate (each persisted
session is ~11 JSON values, so the file trips the cap and is treated
as malformed). Tntegration testing hit this 50 session limit and
failed. (Was failing our internal but also the
openbmc-test-automation robot suite)
I had originally implemented a solution as a "trusted reader"
with no limit
( https://gerrit.openbmc.org/c/openbmc/bmcweb/+/90138 ); however,
the security implications of "no limit" weren't attractive.
This area may be refactored in the near future to represent each
session as a distinct json file. This value bump gets us past
the immediate needs without introducing a lot of churn in code
that will be refactored.
Change-Id: Ifd3514bd8ee15c8bdf1b6c2119451a2965801671
Signed-off-by: Rick Yazwinski <rickyaz@meta.com>
|
|
HTTP/2 connections were not passing the resolved client IP into
the per-request ipAddress field, causing sessions to report
0.0.0.0 instead of the actual client address.
Additionally, authentication::authenticate() was called with an
empty IP, resulting in Basic Auth sessions being created with
an incorrect clientIp in the session store.
Pass the IP through the HTTP2Connection constructor, use it in
the authenticate() call, and assign it to req->ipAddress during
request dispatch in onRequestRecv.
Tested:
- Client IP correctly displayed in Redfish SessionService
on HTTP/2 connections.
- Before patch: ClientOriginIPAddress showed "0.0.0.0"
- After patch: ClientOriginIPAddress shows actual client IP
- Redfish Service Validator (v3.1.4) passed on
/redfish/v1/SessionService tree:
PASS: 44, WARN: 0, FAIL: 0, NOT TESTED: 41
Before:
curl -k https://127.0.0.1:2443/redfish/v1/\
SessionService/Sessions/O9gUpSoxbe -u root:0penBmc
{
"@odata.id": "/redfish/v1/SessionService/Sessions/O9gUpSoxbe",
"@odata.type": "#Session.v1_7_0.Session",
"ClientOriginIPAddress": "0.0.0.0",
"Description": "Manager User Session",
"Id": "O9gUpSoxbe",
"Name": "User Session",
"Roles": [
"Administrator"
],
"UserName": "root"
}
After this patch:
curl -k https://172.31.216.225/redfish/v1/\
SessionService/Sessions/LuttjprSGD -u root:0penBmc
{
"@odata.id": "/redfish/v1/SessionService/Sessions/LuttjprSGD",
"@odata.type": "#Session.v1_7_0.Session",
"ClientOriginIPAddress": "10.0.136.165",
"Description": "Manager User Session",
"Id": "LuttjprSGD",
"Name": "User Session",
"Roles": [
"Administrator"
],
"UserName": "root"
}
Change-Id: I223e5c90448419ba87b690de38cc5e69ffb6a0c3
Signed-off-by: Vijaysankar Ravi <vijaysankarr@ami.com>
|
|
Continue moving OpenSSL into reusable RAII classes that can be used in
unit tests and other places. This is slightly more code, but as we're
adding unit tests, it allows reuse between unit tests rather than
writing C directly. It also encapsulates the complexity of parsing
openssl output (usually in bytes) into standard types (string) that can
be compared/modified.
Functionally this adds two new classes to the "wrappers" functions,
OpenSSLSSLCtx and OpenSSLSSL, which each wrap SSL_CTX and SSL objects
respectively from openssl. These are rough approximations of the boost
equivalents.
Change-Id: Id87ac4ccde88890bd70861deffdb256188ec0e39
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Commit 2d7dc991 changed the body storage from being saved in a separate
field to a std::variant. Calling str() calls emplace<std::string>(),
which destroys the active FileBody in-place on file-backed bodies (e.g.
the Web UI) and closes the FD.
Any subsequent action now tries to access a non-existing FD (dummy
handle returning -1 as FD.)
Fix by returning early from `attemptZstdCompression` when the body is
file-backed, as file-backed bodies are already handled by the streaming
zstdCompressor in the writer.
Tested: cURL with "Accept-Encoding: zstd" to favicon.ico, ensuring that
it returns data again.
Fixes: 2d7dc991 ("Optimize bmcweb memory usage for multipart fw
update")
Change-Id: Ia54712f89d8c5f7dc9ce7c5d040aed06e8f6fded
Signed-off-by: Tan Siewert <tan.siewert@9elements.com>
|
|
After RAII change code accepts UTF8 only, so the code doesn't use
non-UTF8. This test uses ASCII that is UTF8 and duplicates other tests,
so remove NonUTF8UPNSubjectAlternativeName test.
Tested:
Unit tests
Change-Id: I48b8f0cbf644abce99a864e9dccd23f308693908
Signed-off-by: Igor Kanyuka <ifelmail@gmail.com>
|
|
There was a regression on json parsing due to a merge conflict
resolution on 2d7dc991600297d04e50d7958fe9611f9ac42bcf. Update the
connection unit tests to test the body catching this case, and fix the
error.
Tested: Unit tests pass. Redfish service validator passes
Change-Id: I4ad8802b7c75001ca7a2b1619af2f368bfe448ee
Signed-off-by: Ed Tanous <etanous@nvidia.com>
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Current unit tests only covers happy path. Add more unit tests before
changing the code.
Tested: unit tests
Change-Id: Ibba5dbbc1457b59670d5d8f3c828fa9ca112f88c
Signed-off-by: Igor Kanyuka <ifelmail@gmail.com>
|
|
bmcweb openssl usage is a mess. Start cleaning it up.
1. Make RAII objects for any held memory.
2. Move methods from hostname monitor into the ssl namespace, so not all
compile units need to pull in openssl headers
3. Move methods to static where functions can be encapsulated.
Because we're now testing openssl, we need to register memory init so
that the sanitizers don't cause issues when mallocing from non
bootstrapped openssl binaries. Openssl provides a handle for this, so
use it in those unit tests.
Tested:
Unit tests pass. bmcweb launches and can open ssl with curl as it did
previously.
Change-Id: If0340692d2c56a6c45bb8d661d654a4b58ff3d2c
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
To do this, refactor the date string function to a single method that
can be unit tested easily. While we're here, deprecate our usage of
time_t and move to std::chrono
Tested: Redifsh service validator passes
Change-Id: I67d4fe66d40b06ed0a0b60286adc75394c34ea1e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Domain names are case-insensitive per RFC standards [1]. Update
isUPNMatch() to perform case-insensitive comparison of domain
labels.
This ensures UPNs like user@DOMAIN.COM match hostnames like
host.domain.com, as per DNS standards.
Tested:
1. Unit tests
2. Built an image, flashed to real BMC, made sure curl requests that use
mTLS auth with certs that use UPN work
3. Redfish service validator
[1] https://datatracker.ietf.org/doc/html/rfc4343
Change-Id: Ibf5715fdddace4263d0aeef54e518457cf03dfcc
Signed-off-by: Igor Kanyuka <ifelmail@gmail.com>
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
These static variables have the potential to cause reentrancy issues.
In practice, the conditions to cause issues would require someone to
basically write incorrect code, but it makes sense to wrap this into a
state tracker anyway to clean up the code. While we're here, convert to
using std::chrono.
Note, this changes the behavior such that the values produced are now no
longer dependent on timezone. Functionally, Redfish only recently got
the ability to set a timezone, so this is not expected to have any user
facing impact, even though the unit tests need to change.
Tested: RSV Passes
Change-Id: Icb7cff1d289ae23790a5fb1db6604abd73dd68fd
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
docs/COMMON_ERRORS.md documents patterns that are easy to introduce
and hard to catch in review. This adds machine-enforceable ast-grep
rules for those patterns so they get caught in CI instead of review.
Rules cover unsafe integer parsing, throwing JSON APIs, throwing
filesystem APIs, wildcard lambda captures, blocking calls, missing
trailing slashes on routes, and route string concatenation.
Tested: ast-grep scan --error exits 0.
Change-Id: I7ae24b52ac5b120826d29bb3ac6fce8f30199c15
Signed-off-by: Davy Marrero <dmarrero@nvidia.com>
|
|
TLS session resumption allows to bypass full TLS handshake in subsequent
connections, it's enabled in OpenSSL, so clients that support it use it.
One of the optimizations is, the client passes Session ID in subsequent
request and does not pass certificates. Since client certificate is not
passed, the callback that populates user session out of the certificate
is not called, and as result, auth fails for requests sents in
subsequent connections. This change enables session ID in memory cache,
lookup of the certificate in the cache by the session ID received from
the client and constructing user session out of it for subsequent
connections.
The cache is stored in RAM [1]. According to Nginx doc [2], size of one
session is about 250 bytes. If sessions use mTLS, they will also contain
a cert which is typically up 2kb. A client establishes connections as
part of a session, so a session can be associated with multiple
connections. In the worst case, when many clients establish a single
connection at a time, or a client always uses a new session for every
established connection, there will be number of session entries in the
cache equals to the number of connections. While OpenSSL limits cache
size to SSL_SESSION_CACHE_MAX_SIZE_DEFAULT which is 20480 [3], OpenBMC
limits number of established connections to 200 [4], so in the worst
case, memory usage will be ~50kb for non mTLS clients, and ~440kb for
mTLS clients. Typically, when there are just 2-3 clients connected, even
if them maintain multiple connections within their sessions, the cache
size will be less than 10kb for mTLS. To prevent high memory usage by
the cache, the change sets cache size to 100 entries. Expired sessions
are automatically removed on every 255th session [5].
Tested:
Deployed on one of our envs and ran client that quickly sends multiple
requests to the BMC (so the client created several connections), and
make sure the 401 auth problem had been observed before gone. Also, made
sure BMCWeb logged debug messages about existing session detection.
Ran tests from the openbmc-test-automation repository, esp related to
certificate and user management. They do session auth and not
mTLS/multi-connection, so they could not detect/confirm the problem is
fixed, but they confirm the change does not break the primary use case.
[1] https://docs.openssl.org/3.6/man3/SSL_CTX_set_session_cache_mode/#notes
[2] https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_cache
[3] https://github.com/openssl/openssl/blob/5869303daaecf037f0d00dc33a00f9bdc1e71f2f/include/openssl/ssl.h.in#L670
[4] https://github.com/openbmc/bmcweb/blob/master/http/http_connection.hpp#L219C13-L219C28
[5] https://docs.openssl.org/3.6/man3/SSL_CTX_flush_sessions/#notes
Change-Id: Ia94d1e323cd464cc7ca5b0f9c7d6a76e4c780e9a
Signed-off-by: Igor Kanyuka <ifelmail@gmail.com>
|
|
When taking json directly from a user, we should set some limits on
parsing depth as well as total number of value elements. Value elements
are considered any individual value, the start of an array, the start of
a dictionary, or null. This is to prevent flooding type attacks
creating large number of objects, while still keeping under the depth 10
cap. This commit makes use of the nlohmann sax parse to handle this by
injecting a new error handler in between that will impose new limits.
Currently this sets the depth limit to 10 and the total number of keys
to 500; These are intentionally high, and could be tuned or expanded on
in the future.
Tested: Unit tests pass.
Change-Id: I789543679e22b0b0ce0b2b0b71f31377b0759cd7
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
zstd compression allows for reducing the size of payloads with repeating
elements. Pretty printed json is one very specific case where we would
prefer to not change the behavior, but would also like to avoid the
overhead of TLS compression.
When ztd is present, the webserver will look for the Accepts-Encoding
header to contain zstd, and if it does, compress the payload before
sending.
Tested:
Webui loads correctly, and shows zstd being used for download.
Redfish service validator runs. (does not support zstd)
Unit tests pass.
Change-Id: Ic575911fea70d218d1efb4df030d3cc098f1ecd0
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
This commit adds file descriptor and temporary file management to
DuplicatableFileHandle, removing the redundant test-only
TemporaryFileHandle utility.
Changes:
- Add file descriptor constructor and setFd() method
- Add temporary file constructor with string_view content
- Add filePath member and automatic cleanup in destructor
- Add configurable temp-dir meson option (default: /tmp/bmcweb)
- Remove include/file_test_utilities.hpp
- Update all tests to use DuplicatableFileHandle
- Rename stringPath to filePath
These features will be used by the multipart parser to stream
large uploads to temporary files instead of keeping them in memory,
and by the update service to pass file descriptors over D-Bus.
Change-Id: I982f5928d453f9f0c13d91c3525006134ddc87b3
Signed-off-by: Rajeev Ranjan <ranjan.rajeev1609@gmail.com>
|
|
Include cleaner helps the code review process. Add it back, by ignoring
some of the more recent boost headers.
Change-Id: I6eddd0e67cd9f469c93fbb344cc1ab46231e450f
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
The test verifies proper Basic Authentication
header generation and Base64 encoding/decoding
Tested: Unit test passes
Change-Id: I4c4ae7e30b1d781967208849a1919021e69a0b65
Signed-off-by: Kamran Hasan <khasan@nvidia.com>
|
|
This utility function is being removed for several reasons. First, it
does not verify the full string on URIs and paths, so things like
/foo/bar/baz/valid_id would still pass this check.
Second, it is used for both URIs and dbus paths, both of which we have
better utility functions these days respectively, boost::url for urls
and sdbusplus::message::object_path for dbus paths. Neither of the two
is escaped properly when this function is used.
Therefore, remove it and replace it with the appropriate alternatives.
The existing URI functions were found to not accept fragments (given
they are rarely used in PATCH). Add support for fragments to cover the
getNthStringFromPath use cases.
Tested: Redfish service validator passes.
Change-Id: Ibc6755ad69397123d7fef0e0b764042bbb48888b
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
http2 maintains its own frame ACK window per stream. While the defaults
work well in most cases, for large binary uploads, like Redfish
UpdateService, the relatively small default window size of 16KB leads to
slower performance than http1. While it's not expected to see a
performance improvement, we would prefer to not see a regression for a
normal use case.
Update the HTTP2 max frame size to 16KB. Setting the internal buffer to
the same size + the http2 header allows clocking in the entire frame in
one async read. Note, setting the value higher than 16KB doesn't appear
to allow curl to send larger frames.
Also update the HTTP window size to 512KB, or 32 times the max frame
size. Note, all streams including the control stream are set to this
value, which, while somewhat arbitrary, allows for continued
UpdateService pushing without pauses for window ACK.
Tested:
POST /redfish/v1/UpdateService/update-multipart
Of an arbitrary 100MB file through curl shows that --http1.1 option and
--http2 option are within 5% of the same upload time.
Change-Id: I7ff6296a9cc0794aad63f5058620c0f1fb9299e3
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
When using aggregation with http2, :authority headers were getting
forwarded to the client, which didn't know how to deal with them on
http1.
Filter all http2 headers.
Tested: Unit tests pass.
Change-Id: I6a834656b604004eeba1a2aa2f245ef211f28495
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
When these tests hit failures, splitting up these frames makes it a lot
easier to debug.
Tested: Unit tests pass
Change-Id: I29f5906c2f7aa90d0bd9989ba9f9d2525987f4d9
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Within this namespace, we don't need to call crow, we are already in the
crow namespace.
Tested: Code compiles.
Change-Id: Ida57624ef1157f98f2719b5c3af536aebaca601e
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Passing the TLS-provided credentials from the HTTP connection to the
http2 connection got missed, and appears to break mutual TLS for http2
connections. Pass the credentials.
Tested: Mutual TLS is now functional on http2 connections as shown in
the next patch.
Change-Id: Ia2bbcd5383dae859baa96908b76f221b9c74632c
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Given the size of Redfish schemas these days, it would be nice to be
able to store them on disk in a zstd format. Unfortunately, not all
clients support zstd at this time.
This commit implements reading of zstd files from disk, as well as
decompressing zstd in the case where the client does not support zstd as
a return type.
Tested:
Implanted an artificial zstd file into the system, and observed correct
decompression both with an allow-encoding header of empty string and
zstd.
Change-Id: I8b631bb943de99002fdd6745340aec010ee591ff
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
as we have successfully merged patches that enable UserPrincipalName
parse mode, we can start removing Meta only parse mode. This commit
is intended to remove MTLSCommonNameParseMode::Meta from the upstream
code
Tested:
- build bmcweb
- deploy to a device that already use UPN
- check if it works fine by sending curl request /AccountService
Change-Id: Idcf4340a2a9940f035aea41cd30ef4df7bd95530
Signed-off-by: Malik Akbar Hashemi Rafsanjani <malikrafsan@meta.com>
|
|
This commit is intended to implement the UserPrincipalName (UPN) parse
mode on mutual TLS (MTLS). By implementing this we can use the X509
certificate extension Subject Alternative Name (SAN), specifically UPN
to be used as the username
In our case, this feature is needed because we have a specific format
on our Subject CN of X509 certificate. This format cannot directly
mapped to the username of bmcweb because it contains special
characters (`/` and `:`), which cannot exist in the username.
Changing the format of our Subject CN is very risky. By enabling
this feature we can use other field, which is the SAN extension to
be used as the username and do not change our Subject CN on the
X509 certificate
In general, by implementing this feature, we can enable multiple
options for the system. There might be other cases where we want to
have the username of the bmcweb is not equal to the Subject CN of the
certificate, instead the username is added as the UserPrincipalName
field in the certificate
The format of the UPN is `<username>@<domain>` [1][2]. The format
is similar to email format. The domain name identifies the domain
in which the user is located [3] and it should match the device name's
domain (domain forest).
Tested
- Test using `generate_auth_certificate.py` (extended on patch [4])
- Manual testing (please see the script mentioned above for more detail)
- Setup certificate with UPN inside SAN extension
- Change the CertificateMappingAttribute to use UPN
- Get request to `/SessionService/Sessions`
- Run unit tests
[1] UPN Format: https://learn.microsoft.com/en-us/windows/win32/secauthn/user-name-formats#user-principal-name
[2] UPN Properties: https://learn.microsoft.com/en-us/windows/win32/ad/naming-properties#userprincipalname
[3] UPN Glossary: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-wcce/719b890d-62e6-4322-b9b1-1f34d11535b4#gt_9d606f55-b798-4def-bf96-97b878bb92c6
[4] Patch Testing Script: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/78837
Change-Id: I490da8b95aee9579546971e58ab2c4afd64c5997
Signed-off-by: Malik Akbar Hashemi Rafsanjani <malikrafsan@meta.com>
|
|
Verify similar to beb96b0 Break out websockets
Break out the SSE functions into a separate compile unit. This allows
the SSE sockets in beast to be compiled separately, which significantly
reduces the overall compile time by a few seconds. Code is identical
with the exceptions of minor header definitions to convert header-only
to compile unit.
Change-Id: I5aae4f17cbd2badf75b3e0bb644a2309f6300663
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
To support HTTP2 simultaneously on http and https connections, the HTTP
connection classes formerly took the socket as a template option,
allowing passing ssl::stream<tcp::socket> or simply tcp socket. With
the addition of the multiple-sockets option, this would cause two copies
of the template to be instantiated, increasing both compile times and
binary size.
This commit applies the same logic to http2connection as was applied to
HTTPConnection, adding an http type parameter to the constructor, which
allows switching between adapter and adapter.next_level() on each read
or write operation. In compiled code, this means that the connection
classes are only specialized once.
Tested:
When configured for one of each http and https socket and http2
curl --http2 http://<ip>/redfish/v1
succeeds
curl --http2 https://<ip>/redfish/v1 succeeds
Change-Id: I8f33796edd5874d5b93d10a3f253cfadd4f6d7a4
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
This commit attempts to add the concept of an SSL detector from beast,
and add the capability into bmcweb. This allows directing multiple
socket files to the bmcweb instance, and bmcweb will automatically sort
out whether or not they're SSL, and give the correct response. This
allows users to plug in erroneous urls like "https://mybmc:80" and they
will forward and work correctly.
Some key design points:
The HTTP side of bmcweb implements the exact same http headers as the
HTTPS side, with the exception of HSTS, which is explicitly disallowed.
This is for consistency and security.
The above allows bmcweb builds to "select" the appropriate security
posture (http, https, or both) for a given channel using the
FileDescriptorName field within a socket file. Items ending in:
both: Will support both HTTPS and HTTP redirect to HTTPS
https: Will support HTTPS only
http: will support HTTP only
Given the flexibility in bind statements, this allows administrators to
support essentially any security posture they like. The openbmc
defaults are:
HTTPS + Redirect on both ports 443 and port 80 if http-redirect is
enabled
And HTTPS only if http-redirect is disabled.
This commit adds the following meson options that each take an array of
strings, indexex on the port.
additional-ports
Adds additional ports that bmcweb should listen to. This is always
required when adding new ports.
additional-protocol
Specifies 'http', 'https', or 'both' for whether or not tls is enfoced
on this socket. 'both' allows bmcweb to detect whether a user has
specified tls or not on a given connection and give the correct
response.
additional-bind-to-device
Accepts values that fill the SO_BINDTODEVICE flag in systemd/linux,
and allows binding to a specific device
additional-auth
Accepts values of 'auth' or 'noauth' that determines whether this
socket should apply the normal authentication routines, or treat the
socket as unauthenticated.
Tested:
Previous commits ran the below tests.
Ran the server with options enabled. Tried:
```
curl -vvvv --insecure --user root:0penBmc http://192.168.7.2/redfish/v1/Managers/bmc
* Trying 192.168.7.2:80...
* Connected to 192.168.7.2 (192.168.7.2) port 80 (#0)
* Server auth using Basic with user 'root'
> GET /redfish/v1/Managers/bmc HTTP/1.1
> Host: 192.168.7.2
> Authorization: Basic cm9vdDowcGVuQm1j
> User-Agent: curl/7.72.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Location: https://192.168.7.2
< X-Frame-Options: DENY
< Pragma: no-cache
< Cache-Control: no-Store,no-Cache
< X-XSS-Protection: 1; mode=block
< X-Content-Type-Options: nosniff
< Content-Security-Policy: default-src 'none'; img-src 'self' data:; font-src 'self'; style-src 'self'; script-src 'self'; connect-src 'self' wss:
< Date: Fri, 08 Jan 2021 01:43:49 GMT
< Connection: close
< Content-Length: 0
<
* Closing connection 0
```
Observe above:
webserver returned 301 redirect.
webserver returned the appropriate security headers
webserver immediately closed the connection.
The same test above over https:// returns the values as expected
Loaded the webui to test static file hosting. Webui logs in and works
as expected.
Used the scripts/websocket_test.py to verify that websockets work.
Sensors report as expected.
Change-Id: Ib5733bbe5473fed6e0e27c56cdead0bffedf2993
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
base64 decoding comes in two flavors, "normal" which we already
implement, and "url safe" which modifies the alphabet to create base64
encodings that are safe to use in filenames and urls. Functionally this
just involves swapping two characters with underscore and minus in the
encode/decode table. To avoid duplicating a lot of code, this commit
refactors the base64 tables to be generated at compile time.
Tested: Included unit tests pass. No usage until next commit.
Change-Id: I71724fd2e04000f115c22a40d382d411986d7b39
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Copy the latest format file from the docs repository and apply.
Change-Id: I2f0b9d0fb6e01ed36a2f34c750ba52de3b6d15d1
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
Clang-tidy misc-include-cleaner appears to now be enforcing
significantly more headers than previously. That is overall a good
thing, but forces us to fix some issues. This commit is largely just
taking the clang-recommended fixes and checking them in. Subsequent
patches will fix the more unique issues.
Note, that a number of new ignores are added into the .clang-tidy file.
These can be cleaned up over time as they're understood. The majority
are places where boost includes a impl/x.hpp and x.hpp, but expects you
to use the later. include-cleaner opts for the impl, but it isn't clear
why.
Change-Id: Id3fdd7ee6df6c33b2fd35626898523048dd51bfb
Signed-off-by: Ed Tanous <etanous@nvidia.com>
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
SPDX identifiers are simpler, and reduce the amount of cruft we have in
code files. They are recommended by linux foundation, and therefore we
should do as they allow.
This patchset does not intend to modify any intent on any existing
copyrights or licenses, only to standardize their inclusion.
[1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects
Change-Id: I935c7c0156caa78fc368c929cebd0f068031e830
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Previously this function was based on a basic string comparison. This
is fine, but found several inconsistencies, like not handling spaces in
the appropriate places.
This commit creates a new function getContentType, using the new parsing
infrastructure. As doing this, it showed that the existing parser
functions were not handling case insensitive compares for the mime type.
While this is technically not required, it's something we unit test for,
and relatively easy to add.
Note, that because this parser ignores charset, this moves charset=ascii
from something that previously failed, to something that now succeeds.
This is expected.
Tested: Unit tests pass. Good coverage
Change-Id: I825a72862135b62112ee504ab0d9ead9d6796354
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
When sending last-event-id, the previous events were being received
before the the header was completed. This is because the open handler is
being called before the connect call was in place, so you get:
Connection starts
open handler called
sendEvent() called from open handler
sendSSEHeader() called.
This results in a spec violation.
Tested:
curl --user root:0penBmc -vvv -s --no-buffer -k -N -H 'Accept:
text/event-stream' -H 'Last-Event-Id: 4' -X GET
https://localhost:8000/redfish/v1/EventService/SSE
Now succeeds, and wireshark dumps show the header is being sent
correctly. Note that previously this command would fail unless http0.9
header was set.
Unit test coverage for this path without last-event-id passes.
Change-Id: I44bb6eedbcbdc727b257646ec55e808157231f75
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Change-Id: Iefe1b695b86a640d8dfaafd1f77f374fa34246de
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Fix the following clang-tidy errors:
```
../redfish-core/src/filter_expr_executor.cpp:102:21: error: no header providing "nlohmann::json" is directly included [misc-include-cleaner,-warnings-as-errors]
7 | const nlohmann::json& body;
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I2e0d66bb35c1010607b9795d00b3321dc20d6d65
|
|
There are currently 3 function prototypes that hold the name
"sendEvent". This makes them hard to search for, and even though they
take different arguments, and are attached to different classes, they're
still difficult to trace.
Rename two of the classes.
Tested: Code compiles. Rename only.
Change-Id: I5df9c690ba0ca8ebe19c73fc0848e9c3ef4d52f7
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
These were added as part of
d5c80ad9c07b94465d8ea62d2b6f87c30cac765e: test treewide: iwyu
Since then, Nan hasn't been very active on the project, and to my
knowledge, since the initial run, we've never used IWYU again.
clang-include-cleaner seems to work well without needing these pragmas,
and is what we're using, even if it's less useful than IWYU.
Remove all mention of IWYU.
Tested: Code compiles.
Change-Id: I06feedeeac9a114f5bdec81d59ca83223efd8aa7
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
This commit is automatically generated by enabling clang-include-fixer.
Tested: Code compiles.
Change-Id: I475d7b9d43e95bbdeeaadf11905d3b2a60aa8ef3
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
clang-format-18 isn't compatible with the clang-format-17 output, so we
need to reformat the code with the latest version. The way clang-18
handles lambda formatting also changed, so we have made changes to the
organization default style format to better handle lambda formatting.
See I5e08687e696dd240402a2780158664b7113def0e for updated style.
See Iea0776aaa7edd483fa395e23de25ebf5a6288f71 for clang-18 enablement.
Change-Id: Iceec1dc95b6c908ec6c21fb40093de9dd18bf11a
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
The Redfish spec require filtering of SSE entries to be supported.
This commit rearranges the code, and implements SSE sorting as well
as support for Last-Event-Id. To do this it adds a dependency on
boost circular_buffer.
Tested:
SSE connections succeed. Show filtered results.
Change-Id: I7aeb266fc40471519674c7b65cd5cc4625019e68
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
In the initial implementation of metadata indexing the bmc knew at
compile time what schemas it could potentially publish. bmcweb took the
approach of adding all schemas of all versions to the $metadata
resource. Since that was made, two major changes have happened.
First, Redfish has added significantly more versions of each schema, as
well as significantly more schemas to the point where the metadata index
is now 213KB. While this file compresses fairly well, the size is
obvious from the large amount of time that redfish service validator
takes to parse the schemas, compared to actually acquiring BMC redfish
resources.
Second, aggregation was added, where an aggregated Redfish service might
implement any number of schemas, including OEM ones.
In an effort to fix this, this patch takes the compile-time algorithm in
update_schemas.py, and moves it into bmcweb itself, parsing the files on
disk as needed on demand. This has some immediate benefits; First, is
that now schemas can be potentially installed from anywhere, not only
from within the bmcweb build, and they will be resolved at runtime.
Second, patches that want to add support for a given schema need to only
symlink the schema into the correct folder, without needing to rerun
update_schemas.py. This saves time in review.
Finally, this opens to door to reducing the schema versions present in
the metadata to the unique set of only what this bmcweb instance, and
its aggregated BMCs expose.
Tested: Redfish service validator passes. Need A/B checking to verify
the file is byte for byte the same.
GET /redfish/v1/$metadata returns what looks like sane results, with a
correct content-type.
Unit tests require the use of TemporaryFileHandle, so that class is
moved into a more general folder, outside of test/http.
Change-Id: I326159099c6b6c4056023b2e173c5f074ed88ce1
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This is an attempt to solve a class of use-after-move bugs on the
Request objects which have popped up several times. This more clearly
identifies code which owns the Request objects and has a need to keep it
alive. Currently it's just the `Connection` (or `HTTP2Connection`)
(which needs to access Request headers while sending the response), and
the `validatePrivilege()` function (which needs to temporarily own the
Request while doing an asynchronous D-Bus call). Route handlers are
provided a non-owning `Request&` for immediate use and required to not
hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails).
Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In the change made to move to std::format, we defined some custom type
formatters in logging.hpp. This had the unintended effect of making
all compile units pull in the majority of boost::url, and nlohmann::json
as includes.
This commit breaks out boost and json formatters into their own separate
includes.
Tested: Code compiles. Logging changes only.
Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This TODO has been in bmcweb for a very long time. Implement it.
W3 sets rules for what security policies apply to which content
types[1]. Reading through this, essentially CSP should only apply to
HTML files.
Tested: Unit tests pass. Webui loads properly. Chrome network window
Shows headers show up as expected.
[1] https://www.w3.org/TR/CSP2/#which-policy-applies
Change-Id: I5467d0373832668763c72a66da2a8872e07bfb58
Signed-off-by: Ed Tanous <ed@tanous.net>
|
