erdtls* comments

[ford-prefect]

These are some broad thoughts I had on the erdtls plugins and their usage. Also including some comments by @Rugvip (who can correct me if I got these wrong).

1. Clients should use ErDtlsAgent as the object to be set on erdtls* elements In my opinion, this is better than a global hash table (and @Rugvip disagrees, as he finds the current mechanism to be easier to use by clients).
2. ErDtlsAgent should not expose API to provide its context, but instead should offer API to instantiate an ErDtlsConnection (this makes the tight coupling between the two clear, without having internal API exposing implementation details (i.e. the SSL context). In the long term, ErDtlsAgent and ErDtlsConnections can become abstract classes from which specific implementations can be derived (we have openssl now, we could add gnutls).
3. erdtlsenc and erdtlsdec should be merged into a single erdtlsconnection element. The two elements cannot be used independently of the other, and thus there should be a single element to manage both directions of the connection. The element would have always pads for the DTLS send and receive side, and request pads for applications to access send/receive plaintext data (request pads since they may not always explicitly be interested in both directions).
4. erdtlssrtpenc and erdtlssertpdec should be merged into a singl erdtlssrtpbin. Once the erdtlsenc/dec pair are merged, the SRTP bins on top of them will also need to be merged since the two can't exist independently, and share a single DTLS connection. Some complexity is added by the fact that RTCP may or may not be muxed, and this must be handled appropriately. I've illustrated what the bins might look like at: http://arunraghavan.net/downloads/misc/dtls-refactor.txt
5. Evaluate using the srtp-encode/decoder interface on rtpbin with the erdtlssrtpbin -- this might make it easier to handle DTLS/SRTP on the application side, but does need some thought for all the various special cases (optional presence of DTLS, RTCP muxing, etc.). Moreover, making the DTLS data available, as well as making sure both send and receive channels are created even in a unidirectional stream might pose some additional challenges.

[ocrete]

Here are some comments based on my previous attempt at building a DTLS plugin for GStreamer:

• Merging the send and receive pipelines is problematic because when you use it with rtpbin, then you get a loop, which is a problem for state changes.
  • I also don't think we can put the dtls thing cleanly inside rtpbin, I think it has to be outside of it. The request-rtp/rtcp-encoder/decoder signals assume that the encoder/decoder is only SRTP (so unidirectional). With DTLS-SRTP, you always have a p2p connection, so it's a bit different. Also, you could have non-SRTP muxed into it, in particular you could have SCTP muxed. You can also have the case of bundle, where you have two RTP session in the same DTLS session, I think it makes sense to demux them before feeding them to rtpbin.
• Also, I agree with the current code, that hiding the connection object is the way to go, and using a connection id string, otherwise you can't use it from gst-launch, which would be unfortunate. I would also
• Then there is the issue of how OpenSSL is initialised. What happens if something else in the application tries to use OpenSSL also.. Also, I'm not sure where the OpenSSL threading is initialised in there, see https://www.openssl.org/docs/crypto/threads.html ? For GStreamer, we know GnuTLS has the right threading behavior, it may also make sense to investigate NSS if the GnuTLS performance is a problem.

[ford-prefect]

For gst-launch, we could allow initialisation with a null agent to create a default agent (and with a single connection, you would not have to worry about sharing the object between the two elements).

Could you expand a bit about the problem you see with getting a loop on state changes if we merge the two into a single element?

[Rugvip]

@ford-prefect

1. Yep, I think it's easier to set the "pem" property of the decoder than to instantiate an agent, set the "pem" property of the agent, and then set the agent of the decoder. And the former can be done without having to include any headers.
2. I don't see any way around this, unless you put the two implementations in the same file. Again, the function which fetches the SSLContext is private and prefixed with "_". It is an implementation specific requirement and not a part of a would be ErDtlsAgent API. If the ErDtlsAgent and ErDtlsConnection were converted to abstract classes, the function to get the SSLContext would be renamed to whatever fits the new implementation, e.g. _er_dtls_openssl_agent_peek_context.
3. This may or may not be better, I really have no idea. I think there are pros and cons with both solutions. Iirc the main reasons for choosing to use two elements are that it makes the pipeline graph easier to reason, the pipeline becomes less of a mess in general, and you can be more flexible with the state transitions of the individual elements.
   I (think) there's a third possible design here, which is to give the encoder and decoder an extra pad each and link the two together. I have no idea how GStreamer would behave with that solution and if it's implementable. It might also get ugly if the DTLS elements are inside bins.
4. This is really a part of 3.
5. No comment here, except that it can quickly get pretty messy with RTCP muxing etc..

[ocrete]

For gst-launch, what if you have separate rtp and rtcp? We'd still need two contexts. That said, we probably also want a way to share some of the context to be able to do SSL session resumption to make the second connection much faster.

The state changes are sink to source. What if you have:
mediasrc ! rtpbin.sendsink ! rtpbin.sendsrc ! dtlsconnection.sendsink dtlsconneciton.sendsrc ! udpsink udpsrc ! dtlsconnection.recvsink dtlsconnection.recvsrc ! rtpbin.recvsink rtpbin.recvsrc ! mediasink in which order is the state transiton happen, rtpbin or dtlsconnection first ?

For the same reason we use a separate udpsrc and udpsink element even when they share the same socket. Although that indeed means that you can't do a real useful DTLS connection with gst-launch anyway (because you can't share the socket between udpsrc and udpsink).

[Rugvip]

@ocrete

Regarding your second point, I think you're discussion two different things. There are two things that are shared between the different DTLS elements:

• ErDtlsConnection The connection is used to store the state and pass data between an encoder and a decoder. The connection object is set on both an encoder and a decoder through the "connection-id" property, and there can only be one encoder and one decoder with a specific connection-id at the same time.
• ErDtlsAgent The agent contains the prototype encryption context which is copied when creating new connections, which among other things is configured with a certificate. The agent can be shared between any number of DTLS elements, the more the merrier. It is shared through the "pem" property, which is also the certificate that is set only on the decoder element. If the "pem" property is not set, a default agent is used, which has a generated self-signed certificate.

[Rugvip]

@ocrete

Regarding OpenSSL initialisation: this seems like something that would be a compile-time flag. From what I could find it would also be possible to add a lock around the initialisation and then hope the application uses locks as well ^_^

Regarding threads, the DTLS elements have full control of the threading. OpenSSL is used in a single-threaded way, the only thread that is used (timeout thread) is created by the ErDtlsConnection.

[ocrete]

What if some other part of the application uses OpenSSL ? OpenSSL is not used to encode and decode the buffers in the streaming thread? What if the other side sends DTLS (not SRTP?)

[Rugvip]

What if some other part of the application uses OpenSSL ?

I don't see the problem with other parts of the application using OpenSSL, except for the initialisation? The ErDtlsConnection creates a new SSL context, which isn't shared with the rest of the application.

OpenSSL is not used to encode and decode the buffers in the streaming thread?

It is

What if the other side sends DTLS (not SRTP?)

Not sure I follow... the ErDtlsSrtpDec demuxes RTP/DTLS if that's what you're asking.

[Rugvip]

Btw, I don't have anything against swapping out OpenSSL or providing other alternatives. I just don't think it should be done in a way that makes it more difficult to use the plugin, or because the code is greener in the other library.

[ocrete]

All SSL libraries are horrible, I just want to make sure we expose OpenSSL's limitations correctly (because not exposing it and then having it crash randomly will be a pain!)

[superdump]

😄 that's a nice way of looking at the problem.

[sdroege]

I think you mean SSL/TLS is horrible 😄 If it was only the libraries...

[sdroege]

Should this be closed now or is there anything left to be done?