-
-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How does signature verification work in pre-v6 rooms #1988
Comments
To be clear: the problem with v5 rooms and earlier is that they allow JSON fields in events to contain values which are numbers outside the range We don't currently have a good name for this pseudo-canonicaljson that Synapse produces; in any case, the question here is what exactly that format is.
That's about right, unfortunately, though it's not to say we can't make an attempt at documenting the behaviour, if we can figure out what that behaviour is. It's not hard to figure out the behaviour with nothing more complicated than a Python REPL. For example:
So:
Nor do we, really, but I believe it is consistent between Synapse versions and pre-v6 room versions. (Synapse used to accept other values like It's possible that some very old python versions did weird things with floats (particularly, I don't think the transition to exponential format always happened at 1e+16). Again, I don't think we need to worry about them. |
It isn't exactly the same, but we have documented some historical oddities due to Synapse implementation. This could require delving a bit into the Python implementation to see what it does in different situations, unfortunately. |
It's very good to hear that the deviation from canonical json is fairly limited! A couple clarifications:
"don't think we need to worry about them" meaning that we would fully specify the encoding format for pre-v6 rooms, and if some synapse servers have different behavior due to an old python version, they would be non-compliant?
The issue mentioned in that link wouldn't affect signature validation, right? The choice of string/integer would be preserved when encoding for validation. Are you saying that there may be other oddities that do affect signatures that we would need to read through the implementation to find? |
Yes. |
I'm saying there is precedent for adding this information into the spec. What I linked to is not related to signature validation (as long as you round-trip the encoding properly). |
Link to problem area:
Signing JSON, from the v1.12 spec.
Issue
The spec says
Then the "signing details" subsection says
Then the "checking for a signature" subsection says
I read this as "there is no requirement that canonical JSON encoding is used when signing events in pre-v6 rooms, and servers must be able to verify events that were originally signed using any JSON encoding". This is a problem, because a compliant server would need to iterate over all allowed encodings when checking signatures and can only reject an event after testing all of them. There are an unlimited number of encoding allowed by the current spec.
My outside-of-the-spec-text understanding is that the real requirement on encoding for pre-v6 rooms is "exactly what synapse does", which is something based on python's stdlib json encoder. I do not know whether this encoding is consistent between synapse versions, python versions, or pre-v6 room versions.
The text was updated successfully, but these errors were encountered: