Dynamic SRTCP header length depending on RTCP type

[simoneguidialea]

This PR replaces constants octets_in_rtcp_header (set to 8) and uint32s_in_rtcp_header (set to 2) with variables whose values are calculated depending on packets' RTCP types.

In particular [RFC 3550, par. 6.7] specifies a 12 octets (3 uint32s) header for the RTCP APP type.

MC (Mission Critical) specification [TS 24.380, par. 8.1.2] also requires the "Name" part (9-12 bytes) of the RTCP APP header to be unencrypted. This cannot be achieved with current libsrtp implementation due to the use of fixed constants, so this modification allows it.

[pabuhler]

libSRTP is based on RFC 3711, which assumes that all RTCP packets follow the rules defined in RFC 3550 in that they are compound packets where the first packet is at least an empty receiver report. This ensures that there is an SSRC in 2nd 32 bit word that can be used as a key to determine which encryption context should be used. In practice the empty RR can be dropped as long as the first packet in the compound RTCP contains the senders SSRC in the 2nd 32bit word.

The octets_in_rtcp_header is set to 8 as this is the only size that is needed in the context of SRTP.

TS 124.380, par. 8.1.2 explicitly states "This part is encrypted if SRTCP is used." with regard to the APP name.

So I am not sure why you think it is needed to treat the different RTCP types differently with regard to SRTP. Also such a change that you suggest would be breaking compatibility with older version and would need at least a flag to indicated it was in use. At the moment I would think we would reject this change unless you can come with a strong argument convincing us other wise.

[simoneguidialea]

Thank you for your response, it helped us clarifying that Mission Critical specification differs from RFC 3711 with regard to SRTCP.

libSRTP is based on RFC 3711, which assumes that all RTCP packets follow the rules defined in RFC 3550 in that they are compound packets where the first packet is at least an empty receiver report. This ensures that there is an SSRC in 2nd 32 bit word that can be used as a key to determine which encryption context should be used. In practice the empty RR can be dropped as long as the first packet in the compound RTCP contains the senders SSRC in the 2nd 32bit word.

The octets_in_rtcp_header is set to 8 as this is the only size that is needed in the context of SRTP.

I read [RFC 3711, par. 3.4] before submitting the PR, but it was not crystal clear to me whether all RTCP packets had to be compound packets or not. The MC specification added to the confusion (more on that later).

Upon further reading of RFCs 3550 and 3711 I understood that you were right, within RFC standards all RTCP packets have to be compound packets, eliminating the need to treat RTCP types differently in SRTCP.

TS 124.380, par. 8.1.2 explicitly states "This part is encrypted if SRTCP is used." with regard to the APP name.

It states "This part is encrypted if SRTCP is used." with regard to the "Application-dependent data" part, not to the "Name" part.

It seems like MC specification does not conform to SRTCP RFC specification, in particular about RTCP compound packets. In fact [TS 24.380, par. 8.1.1] states:

"The media plane control protocols specified in the present document are based on the RTCP Application Packets (RTCP: APP), as defined in IETF RFC 3550 [3], but the media plane control messages do not conform to the rules for compound RTCP packets or RTCP packet transmission.

Each media plane control message is one RTCP: APP packet. These RTCP: APP packets are not to be sent in compound RTCP packets"

So I am not sure why you think it is needed to treat the different RTCP types differently with regard to SRTP. Also such a change that you suggest would be breaking compatibility with older version and would need at least a flag to indicated it was in use. At the moment I would think we would reject this change unless you can come with a strong argument convincing us other wise.

Since I have now understood that the MC specification deviates from the RFC standards, and the change I proposed is specific to the MC domain rather than general, I'm closing the PR.