Implement key size mismatch

[Sculas]

srt-rs version: commit 1236c22 I have my listener setup like this:

let (_listener, mut receiver) = SrtListener::builder()
    .encryption(KeySize::AES256.as_raw(), "mypass")
    .bind(port)
    .await?;
while let Some(request) = receiver.incoming().next().await {
    let socket = match request.accept(None).await {
        Ok(socket) => socket,
        Err(err) => {
            error!("SRT connection error: {}", err);
            println!("{err:?}");
            continue;
        }
    };
    // ...
}

If I then use ffplay to connect like this:

ffplay "srt://127.0.0.1:51379/?passphrase=wrongpass"

I get this in my console output:

thread 'tokio-runtime-worker' panicked at 'not implemented: Key size mismatch', C:\Users\lucas\.cargo\git\checkouts\srt-rs-5cd6a57c40b8e1b5\1236c22\srt-protocol\src\protocol\pending_connection\hsv5.rs:72:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
SRT connection error: oneshot canceled
Custom { kind: NotConnected, error: Canceled }

If I recall correctly, the server should tell the receiver its key size. Is this a bug or has that not been implemented yet? I know it says not implemented, but since "Encryption" has been marked as working in the README I thought I could use it.

Note: Setting the key size manually in ffplay does work (default 0):

ffplay "srt://127.0.0.1:51379/?passphrase=mypass&pbkeylen=32"

But if you then use an invalid password:

ffplay "srt://127.0.0.1:51379/?passphrase=wrongpass&pbkeylen=32"

You'll get this error in your console output:

SRT connection error: oneshot canceled
Custom { kind: NotConnected, error: Canceled }

[russelltg]

Yeah, it seems like there's a few bugs here:

• We haven't implemented the "key size mismatch" situation (the unimplemented message is correct there) (this issue)
• We don't handle rejection from a server nicely (marked with a todo!()) (opened https://github.com/russelltg/srt-rs/issues/196 for this)

[pierceforte]

Hi @russelltg, I saw you have this PR https://github.com/russelltg/srt-rs/pull/197 to fix the issue.

Would it be possible to merge that?

[pierceforte]

@russelltg I also created this PR: https://github.com/russelltg/srt-rs/pull/216. Can you please take a look when you have a chance?

I think this would be worth having even if the key size mismatch is implemented.

[robertream]

Although we certainly don't want the client to behave in an unexpected or unintuitive insecure way by default, it doesn't appear that the spec is actually very prescriptive about the encryption key size selection algorithm. It might be a good idea to chose a single client API design that accommodates customization or parameterization for the key selection logic similar to what we now have for the multiplexing server listener, thanks to @pierceforte's recent contribution.

Caller-Listener

https://haivision.github.io/srt-rfc/draft-sharabayko-srt.html#name-caller-listener-handshake https://haivision.github.io/srt-rfc/draft-sharabayko-srt.html#section-4.3.1.2.1

If the Encryption Flag field is set to 0 (not advertised), the caller MAY advertise its own cipher and key length. If the induction response already advertises a certain value in the Encryption Flag, the caller MAY accept it or force its own value. It is RECOMMENDED that if a caller will be sending the content, then it SHOULD force its own value. If it expects to receive content from the SRT listener, then is it RECOMMENDED that it accepts the value advertised in the Encryption Flag field.

An alternative behavior MAY be for a caller to take the longer key length in such cases.

The spec also says:

https://haivision.github.io/srt-rfc/draft-sharabayko-srt.html#name-the-conclusion-response

The only case when the Listener can have precedence over the Caller is the advertised Cipher Family and Block Size in the Encryption Field of the Handshake.

Rendezvous

https://haivision.github.io/srt-rfc/draft-sharabayko-srt.html#name-rendezvous-handshake

If Bob is the Initiator and encryption is on, he will use either his own cipher family and block size or the one received from Alice (if she has advertised those values).

[robertream]

@russelltg I also created this PR: #216. Can you please take a look when you have a chance?

I think this would be worth having even if the key size mismatch is implemented.

@pierceforte I published v0.4.4 to crates.io

[pierceforte]

Thank you @robertream!

[robertream]

@russelltg I took a deeper look at how the reference implementation handles differences between encryption settings during connection negotiation, and I found this section of the API Socket Options documentation informative:

https://github.com/Haivision/srt/blob/master/docs/API/API-socket-options.md#srt_km_state

Note that with the default value of SRTO_ENFORCEDENCRYPTION option (true), the state is equal on both sides in both directions, and it can be only SRT_KM_S_UNSECURED or SRT_KM_S_SECURED (in other cases the connection is rejected).

I think the solution that would be most consistent with the reference implementation, as well as true to the latest spec, would be as follows:

1) Enforcing symmetrical encryption should be the default, so effectively the SRTO_ENFORCEDENCRYPTION = true behavior 2) Implement SRTO_ENFORCEDENCRYPTION as an enum KeyNegotiation with EnforceSymmetric and AllowAsymmetric variants exposed via a "key_negotiation" property on the Encryption option struct, allowing non-symmetrical encryption settings, and behavior consistent with the documentation for SRT_KM_STATE 3) Implement the 1.3.0+ SRTO_SENDER behavior as another variant on the KeyNegotiation enum called PrioritizeAsSender 4) [optional] Expose the negotiated Key Material State for an SrtSocket's Sender and Receiver, probably via the Statistics struct