Repository navigation
export-webhook: a userinfo in the URL is sent as authorization: Basic, as 1.5.5 sent it (audit export-webhook #1, HIGH) - #19
Open
MattJackson wants to merge 1 commit into
Open
MattJackson wants to merge 1 commit into
MattJackson wants to merge 1 commit into
Conversation
…, as 1.5.5's reqwest 0.12 sent it Audit export-webhook #1 (HIGH), ARCHITECT ruling (c). 1.5.5 POSTed through reqwest 0.12, whose RequestBuilder::new (extract_authority) took the URL's username and password off the URL, percent-decoded each (a username that is not UTF-8 sends nothing, a password that is not is left out) and appended basic_auth: 'Basic ' + padded standard base64 of '{user}:' then the password. The sink now builds that field from its own url setting and sends it first on every POST, then content-type, then the configured auth header, appended as http::HeaderMap appends: a configured header also named Authorization is sent too, after the userinfo's, as 1.5.5 sent both. The host's connector drops the userinfo from the target the url setting gives the need (busbar, same audit row). Log masking unchanged. Parity: a_userinfo_in_the_url_is_sent_as_basic_as_1_5_5_sent_it (reqwest's decoding and encoding cases, RFC 4648 vectors), the_userinfo_field_rides_beside_a_configured_auth_header_as_in_1_5_5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit pinned export-webhook finding #1 (HIGH), ARCHITECT ruling condition (c).
What. A webhook URL carrying a userinfo (
https://user:pass@host/path) is sent as 1.5.5 sent it:authorization: Basic <base64(user:pass)>on every POST.Why. THE DESIGN §2, "A port keeps 1.5.5's surface ... the default is 1.5.5's behaviour". 1.5.5 POSTed through reqwest 0.12. Its
RequestBuilder::newcalledextract_authority(reqwest-0.12.28src/async_impl/request.rs:169-183,:581-605), which took the URL's username and password off the URL and appendedbasic_auth(src/util.rs:4-25). After the port, the sink sent no credential, and the host refused the need's userinfo-carrying target, so the sink was disabled.How (
export-webhook/src/lib.rs).basic_authorization(url)follows reqwest step by step:Basicplus padded standard base64 of{user}:followed by the password, if any.HeaderMapthe 1.5.5 request held: the Basic field first, thencontent-type, then the configuredauth_header.HeaderMap::appendappends them. A configured header also namedAuthorizationis sent as well, right after the Basic field, which is what 1.5.5 sent.Host side. The connector stops refusing the userinfo in a target that comes from the plugin's own settings through
target_from, and drops it from the target instead (GetBusbar/busbar#842, which also moves busbar's pin to this head). Until busbar's pin carries that change, the host still refuses such a URL. The sink's code here is correct either way.REDs (
export-webhook/src/tests.rs).a_userinfo_in_the_url_is_sent_as_basic_as_1_5_5_sent_itcovers reqwest's decoding and encoding cases: user:pass, user only, empty password, password only,%XX, raw non-ASCII, a stray%, non-UTF-8 password, non-UTF-8 username, no userinfo, and not a URL. It also checks the RFC 4648 vectors.the_userinfo_field_rides_beside_a_configured_auth_header_as_in_1_5_5checks field order and precedence withAuthorizationand with another header name. The target stays path plus query.