X.509: clean up code

[andybalaam]

Part of X.509

Working within the X.509 prototype branch at https://github.com/matrix-org/matrix-rust-sdk/tree/andyrich/x509 , search for TODOs etc and review the code for shortcuts that are not OK for production code. Fix them or create new issues if fixes will be large.

Some ideas for cleanups:

• Rename X509Signer -> X509Signing, X509Verifier -> X509Verifying, or better names if we can think of them. Other candidates: X509ObjectSigner, X509KeySigner, X509JsonSigner.
• Rename X509Sign -> X509Signer RawX509Signer, X509Verify -> X509Verifier RawX509Verifier.
• Remove X509Data. It's only used in OlmMachine.with_store. Either pass the two halves separately, or give OlmMachine a builder pattern (and pass the two halves separately into that).
• Make X509::sign and X509Verify::verify take &[u8] arguments if possible, instead of Vec
• Consider whether we can remove X509SignImpl in bindings/matrix-sdk-ffi/src/client_builder.rs
• Consider whether we should split structs e.g. in crates/matrix-sdk-crypto/src/types/mod.rs into separate files
• Clean up serialization/deserialization of X509 signatures
  • Currently we serialize X.509 signatures to a JSON object. We believe that will break existing client implementations, so we need to turn it into a single string instead.
• ... possibly more here ...

[richvdh]

• Make X509::sign and X509Verify::verify take &[u8] arguments if possible, instead of Vec

I think this is referring to the traits in matrix-sdk-ffi, which are implemented by kotlin-side classes.

It looks like this isn't possible: attempting to do so produces an error in the depths of the uniffi::export macro:

error[E0053]: method `sign` has an incompatible type for trait
   --> bindings/matrix-sdk-ffi/src/client.rs:316:1
    |
316 | #[uniffi::export(callback_interface)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&[u8]`, found `Vec<u8>`

Basically, you're not allowed to pass references over the FFI boundary: it has to be an owned thing.

[poljar]

Basically, you're not allowed to pass references over the FFI boundary: it has to be an owned thing.

That's not completely correct. I'm not sure if that applies to callback interfaces but &[T] is supported by uniffi: https://mozilla.github.io/uniffi-rs/latest/types/builtin_types.html

[richvdh]

That's not completely correct. I'm not sure if that applies to callback interfaces but &[T] is supported by uniffi: mozilla.github.io/uniffi-rs/latest/types/builtin_types.html

Sorry yes, I was talking specifically about Rust -> Foreign calls.

[richvdh]

Ah, mozilla/uniffi-rs#2263 appears to be the relevant tracking issue.

[richvdh]

• Consider whether we can remove X509SignImpl in bindings/matrix-sdk-ffi/src/client_builder.rs

I don't think this works either, mostly because of the previous problem: we need the shim to convert from &[u8] to Vec<u8>. There are other types that need some mapping too.

[richvdh]

Consider whether we should split structs e.g. in crates/matrix-sdk-crypto/src/types/mod.rs into separate files

I'm calling this done, via #6660 and subsequent move of X509Signature to its own file.

[richvdh]

We now use CMS signatures, so I'm calling this done.