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.

Sign up for GitHub

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

Jump to bottom

Implement key size mismatch #195

Open
Sculas opened this issue Mar 22, 2023 · 7 comments · May be fixed by #197
Open

Implement key size mismatch #195

Sculas opened this issue Mar 22, 2023 · 7 comments · May be fixed by #197

Comments

@Sculas
Copy link

Sculas commented Mar 22, 2023 •
edited
Loading

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
Copy link
Owner

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 Handle server rejection properly #196 for this)

@russelltg russelltg changed the title "Key size mismatch" & "oneshot canceled" errors when using encryption Implement key size mismatch Mar 23, 2023
@russelltg russelltg linked a pull request Mar 23, 2023 that will close this issue
Open
@pierceforte
Copy link
Contributor

Hi @russelltg, I saw you have this PR #197 to fix the issue.

Would it be possible to merge that?

@pierceforte pierceforte mentioned this issue May 22, 2024
Merged
@pierceforte
Copy link
Contributor

@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.

@robertream
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor

Thank you @robertream!

@robertream
Copy link
Collaborator

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

implement keysize mismatch russelltg/srt-rs
4 participants
@robertream @russelltg @Sculas @pierceforte