Repository navigation
Check the issuer of every stream the transmitter returns - #20
Merged
Merged
Conversation
A review of the Go and Python ports found that only a created stream had its issuer checked. GetStreamAsync, GetStreamsAsync, UpdateStreamAsync and ReplaceStreamAsync of SsfStreamClient now throw SsfStreamIssuerMismatchException as CreateStreamAsync did, as the SETs of a stream with another issuer would not verify (SSF 1.0, section 8.1.1). SsfStreamRegistrar deletes a stream only when it created it with the wrong issuer, and leaves an existing one alone; it fails without retrying either way. Ported from easyssf b9acc3a. Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
The poll request carries the access token, but the poll endpoint was not checked: one configured or taken from the stream with plain http on any host received the token. SsfPoller now holds it to the new SsfPollerOptions.UriPolicy (https, or http on loopback addresses only) in the one method that sends poll requests, so the acknowledgements flushed on stop pass the same check. The hosting integration sets the policy from AllowInsecureHttp and validates Poll:EndpointUrl at startup. A SET the poll response lists under a key other than its jti (RFC 8936, section 2.4) is reported as invalid_request under that key instead of being handled and acknowledged under a key that is not its own. Ported from easyssf b9acc3a and 910cd24. Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
HttpClient buffers up to 2 GiB of whatever a response carries, so a faulty or hostile transmitter could make the receiver hold a body of its choosing. Metadata, JWK Set, token, stream configuration and poll responses come nowhere near 4 MiB. The new setting EasySsf:Receiver:Http:MaxResponseSize, 4 MiB by default and validated on start, becomes MaxResponseContentBufferSize of the receiver's HTTP clients; every call of the receiver buffers its response, so a larger one fails the call as if the transmitter were unavailable. The README tells a framework-free receiver to bound its own HttpClient. Ported from easyssf b9acc3a. Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
A registration that completed after StopAsync(), because its HttpMessageHandler did not observe the cancellation, still published its stream, and StopAsync() deleted the stream the receiver knew even when a previous stop had deleted it already. Each Start() now begins a run of its own; a registration publishes only while it is the current run, under a lock StopAsync() takes too, so it publishes before the stop goes on or not at all. A registration that outlives its stop deletes a stream it created on the way, or, if the registrar was started again, leaves it to the registration that followed, which adopts it if it finds it and deletes it once it publishes another. A stream whose deletion is under way is not adopted. Orphans are deleted outside the lock and without the stop's cancellation. StopAsync() deletes only a stream published since the last stop and warns after the new StopTimeout. TestTransmitter gets SeveralStreamsAllowed. The README notes what stream management means for several instances. Ported from easyssf b9acc3a, 910cd24, cae740f, b18b62f, c8e0c9c, d7b82ee, 5754b08 and 88c8d60. Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
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.
Ports the hardening of easyssf 0.7.0, which came out of a review of the Go and Python ports.
SsfStreamClientchecks the issuer of every stream the transmitter returns, not only of a created one. The registrar deletes only a stream it createdwith the wrong issuer.
https, orhttpon loopback addresses, unlessAllowInsecureHttpis set). Aconfigured
Poll:EndpointUrlis checked at startup. A polled SET listed under a key other than itsjtiis reported asinvalid_requestinstead of being handled.EasySsf:Receiver:Http:MaxResponseSize), applied asMaxResponseContentBufferSizeofthe receiver's HTTP clients.
StopAsync()publishes nothing and leaves no stream behind. A stream it created goes to the registration thatfollowed, or gets deleted.
StopAsync()deletes only a stream published since the last stop. New:StopTimeout, andTestTransmitter.SeveralStreamsAllowed.Not ported because .NET doesn't have the bugs: the loopback check (
Uri.IsLoopbackalready rejects127.attacker.example), the pending-acks gauge readingNaN, and thepoller thread surviving a restart.
Fixes #19
Signed-off-by: Thomas Darimont thomas.darimont@googlemail.com