GNOME Bugzilla – Bug 726861
srtp: add support for rollover counter
Last modified: 2016-04-15 07:50:50 UTC
The SRTP encoder and decoder currently only support one-to-one communication. If a third party joins an ongoing streaming session it is very likely that it will be unable to unprotect the packets. SRTP has the so called rollover counter which increases every time the RTP sequence number reaches 0xFFFF. The rollover counter is initially 0 so both the server and the first client will have it synchronized. However, if the stream is shared between multiple clients, it is very likely that after a few minutes the server has a rollover counter greater than 0. A second client would not know what the rollover counter is and it will fail verifying and decrypting the incoming packets.
Created attachment 272616 [details] [review] add support for rollover counter From the commit log: We add a new signal, get-rollover-counter, to the SRTP encoder. Given a ssrc the signal will return the currently internal SRTP rollover counter for the given stream. In the SRTP decoder we also add a new signal, request-rollover-counter, that will request a new rollover counter when a new SRTP stream is to be created for a given ssrc. Also, we add a new property window-size to the decoder to be able to increase the replay protection window size.
I am aware of the EKT support in the libsrtp library. But this was much simpler to implement and use for my current needs. EKT support could be added in the future if someone else needs it.
Ping.
Now that we have MIKEY support (thanks Wim!) and that MIKEY transmits the roll over counter, this seems even more useful (and actually is necessary). I haven't tried with MIKEY yet as I was using simply an SDP key exchange. I'll do as soon as I have some time.
Yes, we need to ROC for the mikey message. I'm still pondering about the interface there is on the srtp elements.
Any reason to have a separate SIGNAL_REQUEST_ROLLOVER_COUNTER signal instead of adding just another field in the REQUEST_KEY signal ? I also wonder if the window size shouldn't be settable per ssrc in the caps also ?
(In reply to comment #6) > Any reason to have a separate SIGNAL_REQUEST_ROLLOVER_COUNTER signal instead of > adding just another field in the REQUEST_KEY signal ? Not that I recall. I think I just wanted to keep things separately or probably was in a hurry. It should work in REQUEST_KEY as well. > I also wonder if the window size shouldn't be settable per ssrc in the caps > also ? I guess it wouldn't hurt. If I understand correctly, this can be different for each client right?
Created attachment 276946 [details] [review] add support for rollover counters fixup In this patch, the decoder uses the request-key signal to obtain the ROC from the caps. For the decoder, we still sue the get-rollover-counter action signal. I have also removed the window-size property. We can add it in another patch.
Review of attachment 276946 [details] [review]: ::: ext/srtp/gstsrtpdec.c @@ +1062,3 @@ + /* We finally add the RTP sequence number to the current + * rollover counter. */ + stream->rtp_rdbx.index |= seqnum; I wonder if this shouldn't be something like stream->rtp_rdbx.index &= ~0xFFFF; stream->rtp_rdbx.index |= seqnum; Or are we always certain that this happens immediately after the roc has been reset ? Just using the bitwise or on it's own worries me a bit.
(In reply to comment #9) > Review of attachment 276946 [details] [review]: > > ::: ext/srtp/gstsrtpdec.c > @@ +1062,3 @@ > + /* We finally add the RTP sequence number to the current > + * rollover counter. */ > + stream->rtp_rdbx.index |= seqnum; > > I wonder if this shouldn't be something like > stream->rtp_rdbx.index &= ~0xFFFF; > stream->rtp_rdbx.index |= seqnum; > > Or are we always certain that this happens immediately after the roc has been > reset ? Just using the bitwise or on it's own worries me a bit. Good point. I think it's safer to do what you suggest.
Created attachment 277828 [details] [review] add support for rollover counter 2nd fixup New patch with suggestion from comment 9.
Pushed: commit da30669589cfc8189f60aedfae98a217a9fae994 Author: Aleix Conchillo Flaqué <aleix@oblong.com> Date: Fri Mar 21 22:16:41 2014 -0700 srtp: add support for rollover counters and replay protection window size We add a new signal, get-rollover-counter, to the SRTP encoder. Given a ssrc the signal will return the currently internal SRTP rollover counter for the given stream. For the SRTP decoder we have a new SRTP caps parameter "roc" that needs to be set when a new SRTP stream is created for a given SSRC. https://bugzilla.gnome.org/show_bug.cgi?id=726861