After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 697909 - DTLS-SRTP support for glib
DTLS-SRTP support for glib
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: network
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on: 697907 697908
Blocks:
 
 
Reported: 2013-04-12 20:05 UTC by Olivier Crête
Modified: 2017-10-17 20:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtlsconnection: Add functions to support DTLS-SRTP negotiation (5.69 KB, patch)
2013-04-12 20:05 UTC, Olivier Crête
reviewed Details | Review
tlsconnection: Add status property (3.25 KB, patch)
2013-04-12 20:05 UTC, Olivier Crête
reviewed Details | Review
tlsconnection-gnutls: Implement SRTP related functions (5.66 KB, patch)
2013-04-16 21:33 UTC, Olivier Crête
reviewed Details | Review
gtlsconnection: Add functions to support DTLS-SRTP negotiation (5.64 KB, patch)
2013-05-31 00:18 UTC, Olivier Crête
reviewed Details | Review

Description Olivier Crête 2013-04-12 20:05:17 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.
Comment 1 Olivier Crête 2013-04-12 20:05:30 UTC
Created attachment 241395 [details] [review]
gtlsconnection: Add functions to support DTLS-SRTP negotiation

https://bugzilla.gnome.org/show_bug.cgi?id=697907
Comment 2 Olivier Crête 2013-04-12 20:05:33 UTC
Created attachment 241396 [details] [review]
tlsconnection: Add status property

https://bugzilla.gnome.org/show_bug.cgi?id=697907
Comment 3 Olivier Crête 2013-04-16 21:33:15 UTC
Created attachment 241691 [details] [review]
tlsconnection-gnutls: Implement SRTP related functions
Comment 4 Dan Winship 2013-04-17 12:52:00 UTC
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 5 Dan Winship 2013-04-17 12:53:46 UTC
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
Comment 6 Dan Winship 2013-05-30 12:40:56 UTC
(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 7 Dan Winship 2013-05-30 12:51:43 UTC
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.
Comment 8 Olivier Crête 2013-05-31 00:18:40 UTC
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.
Comment 9 Sebastian Dröge (slomo) 2014-03-08 13:31:58 UTC
(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.
Comment 10 Philip Withnall 2015-07-03 12:00:33 UTC
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 11 Michael Catanzaro 2017-10-17 15:11:36 UTC
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 12 Michael Catanzaro 2017-10-17 15:11:44 UTC
Comment on attachment 245690 [details] [review]
gtlsconnection: Add functions to support DTLS-SRTP negotiation

Ditto.
Comment 13 Olivier Crête 2017-10-17 17:41:17 UTC
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.
Comment 14 Michael Catanzaro 2017-10-17 20:00:52 UTC
OK, in that case, let's close this.