Hi Christer,
Sorry, just now seeing this. Responses inline. PR here:
--RLB
On Sat, Oct 20, 2018 at 6:06 PM Christer Holmberg <christer.holmberg@xxxxxxxxxxxx> wrote:
Reviewer: Christer Holmberg
Review result: Ready with Issues
I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please treat these comments just
like any other last call comments.
For more information, please see the FAQ at
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
Document: draft-ietf-perc-srtp-ekt-diet-09
Reviewer: Christer Holmberg
Review Date: 2018-10-20
IETF LC End Date: 2018-11-01
IESG Telechat date: Not scheduled for a telechat
Summary: The document is well written, and easy to read. But, I think some
things still need to be clarified. I also have some editorial modification
suggestions.
Major issues:
--------------
Q1_1:
The text in section 1 says that EKT removes the need for co-ordinating SSRCs,
and that an endpoint can choose whatever value it wants.
However, I can't find any explanation on how EKT removes that need.
The answer is toward the end of the introduction: "EKT does not control the manner in which the SSRC is generated..." -- it just makes the distribution of security information easier.. I extended that paragraph to clarify.
Q1_3:
The text in section 1 says that EKT is not intended to replace key
establishment mechanisms, but to "be used in conjunction".
However, there is no description on how that works in conjunction with existing
mechanisms (e.g., together with SDP Offer/Answer), and whether existing
mechanisms need to be modified in order to work in conjunction with EKT.
Section 5.3 does say that the SDP O/A exchange is not affected. If that is
true, then you need to describe how SSRC values etc signalled in SDP relate to
values signalled using EKT, what happens if there are mismatches etc.
This document only defines one such augmentation -- the integration with DTLS-SRTP. I extended the relevant part of the introduction to say this.
Minor issues:
--------------
Q1_2:
The text in section 1 says:
"However, there are several cases in which conventional signaling systems
cannot easily provide all of the coordination required."
I think some examples would be useful.
Disagree.
Q1_4:
The text in section 1 says that providing SSRCs etc using signaling systems
cause layer violations.
Is this layer violation described somewhere? If so, I think a reference would
be useful.
I just deleted this sentence.. It wasn't adding much.
Q4-2_1:
The text in section 4.2 says:
"All of the received EKT parameter sets SHOULD be stored by all of the
participants in an SRTP session, for use in processing inbound SRTP
and SRTCP traffic."
What is the reason for SHOULD, instead of MUST? What happens if an endpoint
does NOT store the EKT parameter sets?
I think the SHOULD here is to allow, e.g., cache management. Clarified the consequences.
Q4-2-1_2:
The text in section 4.2.1 says:
"Outbound packets SHOULD continue to use the old SRTP Master Key for
250 ms after sending any new key."
What is the reason for SHOULD, instead of MUST?
I don't think this should be a MUST. For example, how accurately are you going to measure compliance? If the change-over is 10ms short 10% of the time, is the endpoint out of compliance. Plus, there's minimal interop impact; this just helps things go smoothly.
Q4-3_2:
The text in section 4.3 says:
"Section 4.2.1 recommends that SRTP senders continue using an old key
for some time after sending a new key in an EKT tag."
The text in section 4.2.1 contains a SHOULD, so it is more than a
recommendation.
According to RFC 2119, a SHOULD is precisely a recommendation:
"""
3. SHOULD This word, or the adjective "RECOMMENDED", mean that there
may exist valid reasons in particular circumstances to ignore a
particular item, but the full implications must be understood and
carefully weighed before choosing a different course.
may exist valid reasons in particular circumstances to ignore a
particular item, but the full implications must be understood and
carefully weighed before choosing a different course.
"""
Nits/editorial comments:
--------------------------
Q1_5:
I think Section 1 should also indicate that EKT is an DTLS extension, similar
to the Abstract. Otherwise, when you say that EKT is a way to distribute
information, it is unclear why EKT doesn't violate layers.
Done.
I think that Section 2 (Overview) could be combined with Section 1
(Introduction). Introduction sections in RFCs typically provide both
background, problem statement and an overview of the mechanism - and sometimes
also the document structure.
Disagree.
Q4_1:
The text in section 4.1 says:
"EKT MUST NOT be used in conjunction with SRTP's MKI (Master Key
Identifier) or with SRTP's <From, To> [RFC3711], as those SRTP
features duplicate some of the functions of EKT. Senders MUST NOT
include MKI when using EKT. Receivers SHOULD simply ignore any MKI
field received if EKT is in use."
I suggest to put this text in a dedicated Applicability section.
Disagree.
Q4-1_1:
The text in section 4.1 says:
"Together, these data elements are called an EKT parameter set. Each
distinct EKT parameter set that is used MUST be associated with a
distinct SPI value to avoid ambiguity."
I suggest to defined "EKT parameter set" in the same way as the other
terminology. I.e.,
"EKT parameter set: The parameters indicated by the SPI"
.....or something like that.
There's not a terminology section, so I didn't do exactly what you say. But I pulled this out into a separate section so that it doesn't interrupt the flow, and clarified that the SPI-params association is set up by the DTLS-SRTP extension.
Q4-2-1_1:
For the section names of sections 4.2.1 and 4.2.2, I suggest to talk about
sending/transmitting and receiving instead of inbound and outbound.
Disagree
Q4-3_1:
In section 4.3, I think the name of the section ("Implementation Notes") is a
little confusing. Also, is there a reason why the text is not in section 4.2.1
and/or 4.2.2?
Just folded this into section 4.2.2.
Q4-4_1:
Sections 4.4 and 4.4.1 have the same section names. Please change one of them
(e.g., change 4.4.1 to "Default Cipher").
Changed to "AES Key Wrap"
Q4-6_1:
I think the text in section 4.6 should be placed in the Applicability section I
suggested earlier (Q4_1).
Merged it into section 4.
Q5_1:
In section 5, I suggest to change the section name to "DTLS-SRTP
Considerations".
Disagree.
_______________________________________________
Perc mailing list
Perc@xxxxxxxx
https://www.ietf.org/mailman/listinfo/perc
