Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HTTP Streaming Support #130

Merged
merged 8 commits into from
Oct 13, 2022
Merged

Conversation

Bastian-Krause
Copy link
Member

@Bastian-Krause Bastian-Krause commented Apr 21, 2022

RAUC supports HTTP streaming since rauc/rauc#755 (included in rauc >= 1.7). Documentation is available here. This PR aims to add support for HTTP streaming to rauc-hawkbit-updater.

Add a new config option stream_bundle. If set, rauc-hawkbit-updater does not download the RAUC bundle, but passes the bundle URL along with the authentication header directly to RAUC. RAUC streams the bundle during installation.

The install/cancel tests have been adjusted to run with and without HTTP streaming.

Note that the default hawkBit configuration limits the number of HTTP range requests to ~1000 per action and 200 per second. The configuration suggestion and the configuration in the GitHub actions workflow have been adjusted to disable these limitations. Even with this, hawkBit generates an "ActionStatus" for each range request. This needs discussion with upstream - this is the reason for marking this PR "RFC" for the time being.

@Bastian-Krause
Copy link
Member Author

Upstream discussion: eclipse-hawkbit/hawkbit#1249

@jluebbe
Copy link
Member

jluebbe commented Apr 22, 2022

I've looked mainly at the part related to RAUC, which looks good to me. We won't change the API for streaming in RAUC, so I don't see a reason to wait for a release before merging this (it needs to be enabled explicitly here anyway).

@Bastian-Krause Bastian-Krause force-pushed the bst/http-streaming branch 2 times, most recently from 23ce959 to e222a44 Compare May 2, 2022 12:50
@Bastian-Krause
Copy link
Member Author

  • rebased
  • docs: added notes to other config options when used with http_streaming=true

@Bastian-Krause
Copy link
Member Author

  • added: pass through value of ssl_verify config option as tls-no-verify argument to InstallBundle() D-Bus method for streaming installations
  • drop note in reference about ssl_verify having no effect when combined with http_streaming=true

@ejoerns
Copy link
Member

ejoerns commented May 27, 2022

Can now be rebased on #129 (as part of master)

@Bastian-Krause
Copy link
Member Author

  • rebased on master

@ejoerns ejoerns self-assigned this Aug 27, 2022
@Bastian-Krause Bastian-Krause marked this pull request as ready for review August 29, 2022 10:10
@Bastian-Krause
Copy link
Member Author

I received no responses in eclipse-hawkbit/hawkbit#1249. I guess we can review/merge this anyway.

@BubuOT
Copy link

BubuOT commented Oct 4, 2022

Any chance you can make a new release with this included, now that rauc 1.8 with incremental updates is out? :-)

@Bastian-Krause Bastian-Krause requested a review from ejoerns October 5, 2022 08:21
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me, too and successfully tested on my target hardware.

👍

I am not fully sure yet if http_streaming is a proper config value for this feature because of its scope.

What we intend to say is that RAUC handles the download/streaming (instead of rauc-hawkbit-updater). And using HTTP is our scope anyway.

Thus maybe something like rauc_streaming or rauc_bundle_streaming, or stream_bundle?

Or even download-mode=streaming vs download-mode=direct or similar with the benefit we can require an explicit decision and say what the alternatives are.

These are just thoughts, if all are happy with http_streaming I'd also take it 😏

A minor addition I have is that bundle_download_location is still mandatory which quite confusing when using streaming mode. But this could also be made as part of another PR.

I would also like to see this feature more prominent in either the docs or at least in the example in README.md.

@Bastian-Krause
Copy link
Member Author

Looks fine for me, too and successfully tested on my target hardware.

Thanks for reviewing and testing!

I am not fully sure yet if http_streaming is a proper config value for this feature because of its scope.

What we intend to say is that RAUC handles the download/streaming (instead of rauc-hawkbit-updater). And using HTTP is our scope anyway.

Thus maybe something like rauc_streaming or rauc_bundle_streaming, or stream_bundle?

Or even download-mode=streaming vs download-mode=direct or similar with the benefit we can require an explicit decision and say what the alternatives are.

Yep, naming is hard. I think I like stream_bundle, but maybe download-mode is even better. I'm not really happy with direct, though. Maybe we can come up with another variant for that?

These are just thoughts, if all are happy with http_streaming I'd also take it smirk

A minor addition I have is that bundle_download_location is still mandatory which quite confusing when using streaming mode. But this could also be made as part of another PR.

Correct, I had not thought of this. Let me add a commit for that here.

I would also like to see this feature more prominent in either the docs or at least in the example in README.md.

I'll see how we can feature the bundle streaming better. Suggestions welcome :)

@ejoerns
Copy link
Member

ejoerns commented Oct 5, 2022

Yep, naming is hard.

Fully agreed.

I think I like stream_bundle, but maybe download-mode is even better. I'm not really happy with direct, though. Maybe we can come up with another variant for that?

I don't like direct either but it was the first thing that came to my mind.

Maybe download-mode=separate or download-mode=temp-location or download-mode=store or download-mode=conventional ...

Or we stick to stream_bundle=true. More opinions welcome.

These are just thoughts, if all are happy with http_streaming I'd also take it smirk
A minor addition I have is that bundle_download_location is still mandatory which quite confusing when using streaming mode. But this could also be made as part of another PR.

Correct, I had not thought of this. Let me add a commit for that here.

👍

I would also like to see this feature more prominent in either the docs or at least in the example in README.md.

I'll see how we can feature the bundle streaming better. Suggestions welcome :)

For the README.md it might be sufficient to note the key in the example config, like

# enable_streaming=true

For the documentation I would add a Streaming Support chapter above the Plain Bundle Support chapter.

It could say something like

By default, the rauc-hawkbit-updater will first download the bundle to a temporary
storage location and the invoke RAUC to install the bundle.
To not require reserving extra storage and also potentially save download bandwidth
(together with adaptive updates), the rauc-hawkbit-updater can also leverage RAUC's
built-in HTTP streaming support.

To enable it, set ``enable_streaming=true`` in your rauc-hawkbit-updater configuration.

.. note: rauc-hawkbit-updater will add required authentication headers and options to
   the resulting RAUC D-Bus InstallBundle API call.

@ejoerns ejoerns added this to the Release 1.3 milestone Oct 5, 2022
In a future commit, the authentication header will not only be needed by
the download functionality, but also by a function starting the RAUC
streaming installation. This is because RAUC needs to know the
authentication header to pass to hawkBit in order to download the
bundle.

To prepare for that, move the authentication header generation into a
dedicated function that can be called from both code paths.

Signed-off-by: Bastian Krause <[email protected]>
The authentication header needs to be passed to the InstallBundle() DBus method
invoking the RAUC streaming installation. In order to do that, add an
`auth_header` argument to rauc_install() and make it pass the header via
the `install_context` struct. The header is finally passed to
InstallBundle() [1] as "http-headers" argument.

For now, `rauc_install()` is called without an actual authentication
(NULL). This will be changed in future commit.

[1] https://rauc.readthedocs.io/en/latest/reference.html#gdbus-method-de-pengutronix-rauc-installer-installbundle

Signed-off-by: Bastian Krause <[email protected]>
…dle HTTP streaming

For RAUC HTTP streaming installations, the hawkBit authentication header
needs to be passed, so RAUC can access the bundle on the hawkBit server.

A previous commit added authentication header passing from
`rauc_install()` to the DBus method. Now allow passing headers in the
hawkbit-client component via `on_new_software_userdata` as a preparation.
The download thread does not need to pass a header, so pass NULL instead.
A future commit will add streaming installation support, which then
passes an actual authentication header.

Signed-off-by: Bastian Krause <[email protected]>
…verify

RAUC's InstallBundle() D-Bus method supports the "tls-no-verify"
argument. If set to true, server certificate verification errors are
ignored during HTTP streaming installations. This option is similar to
rauc-hawkbit-updater's "ssl_verify" option with the inverse meaning.

So pass the value of the "ssl_verify" config option to the
InstallBundle() call for streaming installations.

Signed-off-by: Bastian Krause <[email protected]>
@Bastian-Krause
Copy link
Member Author

Bastian-Krause commented Oct 13, 2022

Force-pushed:

  • rebased on latest master
  • rename option http_streaming -> stream_bundle, adjust variables that are named accordingly
  • allow omitting download_location when stream_bundle is enabled
  • add basic test without download_location and without stream_bundle options
  • add docs section about streaming supported as suggested by @ejoerns

Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming, adapted tests and added added docs look fine for me.

Add a new config option `stream_bundle`. If set, rauc-hawkbit-updater
does not download the RAUC bundle, but rather passes the bundle URL along
with the authentication header directly to RAUC. RAUC streams the bundle
during installation [1].

This is implemented via `start_streaming_installation()` which skips the
download thread and starts the install thread via rauc_install()
directly.

[1] https://rauc.readthedocs.io/en/latest/advanced.html#http-streaming

Signed-off-by: Bastian Krause <[email protected]>
RAUC HTTP streaming works by translating NBD read requests into HTTP
range requests [1]. The default squashfs block size is 128 KB [2]. The
ranges requested happen to be in the same order of magnitude.

With this in mind, certain default configurations in hawkBit do not work
out of the box:

hawkBit tries to prevent DoS attacks [3]:

  "hawkBit owns the feature of guarding itself from DoS attacks, a DoS
  filter. It reduces the maximum number of requests per seconds which can
  be configured for read and write requests." [4]

This might affect RAUC HTTP bundle streaming because it causes a high
number of HTTP range requests within a time period:

  Registered DOS attack! Client [0:0:0:0:0:0:0:1] is above configured READ request threshold (200)!

The requests are authenticated, so it should be safe to disable the DoS
protection.

hawkBit generats an "ActionStatus" entity for each HTTP range request.
When using RAUC HTTP streaming, a huge number of those requests are
performed. It is likely that the default quota limit of allowed
"ActionStatus" entities is hit:

  Cannot assign 1 ActionStatus entities to Action '1' because of the configured quota limit 1000. Currently, there are 1000 ActionStatus entities assigned.

To allow requesting an unlimited number of ranges per bundle, disable
the "status entries per action quota" altogether.

Generating an "ActionStatus" per range request is probably not a good
idea for this use case. This needs discussion with upstream.

[1] https://rauc.readthedocs.io/en/latest/advanced.html#http-streaming
[2] https://www.kernel.org/doc/html/latest/filesystems/squashfs.html
[3] https://github.com/eclipse/hawkbit/blob/master/hawkbit-http-security/src/main/java/org/eclipse/hawkbit/security/DosFilter.java
[4] https://www.eclipse.org/hawkbit/guides/clustering/

Signed-off-by: Bastian Krause <[email protected]>
Distinguish simulating "normal" RAUC installations and RAUC HTTP
streaming installations by looking at the argument's "http-headers" key.
If it is set, expect an URL as source argument, not a local file.
For streaming installations, download the bundle via 128 KB range
requests just like RAUC would do. Make sure that the checksum matches.

Signed-off-by: Bastian Krause <[email protected]>
Most of the install and cancel test can be run in "normal" download mode
and streaming installation mode. Use pytest.mark.parametrize to realize
that. Use the 'download'/'streaming' strings as mode argument. That way
the tests get a human-readable parameter, e.g.:

  test/test_install.py::test_install_maintenance_window[download]
  test/test_install.py::test_install_maintenance_window[streaming]

While at it, disable the anonymous download option explicitly. That way
it is ensured that hawkBit actually checks the authentication header on
bundle download. This is the default, but better set it explicitly.

Also add a test case making sure that an appropriate error is returned
if neither 'bundle_download_location' nor 'stream_bundle' are given.

Signed-off-by: Bastian Krause <[email protected]>
@Bastian-Krause
Copy link
Member Author

Force-pushed:

  • Use single quotes in error messages around option names for better comprehensibility (as suggested above)

Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me now 🚀

@ejoerns ejoerns merged commit 12ace4e into rauc:master Oct 13, 2022
@Bastian-Krause Bastian-Krause deleted the bst/http-streaming branch October 13, 2022 14:57
Bastian-Krause added a commit to Bastian-Krause/rauc-hawkbit-updater that referenced this pull request Oct 14, 2022
We've missed adding this option in rauc#130.

Signed-off-by: Bastian Krause <[email protected]>
Bastian-Krause added a commit to Bastian-Krause/rauc-hawkbit-updater that referenced this pull request Oct 24, 2022
As explained in rauc#130, bundle streaming with hawkBit does not work work
with hawkBit's default configuration (for most cases). Until now, the
configuration options were only added to the test suite section of the
README. No explanation was provided (not considering the PR and the
commit messages).

To change that, add all required configuration options and an
explanation to the reference docs of `stream_bundle`.

While at it, also mention the default value of the option.

Signed-off-by: Bastian Krause <[email protected]>
Bastian-Krause added a commit to Bastian-Krause/rauc-hawkbit-updater that referenced this pull request Nov 9, 2022
…aming installation

Since the integration of RAUC streaming support in rauc-hawkbit-updater
[1] the "bundle_download_location" config option can be ommited when the
"stream_bundle" config option is enabled [2].

The value of the "bundle_download_location" config option is still used
in streaming installation code paths leading to errors such as:

  g_file_test: assertion 'filename != NULL' failed

To fix this, exit early in bundle clean up code and omit the check for
free space if the "stream_bundle" config option is enabled.

[1] rauc#130
[2] fe30f97 ("hawkbit-client/config-file: implement http streaming support")

Fixes: fe30f97 ("hawkbit-client/config-file: implement http streaming support")
Signed-off-by: Bastian Krause <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants