murillo128
commented
2 years ago Q1: I suggest to change the Section 4 name from "Signaling" to "SDP Considerations", "SDP cryptex attrbute", or something like that.
Q6: I suggest to rename Section 9.1 to "SDP cryptex Attribute".
Done in https://github.com/juberti/cryptex/commit/43e8dfb4ae4b2063a566664b76c87c2b510bfbdb
---------- Forwarded message --------- From: Christer Holmberg christer.holmberg@ericsson.com Date: Fri, Jun 10, 2022 at 6:16 AM Subject: SDP Directorate review: draft-ietf-avtcore-cryptex To: IETF AVTCore WG avt@ietf.org, Murray S. Kucherawy superuser@gmail.com Cc: mmusic mmusic@ietf.org, avtcore-chairs@ietf.org avtcore-chairs@ietf.org
Hi,
I have performed an SDP directorate review of the draft. The review only focuses on the SDP related aspects (Sections 4 and 9.1) of the draft.
Q1:
I suggest to change the Section 4 name from "Signaling" to "SDP Considerations", "SDP cryptex attrbute", or something like that.
Q2:
Section 4 does not talk about subsequent offers and answers. If the attribute is NOT present in a subsequent offer/answer, does that mean that the endpoint no longer indicate support? If so, I think it would be good to explicitly indicate that, to avoid misunderstandings.
Q3:
Section 4 says:
"Once each peer has verified that the other party supports receiving RTP packets encrypted with Cryptex, senders can unilaterally decide whether to use the Cryptex mechanism or not."
I think it is good to point out that the decision to use Cryptex is made for each individual RTP packet, i.e., the sender might use Cryptex for some packets and might not use Cryptex for some packets (based on whatever policy).
Q4:
Section 4 says:
"If BUNDLE is in use and the a=cryptex attribute is present for a media line, it MUST be present for all media lines belonging to the same bundle group. This ensures that the encrypted MID header extensions used to demux BUNDLE can be processed correctly. When used with BUNDLE, this attribute is assigned to the TRANSPORT category [RFC8859]."
First, as the usage of Cryptex is optional, why mandate it on all media lines? Could you explain the MID header processing justficiation?
Second, if mandated on all media lines, it will apply also to non-RTP media lines (e.g., a WebRTC data channel), and then I think you need to have some explicit text about that.
Q5:
The text says:
"Peers MAY negotiate both Cryptex and the header extension mechanism defined in [RFC6904] via signaling, and if both mechanisms are supported, either one can be used for any given packet. However, if a packet is encrypted with Cryptex, it MUST NOT also use [RFC6904] header extension encryption, and vice versa."
To me this does not seem like only an SDP issue, but more a functional issue that should be described elsewhere in the document.
Q6:
I suggest to rename Section 9.1 to "SDP cryptex Attribute".
Regards,
Christer