Diameter Applications Design Guidelines
RFC 7423

Note: This ballot was opened for revision 25 and is now closed.

(Benoît Claise) Yes

(Ted Lemon) (was Discuss) Yes

Comment (2014-08-01 for -25)
No email
send info
IANA has reviewed section 7, so I'm marking the DISCUSS as resolved.   All other comments still apply, of course.

In 4.2:
   It is unusual to delete an existing command from an application for
   the sake of deleting it or the functionality it represents.  This
   normally indicates of a flawed design.  An exception might be if the
   intent of the deletion is to create a newer variance of the same
   application that is somehow simpler than the application initially
   specified.

The second sentence seems to be making the same point as the third sentence: you are deleting a command because the original design is flawed.   By stating it this way you seem to be weakening the point.   Is that what was intended, or is the second sentence referring to the _new_ design?   If the latter, some wordsmithing here might improve the document.

In 4.3.1:

   o  Adding one or more optional AVPs and indicating (usually within
      descriptive text for the command) that at least one of them has to
      be present in the command.  This essentially circumventing the
      ABNF and is equivalent to adding a mandatory AVP to the command.

Isn't this a MUST, not a SHOULD?  In general, if the possibilities enumerated in this list are definitely going to cause interop problems, they should be MUSTs; if they are likely to cause interop problems, you should specify a test that indicates when violating the SHOULD is okay.   I don't think the test will be precise, but a little bit clearer guidance might be beneficial.

In 4.4.1, it might be helpful to talk about the case where a new application makes an AVP mandatory that is not mandatory in existing applications.   Suppose an AVP is received without the M bit set when it's mandatory for that application.   Is that an error?   Should the recipient reject the communication? 

In 5.2:
   As a general recommendation, commands SHOULD not be defined from

SHOULD NOT the NOT be capitalized, to be consistent with the SHOULD? :) I see that this occurs again several times in the document. Ordinarily SHOULD and NOT are capitalized consistently, whether it's "should not" or "SHOULD NOT" and not "SHOULD not." But this is a fairly minor nit.

   Defining a command with most of the AVPs indicated as
   optional MUST NOT be seen as a sub-optimal design introducing too
   much flexibility in the protocol.

It's a little weird to use normative language to tell people how to think about something.   The use of normative language in this document in general is a bit challenging, but I think appropriate because you're specifying best practices for achieving interoperability.   But in this case I think you should not use normative language.   I'd word it this way:

   Defining a command with most of the AVPs indicated as
   optional is good design in many cases, despite the flexibility
   it introduces in the protocol.

Or just restate it as you did, but as a fact rather than an admonition about how to think.

And then:

   The protocol designers SHOULD only
   clearly state the condition of presence of these AVPs and properly
   define the corresponding behaviour of the Diameter nodes when these
   AVPs are absent from the command.

How about:

   Protocol designers SHOULD
   clearly state the reasons why these AVPs might or
   might not be present and properly define the
   corresponding behavior of the Diameter nodes
   when these AVPs are absent from the command.

And:
      NOTE:  As a hint for protocol designers, it is not sufficient to just
      look at the command's CCF syntax specification.  It is also
      necessary to carefully read through the accompanying text in the
      specification.

This looks like a hint for implementors, not protocol designers.   I am confused.   Do you mean that protocol designers should, when defining the spec, include a recommendation that implementors read the accompanying text as well as the CCF specification?

In 5.5:
   Based on these considerations, protocol designers SHOULD carefully
   appraise whether the application currently defined relies on its own
   session management concept or whether the Session-Id defined in the
   Diameter base protocol would be used for correlation of messages
   related to the same session.  If not, the protocol designers MAY
   decide to define application commands without the Session-Id AVP.

This is stated in an overly detailed way, and "if not" is confusing, because the preceding statement contains two possibilities, neither of which is obviously negative.   It's clear on reflection what is meant, but it might be better stated something like this:

   Based on these considerations, protocol designers SHOULD carefully
   consider whether the application currently defined relies on its own
   session management concept.  If not, the protocol designers MAY
   decide to define application commands without the Session-Id AVP,
   and rely on the Session-Id defined in the
   Diameter base protocol for correlation of messages
   related to the same session.

Also:
   If any session management concept is supported by the application, the
   application documentation MUST clearly specify how the session is
   handled between client and server (as possibly Diameter agents in the
   path).

Was that supposed to be "and possibly" rather than "as possibly"?

In 5.6:
   If such extension is
   foreseen or cannot be avoided, it is RECOMMENED to rather define AVPs
   of type Unsigned32 or Unsigned64 in which the data field would
   contain an address space representing "values" that would have the
   same use of Enumerated values.

It is not obvious to me why this is so.   It seems as if using an unsigned integer with some set of known values in application A, and then adding some new values to the same AVP in application B, is no different than defining an enumeration in application A with some set of possible values, and extending the enumeration in application B with some additional values.

I am sure that there is a reason why you have made this distinction, and I could take a guess as to what the reason is, but my point is that you should really just state what the reason is clearly in this section, rather than simply stating your conclusion, because as written the section is really puzzling to me, a reader who is not steeped in the traditions of Diameter.

In section 5.7:
   Whatever the criteria used to establish the routing path of the
   request, the routing of the answer MUST follow the reverse path of
   the request, as described in [RFC6733], with the answer being sent to
   the source of the received request, using transaction states and hop-
   by-hop identifier matching.  In particular, this ensures that the
   Diameter Relay or Proxy agents in the request routing path will be
   able to release the transaction state upon receipt of the
   corresponding answer, avoiding unnecessary failover.  Application
   designers SHOULD NOT modify the answer-routing principles described
   in [RFC6733] when defining a new application.

This seems like something that could, if ignored, result in some bad behavior.   It also seems like something that's not clearly enforced by a mechanism that you've pointed out here.   So it would be good if the risk of abuse by clients responding to such messages could be discussed either here or in the security considerations section, and if there is some recommended mitigation strategy, that it be mentioned as well.   Possibly there is no risk, but that doesn't seem likely since if that were the case, this text would seem unnecessary.

Section 5.11 appears to be speaking to implementors.   The advice it gives is good, but is there an expectation that implementors will read this document?

In 6:
   In the case where the optional
   AVPs can be carried by any application, it SHOULD be sufficient to
   specify such a use case and perhaps provide specific examples of
   applications using them.

I can't see how SHOULD could be normative here, so perhaps it shouldn't be uppercase?

This is a great document.   Thanks for doing it!

(Jari Arkko) No Objection

Comment (2014-08-06 for -25)
No email
send info
Gen-ART review comments from Martin Thomson need to be acted upon; there was some discussion of them but it is not clear to me if all have been done by now. Can the authors or shepherds clarify the situation? Thanks.

(Richard Barnes) No Objection

Alissa Cooper No Objection

Comment (2014-08-05 for -25)
No email
send info
Thanks for putting this document together.

There are a lot of confusing uses of 2119 keywords in this document. Other ADs have commented on most of it, but there are a few instances where a requirement is put on application designers that seems like it would make more sense in lower case, e.g.:

4.3.1:
Application designers
   SHOULD consider the following questions when deciding about the M-bit
   for a new AVP:

(Compare with 4.1 - "application designers should consider the following:" -- why is one normative and not the other?)

5.5:
Based on these considerations, protocol designers SHOULD carefully
   appraise whether the application currently defined relies on its own
   session management concept or whether the Session-Id defined in the
   Diameter base protocol would be used for correlation of messages
   related to the same session.

6:
In the case where the optional
   AVPs can be carried by any application, it SHOULD be sufficient to
   specify such a use case and perhaps provide specific examples of
   applications using them.

Spencer Dawkins No Objection

(Adrian Farrel) No Objection

Comment (2014-08-07 for -25)
No email
send info
Please to a pass to check for unexplained abbreviations. I see:

AVP
EAP

---

Section 9

   Although such an extension may
   be related to a security functionality, the document does not
   explicitly give guidance on enhancing Diameter with respect to
   security.

I find that disappointing. Surely there are ways to extend Diameter that
would compromise security, and ways that extensions should be made in 
order to leverage existing security techniques. Surely there is 
information that could be shipped in Diameter that is confidential or
private and so advice is needed about how to protect that data if an
extension is made to exchange it.

I'm not about to require you to write new text in this section, nor will
I be suggesting text, but if you were to find the time and energy to 
write a few more words you would make an old AD very happy.

(Stephen Farrell) No Objection

Comment (2014-08-07 for -25)
No email
send info
- Hannes' affiliation was not NSN on the date of
publication. Makes one wonder if he saw this going by.

(Brian Haberman) No Objection

(Joel Jaeggli) No Objection

(Barry Leiba) No Objection

Comment (2014-08-06 for -25)
No email
send info
Ted gives some excellent comments; please address them.  In addition:

-- Section 4.3 --

   Although reuse of existing commands is still
   RECOMMENDED, protocol designers MAY consider defining a new command
   when it provides a solution more suitable than the twisting of an
   existing command's use and applications.

This is 2119 error number 1: SHOULD and MAY don't combine this way.  MAY specifies that something is entirely optional, and SHOULD does not.  If you want the 2119 "RECOMMENDED", I suggest you change the "MAY" to "can".

-- Section 5.10 --

   An application MAY define a new set of commands to
   carry application-specific accounting records but it is NOT
   RECOMMENDED to do so.

There's 2119 error #1 again, this time in reverse order.  If it's NOT RECOMMENDED, then it's not a "MAY".  Why say that you can do it, if it has a 2119 SHOULD NOT on it?  Just say, "Defining a new set of commands to carry application-specific accounting records is NOT RECOMMENDED."

-- Section 7 --

   The IANA AAA parameters page can be found at
   http://www.iana.org/assignments/aaa-parameters/aaa-parameters.xml

That's not how IANA wants us to use their URIs.  Please use this one instead:
http://www.iana.org/assignments/aaa-parameters

(Kathleen Moriarty) No Objection

Comment (2014-08-04 for -25)
No email
send info
The draft looks very good, thanks for your work on it!  I just have one suggestion:

The introduction should state what Diameter is and could use a sentence or two from RFC6733, such as the first sentence in the abstract (below) put into this draft's introduction.  There is no mention of what Diameter is in the draft and this would avoid confusion that there isn't another thing called diameter.   

   The Diameter base protocol is intended to provide an Authentication,
   Authorization, and Accounting (AAA) framework for applications such
   as network access or IP mobility in both local and roaming
   situations.


Thanks for addressing the SecDir Review:
http://www.ietf.org/mail-archive/web/secdir/current/msg04241.html

(Pete Resnick) No Objection

Comment (2014-08-06 for -25)
No email
send info
I think section 4.1 could use a good edit pass. I found several of the sentences rather convoluted. The worst seemed to me:

   The need for a
   new application is due to the fact that a Diameter node not upgraded
   to support the new application and therefore the new command will
   reject any unknown command with the protocol error
   DIAMETER_COMMAND_UNSUPPORTED and the transaction will fail.

I think you mean:

   A new application is required so that a Diameter node that has not
   been upgraded to support the new application, and therefore the new
   command, will reject any unknown command with the protocol error
   DIAMETER_COMMAND_UNSUPPORTED and the transaction will fail.
   
There are quite a few sentences that are complicated enough to cause confusion, or at least the need to re-read multiple times.

(BTW: Others have commented on fixing of some of the 2119 usage. The SHOULD in the second paragraph of 4.1 seems to be another instance.)

(Martin Stiemerling) No Objection

Comment (2014-08-05 for -25)
No email
send info
A few comments, for instance, that this document wasn't pushed through the id-nits tools which showed a number of RFC 2119 confusions, see at the end of this email. 

Further, what about the TBDs in Section 5.6.  "Use of Enumerated Type AVPs", i.e., listed with the values 1001 and 1002?

Miscellaneous warnings:
  ----------------------------------------------------------------------------

  == The document seems to use 'NOT RECOMMENDED' as an RFC 2119 keyword, but
     does not include the phrase in its RFC 2119 key words list.

  == Using lowercase 'not' together with uppercase 'MUST', 'SHALL', 'SHOULD',
     or 'RECOMMENDED' is not an accepted usage according to RFC 2119.  Please
     use uppercase 'NOT' together with RFC 2119 keywords (if that is what you
     mean).
     
     Found 'SHOULD not' in this paragraph:
     
     Additionally, application designers using
     Vendor-Specific-Application-Id AVP SHOULD not use the Vendor-Id AVP to
     further dissect or differentiate the vendor-specification Application Id.
     Diameter routing is not based on the Vendor-Id.  As such, the Vendor-Id
     SHOULD not be used as an additional input for routing or delivery of
     messages.  The Vendor-Id AVP is an informational AVP only and kept for
     backward compatibility reasons.

  == Using lowercase 'not' together with uppercase 'MUST', 'SHALL', 'SHOULD',
     or 'RECOMMENDED' is not an accepted usage according to RFC 2119.  Please
     use uppercase 'NOT' together with RFC 2119 keywords (if that is what you
     mean).
     
     Found 'SHOULD not' in this paragraph:
     
     As a general recommendation, commands SHOULD not be defined from
     scratch.  It is instead RECOMMENDED to re-use an existing command
     offering similar functionality and use it as a starting point.  Code
     re-use lead to a smaller implementation effort as well as reduce the need
     for testing.