Revert "res_rtp_asterisk: Count a roll-over of the sequence number even on lost packets."

This reverts commit cb5e3445be6c55517c8d05aca601b648341f8ae9.

The original change from 16 to 15 bit sequence numbers was predicated
on the following from the now-defunct libSRTP FAQ on sourceforge.net:

> *Q6. The use of implicit synchronization via ROC seems
> dangerous. Can senders and receivers lose ROC synchronization?*
>
> **A.** It is possible to lose ROC synchronization between sender and
> receiver(s), though it is not likely in practice, and practical
> steps can be taken to avoid it. A burst loss of 2^16 packets or more
> will always break synchronization. For example, a conversational
> voice codec that sends 50 packets per second will have its ROC
> increment about every 22 minutes. A network with a burst of packet
> loss that long has problems other than ROC synchronization.
>
> There is a higher sensitivity to loss at the very outset of an SRTP
> stream. If the sender's initial sequence number is close to the
> maximum value of 2^16-1, and all packets are lost from the initial
> packet until the sequence number cycles back to zero, the sender
> will increment its ROC, but the receiver will not. The receiver
> cannot determine that the initial packets were lost and that
> sequence-number rollover has occurred. In this case, the receiver's
> ROC would be zero whereas the sender's ROC would be one, while their
> sequence numbers would be so close that the ROC-guessing algorithm
> could not detect this fact.
>
> There is a simple solution to this problem: the SRTP sender should
> randomly select an initial sequence number that is always less than
> 2^15. This ensures correct SRTP operation so long as fewer than 2^15
> initial packets are lost in succession, which is within the maximum
> tolerance of SRTP packet-index determination (see Appendix A and
> page 14, first paragraph of RFC 3711). An SRTP receiver should
> carefully implement the index-guessing algorithm. A naive
> implementation can unintentionally guess the value of
> 0xffffffffffffLL whenever the SEQ in the packet is greater than 2^15
> and the locally stored SEQ and ROC are zero. (This can happen when
> the implementation fails to treat those zero values as a special
> case.)
>
> When ROC synchronization is lost, the receiver will not be able to
> properly process the packets. If anti-replay protection is turned
> on, then the desynchronization will appear as a burst of replay
> check failures. Otherwise, if authentication is being checked, then
> it will appear as a burst of authentication failures. Otherwise, if
> encryption is being used, the desynchronization may not be detected
> by the SRTP layer, and the packets may be improperly decrypted.

However, modern libSRTP (as of 1.0.1[1]) now mentions the following in
their README.md[2]:

> The sequence number in the rtp packet is used as the low 16 bits of
> the sender's local packet index. Note that RTP will start its
> sequence number in a random place, and the SRTP layer just jumps
> forward to that number at its first invocation. An earlier version
> of this library used initial sequence numbers that are less than
> 32,768; this trick is no longer required as the
> rdbx_estimate_index(...) function has been made smarter.

So truncating our initial sequence number to 15 bit is no longer
necessary.

1. https://github.com/cisco/libsrtp/blob/0eb007f0dc611f27cbfe0bf9855ed85182496cec/CHANGES#L271-L289
2. https://github.com/cisco/libsrtp/blob/2de20dd9e9c8afbaf02fcf5d4048ce1ec9ddc0ae/README.md#implementation-notes

(cherry picked from commit f3138af5198a6a802aa49fe7122c1b293ea61649)

diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 599f56fdec5864e3523c6bce4adcea9417e6742b..028150f9503e6eef17600ca9ca0e3ebf73c36b9c 100644 (file)
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4211,7 +4211,7 @@ static int ast_rtp_new(struct ast_rtp_instance *instance,
        /* Set default parameters on the newly created RTP structure */
        rtp->ssrc = ast_random();
        ast_uuid_generate_str(rtp->cname, sizeof(rtp->cname));
-       rtp->seqno = ast_random() & 0x7fff;
+       rtp->seqno = ast_random() & 0xffff;
        rtp->expectedrxseqno = -1;
        rtp->expectedseqno = -1;
        rtp->rxstart = -1;