Don't rewrite "self-addressed" privileged IQs as results. #4348
Conversation
This file contains 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
process_privilege_iq is meant to rewrite the result of a privileged IQ into the forwarded form required by XEP-0356 so it can be routed back to the original privileged requester. It checks whether the impersonated JID (`ReplacedJid`) of the original request matches the recipient of the IQ being processed to determine if this is a response to a a privileged IQ (assuming it has privileged-IQ metadata attached). Unfortunately, it doesn't check the packet type, and this check will also match a privileged-IQ _request_ that is being sent to the same user that's being impersonated. This results in the request itself being rewritten and forwarded back to the sending component, instead of being processed and having the result send back. Instead, just check for IQ results (either a regular result or an error), and as long as it is marked as being a response to a privileged-IQ, always rewrite it and forward it to the sending component. There's no circumstance under which we _shouldn't_ forward a privileged-IQ response, so we don't need to be tricky about checking whether impersonated-user and recipient match.
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.
As described in #4347:
process_privilege_iq is meant to rewrite the result of a privileged IQ into the forwarded form required by XEP-0356 so it can be routed back to the original privileged requester. It checks whether the impersonated JID (
ReplacedJid) of the original request matches the recipient of the IQ being processed to determine if this is a response to a a privileged IQ (assuming it has privileged-IQ metadata attached).Unfortunately, it doesn't check the packet type, and this check will also match a privileged-IQ request that is being sent to the same user that's being impersonated. This results in the request itself being rewritten and forwarded back to the sending component, instead of being processed and having the result send back.
Instead, just check for IQ results (either a regular result or an error), and as long as it is marked as being a response to a privileged-IQ, always rewrite it and forward it to the sending component. There's no circumstance under which we shouldn't forward a privileged-IQ response, so we don't need to be tricky about checking whether impersonated-user and recipient match.