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

Verification event improvements #129

Closed
21 changes: 19 additions & 2 deletions ruma-events/src/key/verification.rs
Expand Up @@ -25,11 +25,13 @@ pub enum HashAlgorithm {
/// A key agreement protocol.
#[derive(Clone, Copy, Debug, PartialEq, Display, EnumString, Serialize, Deserialize)]
#[non_exhaustive]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
#[serde(rename_all = "kebab-case")]
#[strum(serialize_all = "kebab-case")]
pub enum KeyAgreementProtocol {
/// The [Curve25519](https://cr.yp.to/ecdh.html) key agreement protocol.
Curve25519,
/// The Curve25519 key agreement protocol with check for public keys.
Curve25519HkdfSha256,
jplatte marked this conversation as resolved.
Show resolved Hide resolved
}

/// A message authentication code algorithm.
Expand Down Expand Up @@ -64,3 +66,18 @@ pub enum VerificationMethod {
#[strum(serialize = "m.sas.v1")]
MSasV1,
}

#[cfg(test)]
mod test {
use super::KeyAgreementProtocol;

#[test]
fn serialize_key_agreement() {
let serialized =
serde_json::to_string(&KeyAgreementProtocol::Curve25519HkdfSha256).unwrap();
assert_eq!(serialized, "\"curve25519-hkdf-sha256\"");

let deserialized: KeyAgreementProtocol = serde_json::from_str(&serialized).unwrap();
assert_eq!(deserialized, KeyAgreementProtocol::Curve25519HkdfSha256);
}
}
40 changes: 29 additions & 11 deletions ruma-events/src/key/verification/start.rs
Expand Up @@ -27,36 +27,37 @@ pub enum StartEventContent {

/// The payload of an *m.key.verification.start* event using the *m.sas.v1* method.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[non_exhaustive]
pub struct MSasV1Content {
jplatte marked this conversation as resolved.
Show resolved Hide resolved
/// The device ID which is initiating the process.
pub(crate) from_device: Box<DeviceId>,
pub from_device: Box<DeviceId>,

/// An opaque identifier for the verification process.
///
/// Must be unique with respect to the devices involved. Must be the same as the
/// `transaction_id` given in the *m.key.verification.request* if this process is originating
/// from a request.
pub(crate) transaction_id: String,
pub transaction_id: String,

/// The key agreement protocols the sending device understands.
///
/// Must include at least `curve25519`.
pub(crate) key_agreement_protocols: Vec<KeyAgreementProtocol>,
/// Must include at least `Curve25519` or `Curve25519HkdfSha256`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're stricly speaking not compatible with either version of the spec like this, I really like this solution.

pub key_agreement_protocols: Vec<KeyAgreementProtocol>,

/// The hash methods the sending device understands.
///
/// Must include at least `sha256`.
pub(crate) hashes: Vec<HashAlgorithm>,
pub hashes: Vec<HashAlgorithm>,

/// The message authentication codes that the sending device understands.
///
/// Must include at least `hkdf-hmac-sha256`.
pub(crate) message_authentication_codes: Vec<MessageAuthenticationCode>,
pub message_authentication_codes: Vec<MessageAuthenticationCode>,

/// The SAS methods the sending device (and the sending device's user) understands.
///
/// Must include at least `decimal`. Optionally can include `emoji`.
pub(crate) short_authentication_string: Vec<ShortAuthenticationString>,
pub short_authentication_string: Vec<ShortAuthenticationString>,
}

/// Options for creating an `MSasV1Content` with `MSasV1Content::new`.
Expand Down Expand Up @@ -106,8 +107,17 @@ impl MSasV1Content {
/// `MessageAuthenticationCode::HkdfHmacSha256`.
/// * `short_authentication_string` does not include `ShortAuthenticationString::Decimal`.
pub fn new(options: MSasV1ContentOptions) -> Result<Self, InvalidInput> {
if !options.key_agreement_protocols.contains(&KeyAgreementProtocol::Curve25519) {
return Err(InvalidInput("`key_agreement_protocols` must contain at least `KeyAgreementProtocol::Curve25519`".into()));
if !options.key_agreement_protocols.contains(&KeyAgreementProtocol::Curve25519)
&& !options
.key_agreement_protocols
.contains(&KeyAgreementProtocol::Curve25519HkdfSha256)
{
return Err(InvalidInput(
"`key_agreement_protocols` must contain at \
least `KeyAgreementProtocol::Curve25519` or \
`KeyAgreementProtocol::Curve25519HkdfSha256`"
.into(),
));
}

if !options.hashes.contains(&HashAlgorithm::Sha256) {
Expand All @@ -120,11 +130,19 @@ impl MSasV1Content {
.message_authentication_codes
.contains(&MessageAuthenticationCode::HkdfHmacSha256)
{
return Err(InvalidInput("`message_authentication_codes` must contain at least `MessageAuthenticationCode::HkdfHmacSha256`".into()));
return Err(InvalidInput(
"`message_authentication_codes` must contain \
at least `MessageAuthenticationCode::HkdfHmacSha256`"
.into(),
));
}

if !options.short_authentication_string.contains(&ShortAuthenticationString::Decimal) {
return Err(InvalidInput("`short_authentication_string` must contain at least `ShortAuthenticationString::Decimal`".into()));
return Err(InvalidInput(
"`short_authentication_string` must contain \
at least `ShortAuthenticationString::Decimal`"
.into(),
));
}

Ok(Self {
Expand Down