Hi, Ben,
Thanks so much for your feedback! Please find my comments in-line...
On 12/01/2014 08:58 PM, Ben Campbell wrote:
> Document: draft-ietf-opsec-dhcpv6-shield-04 Reviewer: Ben Campbell
> Review Date: 2014-12-01 IETF LC End Date: 2014-12-01
>
> Summary: This draft is almost ready for publication.I have some
> questions and comments that might should be considered prior to
> publication. (I am leaving off my usual "ready for publication as an
> XXX" part, because I have questions on that, below.)
>
> Major issues:
>
> (Note: This is not so much a "major issue" as a meta-concern that
> doesn't fit well in the major/minor/nit structure.)
>
> I have questions about the intended status. The draft lists BCP. It's
> not clear to me if we intend to say that it's a "best practice" to
> implement DHCPv6 filtering, or that, if you do implement it, it's a
> best practice to do it like this. Given the strength of a BCP, I
> think it's worth clarifying that.
BCP of implementing it, if you do.
How about adding this text to the intro:
"This document specifies a Best Current Practice for the implementation
of DHCPv6 Shield (DHCPv6)"
?
> Minor issues:
>
> -- abstract, last sentence:
>
> I didn't find this assertion in the body itself. It would be nice to
> see support for it (perhaps with citations).
I guess one could provide references to some vendor's manuals? Is that
what you have in mind? (although I'd prefer not to do so).
> -- section 4:
>
> Am I correct in understanding that this is opt in only? You would
> disallow a rule of the form of "allow on any port except [list]"?
Not sure what you mean.
The idea is that if you want to enable dhcpv6 shield, you need to
specify on which port(s) the dhcpv6 server(s) is/are connected.
> Nits/editorial comments:
>
> -- section 1, 2nd paragraph:
>
> This is the first mention of "DHCP-Shield" I found, not counting the
> doc name and headers. It would be helpful to have an early mention
> that "DHCP-Shield" is the name of the thing that this draft defines.
>
> -- section 1, 3rd paragraph:
>
> It would be helpful to define what a "DHCP-Shield device" is, prior
> to talking about deploying and configuring them.
How about adding (in Section 1) the following text:
This document specifies DHCPv6 Shield: a set of filtering
rules meant to mitigate attacks that employ DHCPv6-server
packets. Throughout this document we refer to a device
implementing the DHCPv6 Shield filtering rules as the "DHCPv6
Shield device"
?
> -- section 3, paragraph ending with with "... used as follows
> [RFC7112]:"
>
> I'm a bit confused by the citation. Are these defined "as follows",
> or in 7112?
>
> -- section 3, "Extension Header"
>
> -- these are IPv6 extension headers, right?
Yes. Should we explicitly say so?
> -- section 3, "IPv6 Header chain"
>
> Is there a relevant citation for this?
Other than RFC7112?
> Also, while this section talks about some aspects of header chains,
> it never actually defines the term.
Which one?
> -- section 3, "Upper-Layer Header"
>
> Again, this section talks about the term without defining it.
>
> -- section 5, list entry "1": "... the IPv6 entire header chain ..."
Not sure what you mean: Section 3 is all about defining the terms. Am I
missing something?
> should this be "... the entire IPv6 header chain ..."?
Yes. Will fix this.
> -- section 3, rational for item 1: "An implementation that has such
> an implementation-specific limit MUST NOT claim compliance with this
> specification."
>
> That's an odd use of 2119 language; I assume this to be true anytime
> an implementation violates a MUST/MUST NOT assertion.
You're right. Should we just change this to lowercase, or do you think
we should remove the whole sentence?
> -- section 3, rational for list item 3:
>
> It would be helpful if this rational said why dropping unrecognized
> next header values is needed for the purpose, not just the fact that
> 7045 allows it to be dropped.
How about adding this at the end of the RATIONALE:
"An unrecognized Next Header value could possibly identify an IPv6
Extension Header, and thus be leveraged to conceal a DHCPv6-server packet".
> -- section 3, list item 4:
>
> Am I correct in assuming that there might be other (non-dhcp) reasons
> a device might still drop packets that otherwise pass the
> dhcpv6-shield tests?
Yes. Why?
FWIW, the filtering rules in this document just talk about whether a
packet should be blocked or not based on these rules.
> -- section 3, paragraph after list item 4:
>
> The comment that dropped packets SHOULD be logged is redundant with
> the same statement in the numbered rules.
How about changing this:
If a packet is dropped due to this filtering policy, then the packet
drop event SHOULD be logged in an implementation-specific manner as a
security fault. The logging mechanism SHOULD include a drop counter
dedicated to DHCPv6-Shield packet drops.
to:
The above rules require that if a packet is dropped due to this
filtering policy, the packet drop event be logged in an
implementation-specific manner as a security fault. The logging
mechanism SHOULD include a drop counter
dedicated to DHCPv6-Shield packet drops.
?
(and there are a few more requirements when it comes to *what* should be
logged, as noted by others during the IETF LC).
> -- section 7, first paragraph:
>
> Please expand DoS on first mention.
Will do. Thanks!
> -- section 7, 2nd to last paragraph: "... on a port not allowed to
> send such packets ..."
>
> Should that be "forward" or "receive"? (I assume this still talks
> about L2 switch "ports", not UDP ports?)
Yes. But we will clarify the terminology and use "is not allowed to
receive.." in all cases.
Thanks!
Best regards,
--
Fernando Gont
SI6 Networks
e-mail: fgont@xxxxxxxxxxxxxxx
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492