Re: [Dime] Gen-ART review of draft-ietf-dime-nat-control-07.txt

["Frank Brockners (fbrockne)" <[e-mail]>]

FYI. 
The updated draft-ietf-dime-nat-control-08 which I just posted should
also address the great set of comments that Miguel raised as part of his
Gen-ART review (thanks again Miguel for these great comments). 
Please find some additional notes inline...

> > ------ Forwarded Message
> > From: "Miguel A. Garcia" <[e-mail]>
> > Date: Fri, 4 Mar 2011 23:05:38 +0100
> > To: <[e-mail]>, Shwetha bhandari <[e-mail]>,
> > <[e-mail]>, <[e-mail]>, Jouni Korhonen
> > <[e-mail]>, Dan Romascanu <[e-mail]>
> > Cc: General Area Review Team <[e-mail]>
> > Subject: Gen-ART review of draft-ietf-dime-nat-control-07.txt
> >
> > I have been selected as the General Area Review Team (Gen-ART)
> > reviewer for this draft. For background on Gen-ART, please see the
> FAQ
> > at
> > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
> >
> > Please resolve these comments along with any other Last Call
comments
> > you may receive.
> >
> > Document: draft-ietf-dime-nat-control-07.txt
> > Reviewer: Miguel Garcia <[e-mail]>
> > Review Date: 2011-03-04
> > IETF LC End Date: 2011-03-08
> > IESG Telechat date:
> >
> > Summary: This draft is on the right track but has open issues,
> > described
> > in the review.
> >
> >
> > Major issues: none
> >
> > Minor issues:
> >
> > - The draft relies on the roles of a DNCA Manager and a DNCA Agent.
I
> > don't understand why these two new roles need to be introduce, when
> RFC
> > 3588 already provide some roles. In particular I don't see the
> > difference
> > between what you call the DNCA Manager and what RFC 3588 calls a
> > Diameter
> > Server; and I don't see the difference between your DNCA Agent and
> RFC
> > 3588 Diameter Client. Furthermore, I haven't seen a proper
definition
> > of
> > DNCA Manager and DNCA Agent, so, I have my own idea, which basically
> > points to the Diameter Sever and Client I just mentioned.
> >
> > The suggestion is to not invent the wheel twice and refer to the
> > terminology in RFC 3588, unless you can prove that this is
> > insufficient.

...FB: Following the live discussion at the last meeting - and the
confirmation of the result on the mailing list, the document now uses
the generic "Diameter peer" wording, and in case the peer is associated
with a specific network element, the terminology of "DNCA Diameter peer
within the NAT-device" or "DNCA Diameter peer within the NAT-controller"
is used.

> >
> > - When I was reading Section 4, which discusses the procedures, and
I
> > was
> > asking myself whether this application would use the ASR/ASA
> commands.
> > I
> > finished reading Section 4, and I found no reference to these
> commands,
> > and I thought there was an issue there. But then I found the first
> > reference to ASR/ASA in Section 5.3, and elsewhere in the document.
> So,
> > in order to avoid that the reader is as confused as me, I would
> suggest
> > to add a new section, which would be 4.6, and discusses "Session
> Abort"
> > like the rest of the procedures of the application.

...FB: We've added a new section 4.6 describing session abort.

> >
> > - On Section 6.2, the NCA command, I noticed that the
NC-Request-Type
> > AVP
> > is required in the message, because it is enclosed in curly brackets
> > {}.
> > It is not clear to me why this AVP should be mandatory in the answer
> (I
> > agree it should be mandatory in the request).

...FB: NC-Request-Type is now optional.

> >
> > - Also on Section 6.2, the NCA command, I don't see why the Result-
> Code
> > AVP is optional in an answer. I thought an answer should always
> report
> > a
> > result of any kind.

...FB: Result-Code is now mandatory.

> >
> > - On Section 7, page 23, I have a question on the state machine
> marked
> > as
> > "Open   Access Session end detected    Send STR     Discon". I don't
> > understand what is "Access Session end detected". It is not obvious
> > from
> > the rest of the document, so, I guess it should be explained.

...FB: We clarified the term "access session" in the document ("The
state 
   machine description uses the term "access session" to describe the 
   connectivity service offered to the endpoint or host.  
   "Access session" should not be confused with the
   Diameter session ID.")

> >
> > - On Section 7, page 23, there are two instances of the term
> "client".
> > Now I am not sure if this is the Diameter client (that I think you
> > should
> > be using throughout the document) or it is something else.

...FB: Client was referring to the endpoint/user in this context, so
   indeed was confusing. The doc is using more generic wording now

> >
> > - On Section 7, the DNCA Agent state machine (page 24). There is a
> line
> > that reads: "Open  NCR request received, and unable to provide
> > requested
> > NAT control service      Send failed NCA, Cleanup       Idle". So,
if
> I
> > understand correctly, the case is that under an Open state, if the
> DNCA
> > Agent receives an NCR request, but it is not able to provide the NAT
> > control service, so it sends back an NCA with a failed result. The
> > question is whether the NCR created a session or not. From the state
> > machine I understand it didn't create a session. So, I can conclude
> > that
> > a session is created when a successful NCA is sent. If this is the
> > case,
> > then look at Figure 5 on page 13, where there is a "Create session
> > state"
> > in between an NCR and an NCA message. If this NCA had failed, you
> would
> > still had a session created. So, somehow you need to make Figure 5
> > congruent with the state machine of the DNCA Agent. You can modify
> > figure
> > 5 (and the surrounding text) to indicate that the session is created
> > after a successful NCA (and not before), or you can modify the state
> > machine line I mentioned, to indicate that not only you should send
a
> > failed NCA, but also transition to a Disconn state in order to send
> an
> > ASR.

...FB: Figure 5 and the text got updated, stating that the DNCA Diameter
peer within the NAT-device creates session state only if it is able to
comply with the NCR and sends NCA with Result-Code set to
DIAMETER_SUCCESS. 

> >
> > - On Sections 8.2.2 and 8.2.3, the RESOURCE_FAILURE and the
> > UNKNOWN_BINDING_RULE_NAME have both exactly the same description.
The
> > only difference is that the former is a transient failure whereas
the
> > latter is a permanent failure. Honestly, I don't understand the
> > difference between both, and when I should generate one or the
other.
> I
> > would suggest that the latter adds a paragraph starting with "The
> > difference between this code and the RESOURCE_FAILURE is ..."
> >
> > Similarly, I don't see the difference between
> UNKNOWN_BINDING_RULE_NAME
> > and BINDING_FAILURE. I also suggest to clearly explain it or delete
> one
> > of them.

...FB: The two confusing definitions have been resolved to have only: 
  - UNKNOWN_BINDING_RULE_NAME
  - BINDING_FAILURE

> >
> > - Section 12, IANA considerations section. You need to mention the
> > registry and subregistry where you want IANA to operate. For
example:
> > This document instructs IANA to assign values to the [choose:
> > Commands/AVP Codes/etc] subregistry of the Authentication,
> > Authorization,
> > and Accounting (AAA) Parameters registry.
> >
> > Also, in Tables 1 to 4, under the "Reference" column, you don't need
> to
> > refer to a section in this draft, but just right "RFC XXXX", which
> > later
> > will be replaced by the RFC number of this draft.
> >
> > Additionally, the headings of each table needs to be congruent with
> the
> > registry, so, Table 2 should read "AVP Code        Attribute Name ",
> > and
> > so on. Take a look at the registry in
> > http://www.iana.org/assignments/aaa-parameters/aaa-parameters.xml
and
> > you
> > make some similar table.

...FB: We re-formatted the IANA section - to follow the format of RFC
3588
  (we kept some of the reference to sections in place - given that they
   might help some of the readers).

> >
> > - I have an unparseable sentence in Section 12. Please split it and
> > make
> > it understandable.
> >
> >                                                  To secure the
> >     information exchange between the authorizing entity (DNCA
> Manager)
> >     and the NAT device (DNCA Agent) requires bilateral
authentication
> > of
> >     the involved parties, authorization of the involved parties to
> >     perform the required procedures and functions, and procedures to
> >     ensure integrity and confidentiality of the information exchange
> > MAY
> >     be performed.

...FB: Indeed unparseable... - the entire security section (also thanks 
   to Matt's great comments) got extended and re-written.

> >
> >
> >
> >
> >
> > Nits:
> >
> > - Section 3.3, at the beginning of the section, add "server" as
> > follows:
> >
> > s/The role of the DNCA can be/The role of the DNCA server can be
> >                                          ^^^^^^
> >
> > - On Section 6.2, you have a duplicated "*[AVP]" towards the end of
> the
> > command.
> >
> > - On the title of Sections 8.4, 8.5, and 8.6 add "AVPs" as follows:
> > s/Reused from/Reused AVPs from"
> >
> > - On Figure 14 (page 29), towards the bottom, there is a reference
to
> > the
> > "V" bit. However, the "V" is never used in this figure, so, I
suggest
> > to
> > delete the sentence that describes the "V".
> >
> > - Section 8.7.1 under INITIAL_REQUEST, missing "a"
> > s/is used to install binding at/is used to install a binding at
> >
> > - First paragraph in Section 8.7 various missing articles and other
> > typos:
> > s/that references predefined policy/that reference a predefined
> policy
> > s/policy template on DNCA Agent/policy template on the DNCA Agent
> > s/contain static bindings, maximum number/contain static binding, a
> > maximum number
> > s/to be allowed, address pool/to be allowed, or an address pool
> > s/external binding address/external binding addresses

... FB: Thanks for catching those nits. They got addressed as well.

Thanks again for the very through and detailed comments.

Regards, Frank

> >
> >
> >
> >
> >
> > Regards,
> >
> >        Miguel G.
> > --
> > Miguel A. Garcia
> > [phone]
> > Ericsson Spain
> >
> > ------ End of Forwarded Message