Skip to content
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

Identity servers do not always return standard Base64 #1999

Open
zecakeh opened this issue Jan 21, 2025 · 4 comments
Open

Identity servers do not always return standard Base64 #1999

zecakeh opened this issue Jan 21, 2025 · 4 comments

Comments

@zecakeh
Copy link
Contributor

zecakeh commented Jan 21, 2025

As per matrix-org/matrix-spec#2060, Sydent can return both standard and url-safe base64-encoded public keys, but we only expect standard base64 so they fail to deserialize.

Note that both Synapse and Sydent use the python-unpaddedbase64 that decodes both indifferently.

This affects:

ruma_identity_service_api::invitation::store_invitation::v2::PublicKey should be affected too except that it doesn't use Base64 but String.

I am not sure if we should be using Base64 in this case. The problem that I see with this type is that it decodes the string during deserialization and reencodes it during serialization, which can be lossy (regarding to padding). But the server might store the base64-encoded key (this is was Sydent does), so it might not be able to find the key if it does not use the exact same string, on the endpoints to check the validity of the key.

It should be noted that RoomThirdPartyEventContent and ruma_identity_service_api::invitation::store_invitation::v2::Response use their own PublicKey type but it should probably be the same type.

@jplatte
Copy link
Member

jplatte commented Jan 21, 2025

I propose we add struct StupidSydentCompatibleBase64(pub String); with relevant methods like fn decode(&self) -> Result<Vec<u8>, _>.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jan 21, 2025

I like the type name. 😛

@zecakeh
Copy link
Contributor Author

zecakeh commented Jan 23, 2025

Note that state-res is also affected as we try to decode the base64 as part of the authorization check.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jan 25, 2025

Actually, the only place that needs to be able to decode the string should be the homeserver, when it needs to check the signature.

That would mean that ruma_signatures::verify_json would need to be able to decode URL-safe base64. Should we do that unconditionnally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants