GNOME Bugzilla – Bug 697909
DTLS-SRTP support for glib
Last modified: 2017-10-17 20:00:52 UTC
Add methods to specify desired SRTP profiles and to find the negotiated ones after the handshake has completed. Also add a "status" property to figure out when implicit TLS handshakes and re-handshakes have happened.
Created attachment 241395 [details] [review] gtlsconnection: Add functions to support DTLS-SRTP negotiation https://bugzilla.gnome.org/show_bug.cgi?id=697907
Created attachment 241396 [details] [review] tlsconnection: Add status property https://bugzilla.gnome.org/show_bug.cgi?id=697907
Created attachment 241691 [details] [review] tlsconnection-gnutls: Implement SRTP related functions
It seems weird to be putting application-protocol-specific methods into GTlsConnection... Maybe we could add a new interface? "GTlsDtlsSrtpChannel"? And then the methods would be on that, making them more clearly isolated/encapsulated.
Comment on attachment 241396 [details] [review] tlsconnection: Add status property This patch makes sense as-is. >+ * @G_TLS_STATUS_REHANSHAKING: The TLS connection is performing a new handshake >+ * to #G_TLS_STATUS_REHANSHAKING if g_tls_connection_handshake() or typos
(In reply to comment #4) > Maybe we could add a new interface? "GTlsDtlsSrtpChannel"? And then the methods > would be on that, making them more clearly isolated/encapsulated. except then we'd need to create implementations of both GTlsClientConnection and GTlsServerConnection that implemented that... I guess basically DTLS-SRTP is a bit of a gigantic hack... Also, we have (or will have) methods for other specific TLS extensions, so...
Comment on attachment 241395 [details] [review] gtlsconnection: Add functions to support DTLS-SRTP negotiation >+typedef enum { >+ G_TLS_SRTP_PROFILE_NONE = 0, >+ G_TLS_SRTP_PROFILE_AES128_CM_HMAC_SHA1_80 = 1, >+ G_TLS_SRTP_PROFILE_AES128_CM_HMAC_SHA1_32 = 2, >+ G_TLS_SRTP_PROFILE_NULL_HMAC_SHA1_80 = 5, >+ G_TLS_SRTP_PROFILE_NULL_HMAC_SHA1_32 = 6, >+} GTlsSrtpProfile; Do you think it's likely this will be expanded in the future? So far I've managed to avoid needing to pull various TLS cipher/encoding/etc lists into glib, but one idea I'd had if we did eventually need to do it would be to treat them as strings. So you'd have something like g_tls_connection_add_srtp_profile (conn, "SRTP_AES128_CM_HMAC_SHA1_80"); which is less compile-time-safe, but more future-proof and less API-polluting. (I'm not saying we should definitely do it this way, just suggesting it as an alternate possibility. What do you think?) >+ * @server_key: (allow-none): The location where to store the server encryption master SRTP key or %NULL "The location to store" or "The location in which to store". Also, should be annotated "(out)". (Likewise the other arguments.) Also, now that we have GBytes, that would be more appropriate here than GByteArray.
Created attachment 245690 [details] [review] gtlsconnection: Add functions to support DTLS-SRTP negotiation Now with more GBytes and better English As for the string vs enum, the enum is likely to be expanded in the future, if you prefer, it could definitely be strings. I guess compile-time safeness is not that critical as different backends may not support all profiles anyway.
(In reply to comment #7) > Do you think it's likely this will be expanded in the future? > > So far I've managed to avoid needing to pull various TLS cipher/encoding/etc > lists into glib, but one idea I'd had if we did eventually need to do it would > be to treat them as strings. So you'd have something like > > g_tls_connection_add_srtp_profile (conn, "SRTP_AES128_CM_HMAC_SHA1_80"); > > which is less compile-time-safe, but more future-proof and less API-polluting. I think this makes sense, or maybe even make it more generic somehow and remove all notions of SRTP from GIO.
Marking bug #697908 as a dependency, rather than a reverse-dependency, because the DTLS stuff needs to land before we can do DTLS-SRTP. I am working on finish off the DTLS stuff, but I have no plans to touch DTLS-SRTP, so these patches will need rebasing by whoever picks this up.
Comment on attachment 241691 [details] [review] tlsconnection-gnutls: Implement SRTP related functions I don't know about SRTP, but bug #697908 is definitely a dependency, so this isn't ready for the request queue.
Comment on attachment 245690 [details] [review] gtlsconnection: Add functions to support DTLS-SRTP negotiation Ditto.
I'm not sure with DTLS-SRTP in glib, we ended up having plugins that use the TLS library directly in GStreamer, and it's a lot more simple as the GIO async model really doesn't match the GStreamer model.
OK, in that case, let's close this.