Split proxy/client serials based on lower bit#57
Merged
Conversation
sophie-h
commented
Apr 7, 2024
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
2 times, most recently
from
aa36e27 to
5816eaf
Compare
Contributor
Author
|
What changes with this is that clients can re-use serials. I can't guarantee that this doesn't has potential security implications since I'm not familiar enough with the codebase. |
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
5816eaf to
b76beeb
Compare
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
b76beeb to
23581ea
Compare
sophie-h
changed the title
mcatanzaro
reviewed
Apr 12, 2024
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
23581ea to
d36e8b0
Compare
Contributor
|
Thanks for writing a patch! |
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
2 times, most recently
from
225133d to
184b2ce
Compare
swick
reviewed
Apr 15, 2024
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
184b2ce to
779235e
Compare
swick
reviewed
Apr 16, 2024
Contributor
|
LGTM but we should definitely get another review in. |
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
779235e to
935eeda
Compare
Separation between messages created by client and proxy was ensured by requiring the client to use monotonically increasing serials and adding an offset to distinguish the client message from proxy messages. The requirement to only send messages with increasing serials cannot be ensured by libraries godbus or zbus. This commit instead reserves the high-bit=0 space for client messages and the high-bit=1 for messages created by the proxy. This gets rid of any serial translation mechanism and the requirement for increasing serials. However, it limits the possible values of serials available to the client to about half of the usual maximum value. Closes flatpak#46
sophie-h
force-pushed
the
wip/sophie-h/even-odd-serials
branch
from
935eeda to
4f081c7
Compare
Contributor
Author
|
Rebased due to merge conflict. |
alexlarsson
reviewed
May 3, 2024
alexlarsson
reviewed
May 3, 2024
Member
|
Some minor comments from me, but otherwise this looks good to me. |
Member
|
Well, one more comment: Maybe MAX_CLIENT_SERIAL can be made much much larger. We're only emitting "fake" requests at the beginning of the session when setting up the view of bus names, so thre is never going to be 2^31 of them. I think MAX_CLIIENT_SERIAL should be set to something like |
Don't split via lower bit of the serial but instead use a much higher number for MAX_CLIENT_SERIAL since there are very few fake requests we need to do.
Contributor
Author
|
I have changed the code to #define MAX_CLIENT_SERIAL (G_MAXUINT32 - 65536) |
Member
|
lgtm |
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
5 participants
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.
Separation between messages created by client and proxy was ensured by
requiring the client to use monotonically increasing serials and adding
an offset to distinguish the client message from proxy messages.
The requirement to only send messages with increasing serials cannot be
ensured by libraries godbus or zbus.
This commit instead reserves the lower_bit=0 space for client messages
and the lower_bit=1 for messages created by the proxy. This gets rid of
any serial translation mechanism and the requirement for increasing serials.
However, it limits the possible values of serials available to the client
to about half of the usual maximum value.
Closes #46