On Tue, Nov 26, 2024 at 10:14 AM Jon Geater via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Jon Geater > Review result: Has Nits > Hi Jon, Thanks for the review! > *Executive summary:* > I see no obvious security problems _introduced_ by this document. Indeed, it’s > trying to help plug holes caused by earlier work, which is to be applauded. In > light of this I see no reason to object to the document, although the spectre > of unreliable HBH processing remains a bit unsettling. I believe it would be an > improvement for the security considerations section to include guidance that no > security-relevant information should be included in HBH since there is no > guarantee that it will be actioned, or fail close. The allowance that nodes may ignore HBH options was first introduced in RFC8200, this was in reaction to the reality that many routers were already ignoring them (but ignoring them is at least better than dropping them!). RFC9673 further clarified this. Similar to this draft, RFC8200 nor RFC9673 don't explicitly address the concern that security related information could be in HBH options which could be ignored by all nodes. That being said, there is a pertinent difference between this draft and RFC9673. This draft states "Per [RFC8200], a destination host that receives a packet with extension headers must process all the extension headers in the packet before accepting the packet and processing the payload.", but RFC9673 states "Hosts SHOULD process the Hop-by-Hop Options header in received packets.". The intent of the eh-limits wording is that all EH and all EH options, including HBH options, must be properly processed by a host or the packet is dropped because of the security concern you mentioned. This is how it works already in Linux so the vast majority of destination hosts are already covered. This draft gives a non-normative requirement, but maybe a normative update to RFC9673 or RFC8200 should be considered? > > *Observations:* > * The document only has one author. Given numerous opinionated assertions about > what is "realistic" and "useful" it would be good to see evidence that the > broad IPv6 community agrees with the proposed limits (I acknowledge section 8 > has good cover for this). * I’m a little confused why this would be > “Informational” rather than Standards Track for 3 primary reasons: it claims to > extend RFC8504; it includes a lot of normative language; it claims a primary > benefit in interoperability. All of these imply a benefit to establishing some > more formal normative content. The draft started a Proposed Standard, was changed to BCP, and then went to Informational. I think the rationale was that this draft should follow the same trajectory as RFC8504 that obsoleted RFC6434 which was an Informational draft. > I understand this changed along the life of the > document but it now seems a little confused in that regard. * In contrast, the > document is very discursive. Is all the justification and external referencing > really necessary? While this is a great way of defending choices and > establishing (rough) consensus, a specification document just needs to tell > people what to do in a way that ensures they all do it the same. This > discussion will go out of date eventually. The justification is mostly contained in the Appendix as just support (I think it's helpful to give some basis for why the defaults for manifest constants were chosen). Also, the specific default values for limits are subject to change at some point, hence why this draft probably would eventually become BCP not Proposed Standard. > * It’s interesting to me that this > document is almost 3 years old by this point, had WGLC issues from at least 1 > year ago, and has not been discussed officially at (at least) the past 3 IETF > meetings. There has been modest discussion on the list and I am satisfied with > the shepherd’s review but I think that emphasises the ‘Informational’ decision. > Is there adoption/implementer support? * There were a couple of detailed reviews by the chairs, I'm not sure all of that discussion was on the list. Also, for "running code", several of the limits are already in use in Linux for a while. For instance, the limit in Linux for the maximum number of HBH or DestOpts is eight by default (as stated in the draft). This was set more than four years ago so it's widely deployed by now (and hasn't yet brought down the Internet :-) ). > Many of the recommendations say a > server SHOULD not do things unless it is sure every node on the route is able > to deal with it. How can it possibly know this? Surely these amount to > prohibitions (or a dare….) Why not be brave and say “this is the limit”? Mostly because it's not enforceable so if we made these MUSTs that might retroactively put many legacy systems out of protocol compliance. > * Page > 5: “The intent of default limits is to establish an expected baseline of > support.” – again suggests this should be on Standards Track otherwise isn’t it > essentially meaningless? I think it's as meaningful as the requirements in RFC8504 (BCP). > * All in all this seems to point much more to a > problem with the definition of HBH rather than the need for a new document. To be clear, this draft is not just about HBH, but deals with all Extension Headers. > Coming back to my legitimate area of expertise the rather uncertain treatment > of HBH options is troubling if they ever carry security sensitive information. > In some ways it feels very similar to X.509 V3 extensions and the critical > flag. But in and of itself it does not make the situation worse and offers some > ideas for limited improvements. > > *Nits:* > * Grammar/typo in Introduction: "[...] including a myriad of use cases those > are obviously outside the realm of ever being realistic or useful [...]" should > be "[...] including myriad use cases that are obviously outside the realm of > ever being realistic or useful [...]" * typo top of page 5: “limited is > exceeded” should be “limit is exceeded” * grammar/typo: “The potential for this > DOS attack exists routers, hosts, and intermediate nodes.”. Missing “in”? > Will fix. Thanks, Tom > > -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
