juberti / cryptex

IETF Internet-Draft for Completely Encrypting RTP Header Extensions and Contributing Sources
Other
3 stars 4 forks source link

GENART Review #47

Closed aboba closed 2 years ago

aboba commented 2 years ago

https://datatracker.ietf.org/doc/review-ietf-avtcore-cryptex-05-genart-lc-dunbar-2022-04-05/

murillo128 commented 2 years ago

Section 4: this document defines a new "a=cryptex" Session Description Protocol (SDP) [RFC4566] attribute to indicate support. Then the next sentence states that "This attribute takes no value". Why "no value"? The first statement already says a new "a=cryptex" attribute. It is confusing.

I think that the "This attribute takes no value" makes sense in the context of SDP attributes and is clear for anyone familiar with the spec. We could further add that the attribute is a property attribute, either like

"This document defines a new "a=cryptex" Session Description Protocol (SDP) {{RFC4566}} property attribute to indicate support. This attribute takes no value, and can be used at the session level or media level."

or

"This document defines a new "a=cryptex" Session Description Protocol (SDP) {{RFC4566}} attribute to indicate support. This attribute is a property attribute and therefore takes no value, and can be used at the session level or media level."

Section 6.3: what does "region" mean in the statement? "The decryption procedure is identical to that of [RFC3711] except for the region to decrypt" Do you mean the "header" to be encrypted by the scheme described in this document?

Addressed in #49