Re: [Gen-art] Gen-ART LC Review of draft-ietf-opsec-dhcpv6-shield-04

[Fernando Gont <[e-mail]>]

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: [e-mail]
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492