atagen

malice

public
Star0
Watch0
00ca0561eced72aa8fda32228099a5df8c82527d
2
ssh://brewery@pls.lobotomise.me:33/atagen/malice.git
MACLTOK_PROTOCOL_LEAKS.mdMarkdown103 lines4.7 KB

macltok request, keep Matrix protocol knowledge out of hosts

Problem

malice should know how to represent Matrix things (show a homeserver field, print ids as strings) and nothing more. Today it carries protocol knowledge macltok should own, and some of it is written twice, once in malice-server and once in malice-worker. Every other macltok host would have to write the same code. Five requests, ordered by impact.

1. Avatars as MediaId, not MXC URIs

RoomSummary::avatar and the other avatar fields in model.rs are Option<OwnedMxcUri>, so a host fetches them with ClientHandle::mxc_media(&MxcUri, MediaSize). Attachments go through the opaque MediaId and ClientHandle::media. Because of that split, malice validates raw mxc:// strings with ruma::MxcUri::is_valid and sends MXC URIs over its HTTP API to the browser and back.

Request, register avatars like attachments and expose them as Option<MediaId>, or as MediaDescriptor if the mimetype or size is known. Then hosts fetch every image through media(&MediaId, MediaSize). mxc_media can stay for hosts that want it, but no model field should require it.

set_avatar(Option<OwnedMxcUri>) is fine as it is if its input comes from an upload outcome and not from the host.

2. Soft-logout re-login decision

malice runs the same flow in malice-server/src/state.rs (login) and malice-worker/src/lib.rs (Host::login). It restores the store, and on Restore::SoftLoggedOut(soft) checks whether the submitted username is that user. If so it calls soft.password_login(password) to keep the device and its keys. Otherwise it does a fresh PasswordLogin::new(..). The username check is copied in both crates.

fn username_matches(submitted: &str, user_id: &ruma::OwnedUserId) -> bool {
    let submitted = submitted.trim();
    submitted == user_id.as_str() || submitted == user_id.localpart()
}

That check is Matrix knowledge (a localpart or a full MXID names the same account), and it is only correct when the homeserver also matches.

Request, either shape works.

impl SoftLogout {
    /// whether a login form's homeserver and username name this soft-logged-out account
    pub fn matches(&self, homeserver: &str, username: &str) -> bool;
}

Or have Session::login(PasswordLogin, sessions) resume a soft-logged-out record in sessions by itself when it names the same account, so hosts never branch on it. The second shape is preferred because both malice call sites would shrink to one call.

3. Homeserver input validation, and a discovery bug it causes

macltok builds the client with server_name_or_homeserver_url(login.homeserver.trim()), which does .well-known discovery for a bare server name. malice runs normalise_homeserver (malice-core/src/login.rs) first, and that prefixes https:// onto anything without a scheme. So a user typing example.org, whose homeserver is matrix.example.org, gets https://example.org treated as a homeserver URL and discovery never runs. malice will drop its normaliser once macltok covers what it checks.

  • reject empty input and input containing whitespace
  • reject schemes other than http and https
  • reject a userinfo part (user@host) and a missing host
  • reject plain http unless the host opts in, and even then only for loopback hosts (localhost, 127.0.0.1, ::1)

Suggested shape is a builder option plus a typed error.

PasswordLogin::new(homeserver, username, password)
    .allow_insecure_loopback(true)

with a distinct AccountError variant (or ErrorKind) for invalid homeserver input, so a host can answer 400 with the message instead of a generic login failure. The same validation should apply to the OAuth login path.

4. AccountError::Operation hides the homeserver’s answer

Operation displays as "{context} failed" and the useful part (the homeserver’s errcode and message) is only in source(). malice has a describe_error helper that walks the source chain to build a readable message. Every host would need it.

Request, include the innermost meaningful cause in Display, or add an accessor such as AccountError::user_message() -> String that renders the chain.

5. Re-export ruma

macltok’s public API is typed with ruma (OwnedRoomId, OwnedEventId, OwnedUserId, OwnedRoomOrAliasId, OwnedMxcUri), so hosts must depend on ruma at exactly macltok’s version. malice pins ruma = "0.16" in two places to match.

Request, pub use ruma; in lib.rs, same as imbl. malice will then parse ids through macltok::ruma and drop its own ruma dependency.

Not needed

VerificationAction already derives Deserialize under the serde feature, so malice will drop its hand-written string match on its own.