Hi David, Thank you for the follow up. The diff can still be tracked in the same URL: https://author-tools.ietf.org/api/iddiff?url_1=https://netmod-wg.github.io/enhanced-acl-netmod/draft-ietf-netmod-acl-extensions.txt&url_2=https://netmod-wg.github.io/enhanced-acl-netmod/TSV-review/draft-ietf-netmod-acl-extensions.txt. Please see inline. Cheers, Med > -----Message d'origine----- > De : Black, David <David.Black@xxxxxxxx> > Envoyé : vendredi 8 novembre 2024 19:55 > À : BOUCADAIR Mohamed INNOV/NET <mohamed.boucadair@xxxxxxxxxx>; > tsv-art@xxxxxxxx > Cc : draft-ietf-netmod-acl-extensions.all@xxxxxxxx; last- > call@xxxxxxxx; netmod@xxxxxxxx; Black, David > <David.Black@xxxxxxxx> > Objet : RE: Tsvart last call review of draft-ietf-netmod-acl- > extensions-11 > > > Hi Med, > > Working bottom-up ... > > >> Editorial - Payload: The use of the word "payload" by itself > as a type initially confused me, > > > However, I changed the name of the grouping to "payload-match" > as that is indeed better. Thanks. > > That's fine, check this one off as resolved. [Med] ACK. > > >> Editorial - TCP flags: It would be helpful for the description > to include a list of which flag is in which position in the > bitmask (see above). > > > [Med] I can add a pointer to ... > > In section 3.4 of the revised draft, I see the added mention of > Section 3.1 to focus the reference to RFC 9293 on that section. > That will suffice. > [Med] Thanks. To better help readers to find up-to-date meaning of the flags, I also added the following: NEW: Assigned TCP flags are maintained in the "TCP Header Flags" registry under the "Transmission Control Protocol (TCP) Parameters" registry group {{IANA-TCP-FLAGS}}. > >> [B] Minor technical issue: payload encryption. > >> > >> The security considerations ought to point out that payload > match won't work on encrypted payloads, even though that's > relatively obvious. > > > [Med] We don't actually need to decrypt the packet. The match > is based on the observable pattern (ddos mitigation, typically): > > > > +-- prefix? Binary > > We're talking past each other, which is an indication that > something does need to be added. Section 3.6 says: > > Some transport protocols use existing protocols (e.g., TCP > or UDP) as substrate. > The match criteria for such protocols may rely upon the > 'protocol' under 'l3', > TCP/UDP match criteria, part of the TCP/UDP payload, or a > combination thereof. > > If the match on "part of the TCP/UDP payload" is looking for a > specific value at a specific offset in the payloads (e.g., all > packets for the protocol have the same value at that specific > offset), then encryption of the payload that includes encryption > of the value at that specific offset results in different packets > having different and unpredictable values at that offset, > rendering it infeasible to match on the specific value at that > specific offset. > > Something should be said, but not what I just wrote - it ought to > suffice to point out that payload match only works as intended on > unencrypted payloads or unencrypted portions of partially > encrypted payloads. As I noted earlier, this is relatively > obvious, and does not require a reference to RFC 9065. [Med] I added the following text: NEW: The document defines a match policy based on a pattern that can be observed in a packet. For example, such a policy can be combined with header-based matches in the context of DDoS mitigation. Filtering based on a pattern match is deterministic for packets with unencrypted data. However, the efficiency for encrypted packets depend on the presence of an unvarying pattern. Please let me know if we need to say more. Thanks. > > > > [A] Minor technical issue: TCP flags bitmask > > I'm still lost here, as I can't quickly figure out how to map the > provisions in this draft to the specific flags in the TCP header. > This should be blindingly obvious to the reader in order to avoid > potential interoperability problems. > [Med] We are pointing to the exact section in 9293 where the flags are defined. Please note that this is similar to the details in https://datatracker.ietf.org/doc/html/rfc8955#name-type-9-tcp-flags, which is a typical trigger for the ACL policy. That's said, I added a new sentence to point to the IANA registry as well for the position and meaning of each flag. The match value can be even on flags that are now assigned or "weird" values (set SYN/FIN, unset all flags, etc.). > Thanks, --David > > -----Original Message----- > From: mohamed.boucadair@xxxxxxxxxx <mohamed.boucadair@xxxxxxxxxx> > Sent: Tuesday, November 5, 2024 2:56 AM > To: Black, David <David.Black@xxxxxxxx>; tsv-art@xxxxxxxx > Cc: draft-ietf-netmod-acl-extensions.all@xxxxxxxx; last- > call@xxxxxxxx; netmod@xxxxxxxx > Subject: RE: Tsvart last call review of draft-ietf-netmod-acl- > extensions-11 > > > [EXTERNAL EMAIL] > > Hi David, > > Thanks for the review. > > The diff to track the changes can be seen here: > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > Furldefense.com%2Fv3%2F__https%3A%2F%2Fauthor- > tools.ietf.org%2Fapi%2Fiddiff%3Furl_1%3Dhttps%3A**Anetmod- > wg.github.io*enhanced-acl-netmod*draft-ietf-netmod-acl- > extensions.txt%26url_2%3Dhttps%3A**Anetmod-wg.github.io*enhanced- > acl-netmod*TSV-review*draft-ietf-netmod-acl- > extensions.txt__%3BLy8vLy8vLy8v!!LpKI!hBXAxRar8NOLzgnyfwEv94Rq2OW > RFUp23238NekAf- > 2L7QMq0xGpNQKSHb1khck0bENVGvIKb3JJTvE0eqvBu_sdm0lx%24&data=05%7C0 > 2%7Cmohamed.boucadair%40orange.com%7Cc42ee4ab709b44d7293908dd0026 > d746%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638666889008732 > 406%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=LwxiUJmYhIcsD6nF > kfPEheGKehjd%2B20mq1s47Jg%2FgqQ%3D&reserved=0 [author- > tools[.]ietf[.]org]. > > Please see inline. > > Cheers, > Med > > > -----Message d'origine----- > > De : David Black via Datatracker <noreply@xxxxxxxx> > > Envoyé : lundi 4 novembre 2024 19:01 > > À : tsv-art@xxxxxxxx > > Cc : draft-ietf-netmod-acl-extensions.all@xxxxxxxx; last- > > call@xxxxxxxx; netmod@xxxxxxxx; david.black@xxxxxxxx > > Objet : Tsvart last call review of draft-ietf-netmod-acl- > > extensions-11 > > > > > > Reviewer: David Black > > Review result: Ready with Issues > > > > This document has been reviewed as part of the transport area > > review team's ongoing effort to review key IETF documents. > These > > comments were written primarily for the transport area > directors, > > but are copied to the document's authors and WG to allow them > to > > address any issues raised and also to the IETF discussion list > > for information. > > > > When done at the time of IETF Last Call, the authors should > > consider this review as part of the last-call comments they > > receive. Please always CC tsv-art@xxxxxxxx if you reply to or > > forward this review. > > > > This draft extends the ACL model defined in RFC 8341 to > > additional areas of functionality. This transport area review > > focuses on the TCP flags and payload extensions. I found a > minor > > issue in each of these two areas. > > > > [A] Minor technical issue: TCP flags bitmask > > > > Under "grouping tcp-flags", I see: > > > > case builtin { > > leaf bitmask { > > type uint16; > > description > > "The bitmask matches the last 4 bits of byte 12 and > > 13 of > > the TCP header. For clarity, the 4 bits of byte > 12 > > corresponding to the TCP data offset field are not > > included in any matching."; > > reference > > "RFC 9293: Transmission Control Protocol (TCP), > > Section 3.1"; > > } > > } > > > > That's peculiar. Byte 12 in the TCP header is not involved, so > > why is it included? Is that because uint16 the smallest type > > that can be used? > > Also, why is the bitmask limited to 4 bits when there are 8 > flag > > bits that are defined for the other representation case ("case > > explicit" > > references "identity tcp-flag")? > > > > [Med] We assumed a byte offset of 0 when counting, but I think > this may be confusing. Changed the text to refer to bytes 13 and > 14. We can restrict the range for uint16. The reason we didn't > initially is because we wanted the type to match what is defined > in other protocols that would trigger such ACL. See > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > Furldefense.com%2Fv3%2F__https%3A%2F%2Fdatatracker.ietf.org%2Fdoc > %2Fhtml%2Frfc8955*name-type-9-tcp- > flags__%3BIw!!LpKI!hBXAxRar8NOLzgnyfwEv94Rq2OWRFUp23238NekAf- > 2L7QMq0xGpNQKSHb1khck0bENVGvIKb3JJTvE0eqvBu3Q9oIXY%24&data=05%7C0 > 2%7Cmohamed.boucadair%40orange.com%7Cc42ee4ab709b44d7293908dd0026 > d746%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638666889008754 > 148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=lLGqO0xvPaiVlQ7s > eU8msY%2BH4OkbKF928S2eUMGhi%2Bk%3D&reserved=0 > [datatracker[.]ietf[.]org] (2-octet bitmask in particular). Let > me know if you have a preference here. > > > [B] Minor technical issue: payload encryption. > > > > The security considerations ought to point out that payload > match > > won't work on encrypted payloads, even though that's relatively > > obvious. > > [Med] We don't actually need to decrypt the packet. The match is > based on the observable pattern (ddos mitigation, typically): > > +-- prefix? binary > > A sentence or two to that effect ought to suffice, > > perhaps complemented by referencing the extensive discussion of > > transport header confidentiality implications in RFC 9065 > > (whether to add a reference to RFC 9065 is left to the authors' > > discretion). > > [Med] RFC 9065 includes the following > > "Another method, if the endpoints do not provide this > information, is to use an on-path network device that relies on > pattern inferences in the traffic..." > > Which seems to be close to the match we are discussing. > > I'm not sure if a modification is needed here. > > > > > Editorial - TCP flags: It would be helpful for the description > to > > include a list of which flag is in which position in the > bitmask > > (see above). > > > > [Med] I can add a pointer to > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > Furldefense.com%2Fv3%2F__https%3A%2F%2Fdatatracker.ietf.org%2Fdoc > %2Fhtml%2Frfc9293*table- > 7__%3BIw!!LpKI!hBXAxRar8NOLzgnyfwEv94Rq2OWRFUp23238NekAf- > 2L7QMq0xGpNQKSHb1khck0bENVGvIKb3JJTvE0eqvBuyOs3R4A%24&data=05%7C0 > 2%7Cmohamed.boucadair%40orange.com%7Cc42ee4ab709b44d7293908dd0026 > d746%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638666889008768 > 372%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7qZhLTawBoMc1hPR > yvAcuiugszo5H94rSgDGbgapZcM%3D&reserved=0 > [datatracker[.]ietf[.]org], but that information will be stale as > (in theory) we can define new flag bits. > > > Editorial - Payload: The use of the word "payload" by itself as > a > > type initially confused me, e.g., in: > > > > augment "/acl:acls/acl:acl/acl:aces/acl:ace" > > + "/acl:matches" { > > description > > "Adds a match type based on the payload."; > > choice payload { > > description > > "Matches based upon a prefix pattern."; > > container prefix-pattern { > > if-feature "match-on-payload"; > > description > > "Indicates the rule to perform the payload-based > > match."; > > uses payload; > > } > > } > > > > I initially read "payload" as an object representing the > payload, > > whereas it actually refers to an object that contains rules for > > matching on the payload (which is the only possibility in this > > context). This would be somewhat clearer if "payload" were > > changed to "payload-match" here and elsewhere in the draft. > > > > > [Med] We don't use "match" in the name of the choice because the > node is under matches > > augment /acl:acls/acl:acl/acl:aces/acl:ace/acl:matches: > +--rw (payload)? > > RFC8407 says: > > " Child nodes within a container or list SHOULD NOT > replicate the parent identifier." > > However, I changed the name of the grouping to "payload-match" as > that is indeed better. Thanks. > _________________________________________________________________ > ___________________________________________ > Ce message et ses pieces jointes peuvent contenir des > informations confidentielles ou privilegiees et ne doivent donc > pas etre diffuses, exploites ou copies sans autorisation. Si vous > avez recu ce message par erreur, veuillez le signaler > a l'expediteur et le detruire ainsi que les pieces jointes. Les > messages electroniques etant susceptibles d'alteration, > Orange decline toute responsabilite si ce message a ete altere, > deforme ou falsifie. Merci. > > This message and its attachments may contain confidential or > privileged information that may be protected by law; > they should not be distributed, used or copied without > authorisation. > If you have received this email in error, please notify the > sender and delete this message and its attachments. > As emails may be altered, Orange is not liable for messages that > have been modified, changed or falsified. > Thank you. ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
