Skip to content

Allow disabling Authorization for bundle downloads#174

Merged
ejoerns merged 3 commits into
rauc:masterfrom
fardke:fardke/feature/disable_auth_header_for_downloads
Jul 11, 2025
Merged

Allow disabling Authorization for bundle downloads#174
ejoerns merged 3 commits into
rauc:masterfrom
fardke:fardke/feature/disable_auth_header_for_downloads

Conversation

@fardke

@fardke fardke commented Mar 15, 2024

Copy link
Copy Markdown
Contributor

Hello,

To be able to download bundle from some external storage (like aws S3 or azure blob storage) it can be useful to not send Authorization header.

In order to test this modification correctly I add a sniffer (using scapy module). It's allow us to check requests send by rauc-hawkbit-updater to hawkbit container and validate headers. To make it works we must add specific capabilities. It's annoying, but I don't find any alternatives.

@Bastian-Krause Bastian-Krause left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the long waiting time.

The feature itself looks good. Please see my comments below.

I don't really like the scapy testing approach. We could add an nginx include instead that makes nginx respond with an error if auth headers are detected, mimicking what the problematic storage providers do. If you'd like to add that, please base your work on #188.

Comment thread docs/reference.rst Outdated
Comment thread docs/using.rst Outdated
Comment thread include/config-file.h Outdated
Comment thread src/config-file.c Outdated
Comment thread src/config-file.c Outdated
@Bastian-Krause Bastian-Krause added the enhancement New feature or request label Mar 12, 2025
@fardke
fardke force-pushed the fardke/feature/disable_auth_header_for_downloads branch from cfca7ac to 98f7cfe Compare March 12, 2025 18:53
@fardke

fardke commented Mar 12, 2025

Copy link
Copy Markdown
Contributor Author

Sorry for the long waiting time.

The feature itself looks good. Please see my comments below.

I don't really like the scapy testing approach. We could add an nginx include instead that makes nginx respond with an error if auth headers are detected, mimicking what the problematic storage providers do. If you'd like to add that, please base your work on #188.

Ok I think it's done. Tell me if it's ok for you.

Thank you.

@fardke
fardke requested a review from Bastian-Krause March 12, 2025 18:56
@Bastian-Krause
Bastian-Krause force-pushed the fardke/feature/disable_auth_header_for_downloads branch from 98f7cfe to 21ec155 Compare March 13, 2025 17:08
@Bastian-Krause
Bastian-Krause marked this pull request as draft March 13, 2025 17:08
@Bastian-Krause

Bastian-Krause commented Mar 13, 2025

Copy link
Copy Markdown
Member

Thanks for your work!

I've..

  • rebased this on latest master (preventing CI failures due to test: conftest: drop anonymous download hawkBit option #190)
  • reworded the documentation and commit messages
  • made nginx return 401 (I think that's more decriptive than 500)
  • reworked the test to serve the bundle externally (this should be more stable as stumbling over wrong checksum as we did before)
  • added a test that makes rauc-hawkbit-updater fail with the 401 error case.

I think this can be merged once #188 is merged (I've marked this as draft to prevent premature merging) if @ejoerns has no objections.

@fardke

fardke commented Mar 18, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for adjustments.

@Bastian-Krause
Bastian-Krause force-pushed the fardke/feature/disable_auth_header_for_downloads branch from 21ec155 to 1bbf979 Compare July 8, 2025 12:40
@Bastian-Krause
Bastian-Krause removed their request for review July 8, 2025 12:40
@Bastian-Krause
Bastian-Krause force-pushed the fardke/feature/disable_auth_header_for_downloads branch from 1bbf979 to 9d4804c Compare July 10, 2025 11:52
@Bastian-Krause
Bastian-Krause marked this pull request as ready for review July 10, 2025 11:53
fardke and others added 3 commits July 10, 2025 14:31
Adds a `send_download_authentication` option defaulting to `true`, keeping
the previous behavior. If set to false, no authentication data is
sent. This is useful for artifact downloads from external storage
providers.

Signed-off-by: Kevin Fardel <kevin.fardel@rtone.fr>
[bst: rewording of documentation and commit message]
Signed-off-by: Bastian Krause <bst@pengutronix.de>
We add an nginx include file to send an HTTP 401 error if the test bundle
is downloaded with "authorization" header. Then we run
rauc-hawkbit-updater with `send_download_authentication=false` to verify
that authorization headers are not sent.

Signed-off-by: Kevin Fardel <kevin.fardel@rtone.fr>
[bst: make nginx exit with 401, fix docs, rework test to serve the
bundle externally]
Signed-off-by: Bastian Krause <bst@pengutronix.de>
Test that rauc-hawkbit-updater fails when sending authentication header
to external storage provider with send_download_authentication=true.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
@Bastian-Krause
Bastian-Krause force-pushed the fardke/feature/disable_auth_header_for_downloads branch from 9d4804c to 5227e50 Compare July 10, 2025 12:37
@Bastian-Krause

Bastian-Krause commented Jul 10, 2025

Copy link
Copy Markdown
Member

Now that #169 is merged, setting the new send_download_auth_header=true also prevents sending SSL client certificate authentication for bundle downloads. Since there are no headers involved, I've renamed the option from send_download_auth_header to (the more generic variant) send_download_authentication.

@Bastian-Krause

Copy link
Copy Markdown
Member

Since I've modified the PR quite heavily myself, I'd like a review from @ejoerns before merging.

@Bastian-Krause
Bastian-Krause requested a review from ejoerns July 10, 2025 12:44
@Bastian-Krause Bastian-Krause changed the title Allow disabling Authorization headers for bundle downloads Allow disabling Authorization for bundle downloads Jul 10, 2025
@ejoerns
ejoerns merged commit 350ba55 into rauc:master Jul 11, 2025
9 checks passed
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.

3 participants