Hi Lars, Thanks for the review, please find my comments below.
Yours, Daniel On Tue, Apr 5, 2022 at 10:25 AM Lars Eggert via Datatracker < [email protected]> wrote: > Lars Eggert has entered the following ballot position for > draft-ietf-lwig-minimal-esp-08: No Objection > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to > https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-lwig-minimal-esp/ > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Section 2. , paragraph 6, comment: > > [RFC4303] does not require the SPI to be randomly generated over 32 > > bits. However, this is the recommended way to generate SPIs as it > > provides some privacy benefits and avoids, for example, correlation > > between ESP communications. To randomly generate a 32 bit SPI, the > > node generates a random 32 bit valueand checks it does not fall in > > the 0-255 range. If the SPI has an acceptable value, it is used to > > index the inbound session, otherwise the SPI is re-generated until an > > acceptable value is found. > > Wouldn't it be simpler to compute a 24-bit random value and left-shift it > by > eight? Or left-shift the 32-bit value; both remove the need to check. > I think the situation we want to avoid is to have the 24 right most bits to be set to zero. With a random 32 bit value, the probability to have are rejected value is 2**8 / 2**32. If you take a 24 bit value that you left-shift by eight that probability becomes 2**8/2**24. If you take a 32 bit value you left shift by eight that probability becomes 2**16/2**32. Unless I am missing something, we cannot avoid the check. > > Found terminology that should be reviewed for inclusivity; see > https://www.rfc-editor.org/part2/#inclusive_language for background and > more > guidance: > > * Term "dummy"; alternatives might be "placeholder", "sample", "stand-in", > "substitute". > I perceive dummy packets as almost a terminology of RFC4303 (see section 2.6). If we were to change it, I would propose a "random packet". I am waiting for more guidance to change it. > Thanks to Roni Even for their General Area Review Team (Gen-ART) review > (https://mailarchive.ietf.org/arch/msg/gen-art/W3R6WdPRLgAuvMIJYWnld5uaCmU > ). > > > ------------------------------------------------------------------------------- > NIT > > ------------------------------------------------------------------------------- > All comments below are about very minor potential issues that you may > choose to > address in some way - or ignore - as you see fit. Some were flagged by > automated tools (via https://github.com/larseggert/ietf-reviewtool), so > there > will likely be some false positives. There is no need to let me know what > you > did with these suggestions. > Section 2. , paragraph 6, nit: > - node generates a random 32 bit valueand checks it does not fall in > + node generates a random 32 bit value and checks it does not fall in > + + > > changed > Section 3. , paragraph 6, nit: > - are no requirements to implement an anti-replay protection mechanism > - implemented by IPsec. Similarly to the SN the implementation of anti > - ----------------------- > + are no requirements to implement an anti-replay protection mechanism. > + + > I am missing the change. > Section 4. , paragraph 4, nit: > - would typically be the case when the Data Payload is of fix size. > + would typically be the case when the Data Payload is of fixed size. > + ++ > > changed > Document still refers to the "Simplified BSD License", which was corrected > in > the TLP on September 21, 2021. It should instead refer to the "Revised BSD > License". > > Uncited references: [RFC2119] and [RFC8174]. > > Section 1. , paragraph 1, nit: > > igure 1 describes an ESP Packet. Currently ESP is implemented in the > kernel o > > ^^^^^^^^^ > A comma may be missing after the conjunctive/linking adverb "Currently". > > changed > Section 1. , paragraph 2, nit: > > y to fit multiple purpose usage of these OS. However, completeness of > the IPs > > ^^^^^^^^ > The plural demonstrative "these" does not agree with the singular noun > "OS". > > changed to these OSes. > Section 1. , paragraph 2, nit: > > as well as multipurpose scope of these OS is often performed at the > expense > > ^^^^^^^^ > The plural demonstrative "these" does not agree with the singular noun > "OS". > > changed > Section 1. , paragraph 3, nit: > > or constrained devices remains inter-operable with the standard ESP > implemen > > ^^^^^^^^^^^^^^ > This word is normally spelled as one. > > changed > Section 2. , paragraph 2, nit: > > ommunications, this document recommends to index SA with the SPI only. > The i > > ^^^^^^^^^^^^^^^^^^^ > The verb "recommends" is used with the gerund form. > > changed to recommends indexing > Section 2.1. , paragraph 3, nit: > > g the key is being used. For example, a SPI might be encoded with the > Securit > > ^ > Use "an" instead of "a" if the following word starts with a vowel sound, > e.g. > "an article", "an hour". > > keeping "a SPI" as "a security policy index". > Section 2.1. , paragraph 4, nit: > > h privacy and security concerns. Typically some specific values or > subset of > > ^^^^^^^^^ > A comma may be missing after the conjunctive/linking adverb "Typically". > > added > Section 2.1. , paragraph 5, nit: > > ed information by ESP itself, these information may also be leaked > otherwise > > ^^^^^^^^^^^^^^^^^ > The plural demonstrative "these" does not agree with the singular noun > "information". > > changed to pieces of information > Section 2.1. , paragraph 5, nit: > > affic pattern before determining non random SPI can be used. Typically, > temp > > ^^^^^^^^^^ > This expression is normally spelled as one or with a hyphen. > > changed to a non random > Section 2.1. , paragraph 5, nit: > > s, used outdoors may not leak privacy sensitive information and most of > its t > > ^^^^^^^^^^^^^^^^^ > This word is normally spelled with a hyphen. > > I do not see what needs to be changed > Section 2.1. , paragraph 5, nit: > > opened) may leak truly little privacy sensitive information outside the > local > > ^^^^^^^^^^^^^^^^^ > This word is normally spelled with a hyphen. > > idem > Section 2.1. , paragraph 6, nit: > > packet. The SN is set by the sender so the receiver can implement > anti-repl > > ^^^ > Use a comma before "so" if it connects two independent clauses (unless > they are > closely connected and short). > > I did not add the coma. I do see them related and not independent. > Section 3. , paragraph 6, nit: > > ability to spoof and replay an acknowledgement is of limited interest > and mig > > ^^^^^^^^^^^^^^^ > Do not mix variants of the same word ("acknowledgement" and > "acknowledgment") > within a single text. > > I do not understand the comment. > Section 3. , paragraph 6, nit: > > y discarding any packets that present a SN whose value is too much in > the pa > > ^ > Use "an" instead of "a" if the following word starts with a vowel sound, > e.g. > "an article", "an hour". > > keeping a SN as a sequence number > Section 3. , paragraph 6, nit: > > s based on the largest possible value a SN can take over a session. When > SN > > ^ > Use "an" instead of "a" if the following word starts with a vowel sound, > e.g. > "an article", "an hour". > > keepin a SN > Section 3. , paragraph 7, nit: > > ned devices, this document recommends to implement some rekey mechanisms > (see > > ^^^^^^^^^^^^^^^^^^^^^^^ > The verb "recommends" is used with the gerund form. > > change to recommends implementing > Section 4. , paragraph 5, nit: > > ns - may also reveal important privacy oriented information. Some > constrained > > ^^^^^^^^^^^^^^^^ > This word is normally spelled with a hyphen. > > I do not understand the suggested change > Section 4. , paragraph 5, nit: > > a sufficient tradeoff between the require energy to send additional > payload > > ^^^^^^^ > The word "require" is not a noun. Did you mean "requirement"? > > change to the required energy > Section 5. , paragraph 4, nit: > > on the cryptographic suite used. Currently [RFC8221] only recommends > cryptog > > ^^^^^^^^^ > A comma may be missing after the conjunctive/linking adverb "Currently". > > coma added > Section 5. , paragraph 4, nit: > > ent with a size different from zero. It length is defined by the > security rec > > ^^ > It seems that the possessive pronoun "its" fits better in this context. > Please > verify. > > changed to its length > Section 7. , paragraph 2, nit: > > oss reboots, this document recommends to consider algorithms that are > nonce > > ^^^^^^^^^^^^^^^^^^^^^^ > The verb "recommends" is used with the gerund form. > > changed to recommends considering > Section 7. , paragraph 5, nit: > > of the encryption algorithm transform or the energy associated with it > are es > > ^^^ > Use a comma before "or" if it connects two independent clauses (unless > they are > closely connected and short). > > no changes. > Section 7. , paragraph 10, nit: > > eration must follow [RFC4086]. In addition [SP-800-90A-Rev-1] provides > approp > > ^^^^^^^^ > A comma may be missing after the conjunctive/linking adverb "addition". > > coma added > Section 9. , paragraph 2, nit: > > In particular Scott Fluhrer suggested to include the rekey index in the > SPI. > > ^^^^^^^^^^^^^^^^^^^^ > The verb "suggested" is used with the gerund form. > > > changed > > _______________________________________________ > Lwip mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/lwip > -- Daniel Migault Ericsson
_______________________________________________ Lwip mailing list [email protected] https://www.ietf.org/mailman/listinfo/lwip
