Subject: [Perc] Review of draft-ietf-perc-double-07

From: Nils Ohlmeier <[email-masked]>
Date: Wed, 15 Nov 2017 11:44:19 +0800

Hello,

As an individual contributor I reviewed draft-ietf-perc-double-07.

I don’t think that the draft is ready for moving forward yet, because of the TODO in section 7.2.

Here is the more detailed list of my findings:
Section 1:
“See the framework document that describes this concept in more detail  in more detail in” one time “in more detail” should be enough.
“The original value of any RTP header field that is changed is included in a new RTP header extension called the Original Header Block.” double no longer uses an RTP header extension.
Section 3:
“Assign the key and salt values generated for the inner (end-to-end) algorithm to the first half of the key and salt for the double algorithm.” To me that sentence doesn’t make it 100% clear if only half of the salt is used or not. I would propose to change this to “… algorithm to the first half of the key and the first half to the salt for the double algorithm.” Same applies to the next bullet point.
Section 4:
From my reading when the receiving endpoint wants to extract the OHB it will read the last byte of the RTP payload (after decrypting and verifying the outer encryption). Then depending on the content of that byte, the Config, it might need to read up to three more bytes. Wouldn’t it be easier to always put the full 4 bytes of OHB at the end of the RTP payload and zero it out if not used? Then the receiving endpoint knows that it can always split of the last 4 bytes no matter what their content is.
It would be easier to read if the list of letter acronyms would be sorted by first occurrence in the config byte diagram.
Section 5.1
4th step: “Apply the inner cryptographic algorithm to the RTP packet.” Which packet is that? The packet from step 1 or the synthetic packet from step 3?
Section 5.2
2nd step: “… and are allowed …” should be “… and is allowed …”.
2nd step: I think we need a reference to a list of the part of the RTP packet it is allowed to modify.
3rd step: To me the first and the second sentences are contradicting them self a little bit. The first sentence starts with “If a changed RTP header field is not already in the OHB,…”. But that is not really an if condition here, because as the next sentence explains the relay would have not been allowed to touch/change the value if the value was already in the OHB (because it got already modified by a previous relay).
3rd step: similar to my comment for section 4, adding something to the OHB actually means removing the empty config byte of OHB, adding the original value and then append the new config byte.
3rd step: maybe the easier description would be here something like: Remove the original OHB from the end of the packet. If the value is already contained in the OHB it can’t modify the value again. Append all original values of values the relay modified and append the new config byte of the OHB.
3rd step: As we are appending now OHB at the end of the packet, we could actually allow a chain of media relays to modify values which the previous relay had modified already. The second relay would then have to append another OHB to the packet. As long the endpoint receives only one OHB with the original values from the sender I think this might work, if people are interested in such a feature.
Why is there no section describing how to handle repair packets for media relays?
Section 5.3
2nd step: why does this step say to skip the rest of the steps? I think the handling actually depends on the repair mechanism. But without further instructions this sounds like an endpoint could simply “play the media”, where I think it still needs to do inner decryption, because the repair packets from the media relay hopefully will still be encrypted end-to-end.
Section 7.1
It might be worth pointing out how to handle the receiver part of RTX. Which I believe is decrypt HBH2, undo RTX transformation, then follow the steps in section 5.3
Section 7.2
This section says literally ‘TODO’. This needs to written out how to handle option A from the Prag slides.
Section 7.3:
“… it MUST be encrypted in repair mode packet.” doesn’t quite parse for me. I guess this meant to be “… it MUST be encrypted in packet repair mode.”.
Section 8
The last paragraph which talks about the changes in size of the packet probably needs to updated, because it still considers the OHB to be a RTP header extension. And I think it should also include the sizes caused by doing tripple encryption for any of the repair mechanisms.
Section 9
This section also looks to me like it needs a brief update for the fact that the OHB no longer goes into the RTP header extensions, e.g. “Any additional headers added by the Media Distributor…”.
Section 10.1
This section needs to be deleted as we no longer use a RTP header extension.
Best regards
  Nils Ohlmeier, as an individual contributor