Re: [savi] Working group last call on draft-ietf-savi-dhcp

["Jun Bi" <[e-mail]>]

Many thanks to Eric and Alberto’s commens.
We are revising the draft and will reply soon.

Best Regards,
Jun Bi

From: Alberto García 
Sent: Saturday, November 20, 2010 1:24 AM
To: 'Eric Levy-Abegnoli' ; 'Christian Vogt' 
Cc: 'SAVI Mailing List' ; 'Yao' ; [e-mail] ; [e-mail] ; 'Combes' 
Subject: Re: [savi] Working group last call on draft-ietf-savi-dhcp

Hi,

I have reviewed the document and, in general, I think the mechanisms described are sound and mature enough. 

 

However, I have a number of comments which I hope may help to improve document’s readability:

 

 

Introduction 

- The following sentence 

" In a stateless DHCP scenario [RFC3736], DHCP is used to configure

   other parameters but rather IP address. The address of the client

   SHOULD be bound based on other SAVI solutions, but rather this

   solution designed for stateful DHCP."

should be rephrased to show the motivation directly. I mean "This solution is designed to be used for scenarios in which stateful DHCP is deployed, so it is not applicable for the stateless DHCP scenario [RFC3736]."

 

Sec. 3. I think the text of Section 3 should be included into the introduction.

 

Sec. 4. If the terminology section does not define any terminology, I don't see the point in having such a section. I would remove the section and the text inside.

 

Sec. 5. I don't see the point in defining the two different conceptual structures. The whole information required is in the Binding State Table, and as 'conceptual structure' suggest, it is not intended to describe precisely any realization of the mechanism. So, although I assume that real implementations will probably have different structures for control and data planes, I don't see the point in showing this in the document. 

 

 

Sec. 6

- Second paragraph. s/"primarily designed for a pure DHCP scenario"/" primarily designed for a pure DHCP scenario"

- Second paragraph.  I would take out the sentence "In a

   mixed address assignment scenario where multiple address assignment

   methods such as DHCPv6 and SLAAC, or DHCPv4 and manually

   configured assign addresses that share the common prefix, the SAVI

   device may need additional state in the state machine to detect and

   avoid address conflict." 

 

- Fig 3. The scenario depicted in fig. 3 is too simple. A single SAVI element is included, while I assume multiple SAVI devices may coexist in the link. 

I don't see why the DCHP Server is connected to the same SAVI device to which the DHCP Relay is connected. One typical case is that the SAVI device is a switch, so I don't see why the DHCP Server and the Relay are in the same link. 

One or more routers should also be depicted, to provide a more detailed picture of the scenario.

 

In my opinion, section 6 (the scenario) should be before section 5, in which the data structures of the solution are presented. Moreover, the scenario should not be between two components of the solution (such as the data structures, and the rest of the solution).

 

Sec. 7

- This section is hard to understand. Here, a reference to the 'enforcement perimeter' concept presented in the framework document should be stated from the beginning (not in the 7.4 'SAVI-SAVI attribute' discussion), and also some homogeneity in the terms being used. In particular, I don't like the 'attribute' term for talking about the policy to apply. 

 

- Why is there a 'No attribute' attribute? I mean, why is not the SAVI-Validation case the default value, since it seems it provides the maximum security DHCP SAVI can provide? 

- Why is the separation between SAVI-DHCP-Trust and SAVI-SAVI? If you receive a DHCP message from other SAVI device, it should be snooped, isn’t it? If you receive a data packet (or any other packet than DHCP messages) from a SAVI-DCHP-Trust, it will always be forwarded, isn’t it? So why just not defining a single binding policy, with both cases (or alternative, clearly stating a case in which they cannot be the same?)

- Editorial comment: In general, I personally don't like sections (subsections in this case) which contain just one paragraph, and this section is full of them. I think everything could be more readable if the subsections were removed, and the text merged at the same level.

 

Sec. 8

- First paragraph. s/"on  control packet snooping"/"on DHCP message snooping"

 

8.1 Rationale. 

- I don't understand this. In the first paragraph, it says " if a node attached to a

   binding anchor intends to use a valid DHCP address"

what does 'a valid DHCP address' mean? That the node has been assigned the address?  That the address has been assigned to other node, and this tries to forge the address?

- I don’t understand also the following text: "the DHCP

   procedure which assigns the address to the node goes first on the

   same binding anchor."

- Then there is a consideration about the stability of forwarding at the link layer (I don't like very much the term routing for this). I think this consideration deviates the attention from the basic mechanism that you are about to describe to an event that should be discussed separately. I would remove all this stuff here.

 

8.2. 

- (weak suggestion:) I think the term INIT does not reflect the state you are describing. I think something like INVALID or NO_BIND should be more illustrative.      

- (weak suggestion:) The same for the term 'START'. Does not fully reflect what is happening. Maybe CHECKING...

- Is it being said that when the state is INIT, data packets are discarded? Not in this part, which I think it should. The same for START

 

8.3.2

- First paragraph. This is the first time the 'binding entry limit' concept is used. It should have been presented before.

- I feel that in general events are mixed with behaviors. Example: 

"EVE_DHCP_REPLY: A DHCPv4 Acknowledgement or DHCPv6 Reply message is

   received from a binding anchor with SAVI-DHCP-Trust attribute, and

   the message should be forwarded to a binding anchor with SAVI-

   Validation attribute, which has an entry in the state of START."

Here there are a list of conditions, and the results of this if there is a given state... I think that separating events from behaviors is difficult... and so I think that this event section could be removed, and the content be included into section 8.4, which describes the state machine. In addition, some behaviors depend on the initial state...

 

- what is the TID field (first appearing in In 'EVE_DHCP_REPLY')?

 

- It is not stated that EVE_DHCP_REPLY_RENEW must be received from a binding anchor with SAVI-DHCP-Trust attribute

 

8.4.1.1

Apart from describing the triggering events, the following paragraphs are about a security consideration, which I think it is not properly detailed (which is the risk, and which is the detail of the situation in which it occurs). Maybe this could be moved later, to a security consideration section, and the case be described more in depth.

 

8.5

s/NOT SUGGESTED/NOT RECOMMENDED

 

Sec. 9

- s/link topology change / link topology changes

- s/send packet  to a different port with the bound port/send packet through a different port to the bound port

- What is the meaning of the following sentence?: "In DHCP  scenario, till this document is finished, link topology change is the

   only two events that must be handled through this supplemental  binding process."

I count one event ('link topology change'), but besides, opposed to which situation? What is this for?

 

- "For implementations that will face the above problem:"   Which implementations may not face the problem stated above? I think any implementation may face it, isn’t it (it is different that someone decides or not to implement the mechanism, as stated later).

- "This function SHOULD be implemented if the vendor has such ability"  While I adhere to this recommendation in general, I think is too tautological, and not informative in this point. I don't understand the second part of this sentence (in which case do that... and why?) 

 

- From reading the text, it is not clear to me which are the conditions under which it is recommended (and if it is not recommended in the rest of the conditions...)

 

- I think the sentence "Other techniques may be prudently chosen as alternative if found to

   have equivalent or even better function to avoid permanently blocking

   after discussion, implementation and deployment."

Is dispensable. Of course other techniques can be used. However, they may impact negatively in other behavior of DHCP SAVI... In any case, as RFC 1958 states, "If there are several ways of doing the same thing, choose one", and you already have one.

 

9.1

- I question why each binding anchor need to be separately configurable to allow or not the recovery process (by setting or not the SAVI-BindRecovery attribute). I think this is too much detail and too complex. If this is to be settable, then one global switch should be enough.

- "1. If the address has a local conflict, meaning the DAD on the address fails,".  Who is doing the DAD? What does it mean 'DAD fails' (receiving or not a DAD_NADV)?

- s/ Denial of Services attack/Denial of Service attacks

- The text format for the alternatives for step 2 does not clearly indicates which are the parts inside this step 2.

- s/Two data based processes/ Two data-triggered recovery processes

- In "In case that the SAVI device is a pure layer-2 device, DHCP Confirm

   MAY be used to replace the DHCP LEASEQUERY. The security degree may

   degrade for the address may not be assigned by DHCP server."

Why is this for a pure layer-2 device? Who is issuing the message? Why this is a MAY (and not a SHOULD)...

 

- In the last two paragraphs some considerations are stated for 'LEASEQUERY'. Isn't this applicable also for DHCPLEASEQUERY?

 

- In general, I think this mechanism is under-specified: a state machine should be defined for it

 

9.2 

You have presented in 9.1 a mechanism to use any packet to try to recover the DHCP binding, and you said that was optional (in section 9). Now you present an alternative, that is mandatory, based on doing something similar to 9.1 but only for a subset of all the possible packets. 

Why the first is optional and this is mandatory? Isn't this more expensive in terms of computing (you must identify each of the specific message types)? Is there any security reason for doing this (I don't see it - any attacker would use 9.2 messages instead of data packets, if this enables any advantage for him). I think that 9.1 is easier and better than 9.2

Besides, you list some mechanisms, but each mechanism may involve many messages. Which are being used for this (anyone)? Why not Router Solicitation (I don’t see the criteria to select some and not other).

 

Sec. 11.

- I think 'short time' should be further detailed.

- "If the binding anchor turns on-link during the period, recover bindings." The meaning behind this sentence is assumed to be stated before in the sentence saying the stated should be kept. I would remove this.

- I don’t understand the security problem: " It may result in some security problem, e.g., a malicious

   node immediately associates with the binding anchor got off by a

   previous node, and then it can use the address assigned to the

   previous node."

 

Sec. 12.

- option number 2... how is different from number 1? Aren't all bindings associated with SAVI-Validating anchors?

- In option number 3, what does it mean the 'reverse indication' issue?

 

Sec. 13.

Wouldn’t be enough the recovery mechanism defined in section 9? With the mechanism of section 9, if a bridge reboots, hosts will send data, the switch will ask the DHCP server... and everything will recover seamlessly.

 

Sec. 14.

I think that the content of this section should be integrated into de main description of the state machine.

 

Sec. 16. 

The only case in which two bindings may exist is because the node has moved, isn`t it? Then I think that this should be better discussed in section 9, in which layer-2 mobility is discussed. (if there are more cases, they should be indicated)

 

Sec. 17.

A default value should be defined for BIND_RECOVERY_INTERVAL. Besides, there should be a discussion on which parameters this value depend (it is not very useful for an implementor to say that it just depends on the capacity of the device)

 

Sec. 18.

Wow. There are no security considerations! 

I think there are many security considerations to discuss (in fact, some of the sections before are in fact security considerations - binding number limitation, the fact that a binding is kept when a node moves from one point to the other).

A relevant security consideration should be around the security of DHCP. Is there any DHCP-specific security mechanism required/advised for DHCP operation? If not, then are there any considerations (mainly for IPv4) in order to prevent an attacker or a set of attackers to exhaust the number of available addresses (any suggestion of configuration of the maximum number of bindings, etc.)?

I also think a comprehensive summary of the remaining risks from deploying DHCP SAVI should be included here.

 

What happen if there are misconfigurations in the SAVI attributes? 

 

References

Many references should be moved from informative to normative (for example, DHCP references!, but some more).

 

 

Regards,

Alberto

 

 

 

De: [e-mail] [mailto:[e-mail]] En nombre de Eric Levy-Abegnoli
Enviado el: miércoles, 10 de noviembre de 2010 9:13
Para: Christian Vogt
CC: [e-mail]; Yao; SAVI Mailing List; Combes; [e-mail]
Asunto: Re: [savi] Working group last call on draft-ietf-savi-dhcp

 

Christian Vogt a écrit : 

SAVI folks, this is a working group last call for comments on the document "SAVI Solution for DHCP":     https://datatracker.ietf.org/doc/draft-ietf-savi-dhcp/ As discussed during our working group meeting yesterday, we believe that the document is now mature enough to take this next step.  Please read the document carefully, and comment on it on this mailing list by Friday, November 26, 2010. - Jean-Michel and Christian _______________________________________________savi mailing [e-mail]://www.ietf.org/mailman/listinfo/savi   My review below. I focussed on important issues as there are still a few in my opinion. 
Regards
Eric

Dear authors,
here are my comments

Comment #1

7.2. SAVI-Validation Attribute

   "SAVI-Validation attribute is used on binding anchor on which the source addresses are to be validated."

 

Couldn't you have a binding anchor with several addresses? For instance a “Link Local” and several “Global” addresses? The granularity of the attribute looks bogus to be, and it drives some confusion on section 8.3.2 (See comment #4) Maybe it is just a matter of clarification in the case you have several addresses behind one anchor. What happens if one address get allocated by the server (REPLY seen) and a second has not reach this stage yet (REQUEST seen)?  

 

Comment #2

Section 8.3.2: 

 

The section associates the arrival of a control message with an event in the state machine. For instance a REQUEST or a REPLY triggers a EVE_DHCP_REQUEST or a EVE_DHCP_REPLY respectively. In return an event is always associated with a single entry in the binding table (section 8.4.1.1. Trigger Event). However it may happen that more than one address (IADDR option) is present in the DHCP message, in particular in the REQUEST/REPLY. It should end up with several entries in the binding table, but this is never discussed. 

The whole text should be cleaned up with the consideration of several addresses in the message, for instance:

"The SAVI device MUST generate _a_ new entry in BST and FT. "  

should say "as many addresses as the number of IADDR found in the message".

 

Comment #2.1

 

One side effect (of previous comment) is that with several addresses, the message (REQUEST) could be received while the "binding entry limit on the binding anchor" has not been reached but will be reached before all addresses have been processed. This should be discussed as well, or else we will see multiple interpretations.

 

Comment #3

8.3.2. Control message arriving events

 

I can still see references to state LIVE and DETECTION.  They should be removed throughout the document.

 

Comment #4

8.3.2. Control message arriving events

This section uses the following terminology:

"EVE_DHCP_xxx: A DHCP xx message is received from a binding anchor with SAVI-Validation attribute, ..."

 

If there are several addresses behind an anchor, how does that work? Especially if they are in different state (can they be). Does the event affect all addresses at once? If not, the granularity of the event is bogus.   

 

 

Comment #5

Section 8.4.1.2. Following Actions

"If the triggering event is EVE_DHCP_REQUEST/EVE_DHCP_OPTION_RC:

   The SAVI device MUST forward the message.

"

 

In the mixed scenario, for IPv6 only, the source address (LL) of the REQUEST should be known –in the binding table- when the SAVI device receives the request, right? So, in the absence of a binding entry, a MUST here sounds unsecure. It is probably something that should be reviewed in the mixed scenario document (what to do if one method says drop and the other one says forward?) but it would be useful to refer it here, just because of the “MUST”.

I believe that is worded differently in section 10.2 (and more correctly).

 

Comment #6

Section 8.4.1.2. Following Actions

The section refers to a "lease time". But in DHCPv6, there is no such terminology. Instead, it uses the IPv6 terminology (preferred-lifetime and      valid-lifetime). You should refer to those and tell which one (presumably the latter) should be used.

 

 

Comment #7

Section 9 Supplemental Binding Process: Handling Link Topology Change

I find this section very hard to read. I just don't understand what is explained here, which is annoying because it has MUST and SHOULD. I believe it needs a rewrite.

 

Section 9.1 refers to baker-savi-one-implementation-approach, which is not a WG document, is expired, isn't this a problem? In addition, the section put MUST on implementation details ("a FIFO queue or register MUST be used to save recently filtered packets... SAVI device will fetch packet from the queue/register ...). As a vendor and as an implementer, I don't find the wording acceptable, and I don't see how this can be verified anyway. The text should stick with the functional aspects: if no binding is found, and the state is xxx, data packets will be used to this and that ...  You should leave the implementation details outside this document.

 

Comment #8

Section 9.2. Extended Control Packet Snooping Process

This section should be removed. Part of it belongs to the mix-scenario. And part is out of scope.  

 

 



--------------------------------------------------------------------------------
_______________________________________________
savi mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/savi