Subject: [AAA-WG]: Comments on Issue 100

From: Pat Calhoun <[email-masked]>
Date: Mon, 6 Aug 2001 00:51:19 -0700

Here are my comments on issue 100:

> In 3.0 
> - Commands flags, r(eserved): should state that an error must be
generated 
> if a packet is received with such a bit set to one. 

So my proposed text is:

	r(eserved)  - this flag bit is reserved for future use, and MUST
be
	set to zero, otherwise an error MUST be sent to the sender.

I thought about adding more details here, but it really seems
unnecessary. By looking at the Result-Code AVP, one can quickly
determine which error to use. 

> In 3.3: 
> - 1st paragraph: "Diameter commands typically start with an object
name". 
> Not that sure :-) 
> - 2nd paragraph: "The corresponding answer MUST contain either a
positive 
> or negative result code". What about multi-round trip messages, and
redirects?

Yes, I guess this section actually needed more of a re-write than
anything else. Here's my proposed text:

"  Diameter commands typically includes one or more English words
followed 
   by the verb Request or Answer. Each English word is delimited by a
hyphen.
   An acronym for both the request and answer is also normally provided.

   An example is a message set used to terminate a session. The command
name is
   Session-Terminate-Request and Session-Terminate-Answer, while the
acronyms
   Are STR and STA, respectively. 

   Both the request and the answer for a given command share the same
   command code. The request is identified by the R(equest) bit in the
   Diameter header set to one (1). The request is sent to a peer to
   request that a particular action be performed, such as authorizing a
   user or terminating a session. Once the receiver has completed the
   request it issues the corresponding answer, which includes a result
code
   that communicates one of the following:

      - The request was successful 
      - The request failed
      - An additional request must be sent to provide information the
        peer requires prior to returning a successful or failed answer.
      - The receiver could not process the request, but provides
        information about a Diameter peer that is able to satisfy the
        request, known as redirect.

   Additional information, encoded within AVPs, MAY also be included
   in answer messages."

> In 4.0: 
> - 1st paragraph: "Diameter AVPs carry specific authentication,
accounting 
> and authorization information, security information as well as 
> configuration details..." They can also carry routing information
(unless 
> that's what is meant by "configuration details".

My new proposed text reads:

"  Diameter AVPs carry specific authentication, accounting,
   authorization, routing and security information as well as 
   configuration details for the request and reply."

> In 4.1: 
> - AVP Code: "The first 256 AVP numbers are reserved for backward 
> compatibility with RADIUS". Even if Vendor-Id is non-zero? 

Proposed text:
"  The first 256 AVP numbers, with the Vendor-Id set to zero (0)
   are reserved for backward compatibility with RADIUS..."

> - AVP Flags, "Note that subsequent Diameter applications MAY define 
> additional bits within the AVP Header, and an unrecognized bit SHOULD
be 
> considered an error". Didn't find an error number for that one in
7.1.x. 

ok, here's the new result-code value:
      DIAMETER_INVALID_AVP_BITS          TBD
         A request was received that included an AVP whose flag bits
         are set to an unrecognized value, or that is inconsistent 
         with the AVP's definition.

> - AVP Flags, 2nd paragraph, "If an unrecognized AVP with the 'M' bit
set is 
> received by a Diameter node..." replace node with "client, server or 
> proxy". The client thing raises the issue stated in the 7.1.4/7.1.5
section 
> about errors in answers, though. Also, it should not be only if the
AVP is 
> unrecognized, but also if the requested AVP or value is recognized,
but the 
> node does not accept it. 

Check, so the new text would read:

"  The 'M' Bit, known as the Mandatory bit, indicates whether support of
   the AVP is required. If an AVP with the 'M' bit set is received by a
   Diameter client, server or proxy and either the AVP or its value is 
   unrecognized, the message MUST be rejected. Diameter Relay and 
   Redirector agents MUST NOT reject messages with unrecognized AVPs."

> - AVP Flags, 3rd paragraph, "In order to provide service to the user,
the 
> node at fault MUST re-issue...". The client might decide that if the
server 
> doesn't understand that AVP, it doesn't want to provide service to the

> user. Make that explicit rather than implicit. 

Ok, added the following text to the end of the paragraph:

"Alternatively, the node at fault MAY decide to simply not provide
service to the user."

> - AVP Flags, 4th paragraph, add a comma between "ABNF" and "is at
fault". 
> - AVP Flags, 5th paragraph, "AVPs with the 'M' bit cleared are 
> informational only and a receiver that receives a message with such an
AVP 
> that is not supported MAY simply ignore the AVP." The AVP might be 
> supported but not the value, and it could be ignored also, right? Same

> thing if the AVP and value are supported but the node does not want to

> honor that AVP or value.

Well, isn't ignoring an AVP also mean its value is ignored? I'm not sure
that I follow your logic here :(

> In 4.3, 
> - the definitions for Float32, 64 and 128 are truncated: "'V' bit is 
> enabled)." is missing.
Hmmm... an nroff bug. Fixed.

> In 4.4, 
> - 1st paragraph, "In addition to the AVP Base Data Formats" -> "In
addition 
> to *using* the AVP Base Data Formats". 

Why?

> - 1st paragraph, "New AVP Derived Data Formats MUST be registered with

> IANA." How could they be registered? There is no "unique identifier"
or 
> anything of the sort. And since the AVP contents are actually opaque
to 
> those that are not involved, I don't quite see the need. They should
just 
> be defined in any application that uses them. Also, if they must
indeed be 
> registered, a section in 11.x is missing. 

You are correct, and the two paragraphs have been modified to a single
Paragraph, which reads as:

"  In addition to the AVP Base Data Formats, applications may define
   data formats derived from the AVP Base Data Formats. An application 
   that defines new AVP Derived Data Formats MUST include them in a
   section entitled "AVP Derived Data Formats", using the same format
   as the definitions below. Each new definition must be either defined
or 
   listed with a reference to the RFC that defines the format."

> - IPAddress, what if an AVP must explicitly have an IPv4 or IPv6, and
not 
> "any of those"? 

Then I suspect that the actual AVP would need to state that clearly. We
don't
To my knowledge, have any such AVPs, though.

> - UTF8String, 1st paragraph, reference wrong [24] not [29] (it might
be a 
> good thing to actually check all refs) 

hmmm.... I thought I had at one point, but I guess another time around
before
I submit the next revision. Thanks for the heads up, though.

> - DiameterIdentity, 4th paragraph, "A Diameter node MAY advertise
different 
> identities...". Glurgs! Certainly not! It MUST be the same identity on
all 
> connections (unless the host is a gateway between two routing domains
(e.g. 
> private and public)). Otherwise there's no way to detect duplicate 
> connections, loops, etc. consistently. Remember, the DiameterIdentity,
as 
> used in CER/CEA and Route-Records is just considered as an opaque and 
> unique identifier. 

Damn, I knew making that Route-Record change was a bad idea, and would
bite
us in the ... later :( Ok, so in my case where I'm talking about two
identities
because of a public vs. private routing domain split, what should we do?

> - DiameterIdentity, 5th paragraph, this is not needed in duplicate 
> connection detection and loop avoidance, since the identifier is
opaque. It 
> can save time, however, when one gets an external reference to another
host 
> (via SLP, DNS, Redirect, Alternate-Host...) and one wants to check
that 
> this new host doesn't match one it already knows/already is connected
to. 
> But even if that's not done, a match would be detected upon connection
to 
> the new host when the CER/CEAs are exchanged and the "real" (unique) 
> identity is discovered and can be matched against existing
connections.

So you are stating that this paragraph can be deleted, and in no
circumstances will
we ever have a situation where a node will have to parse another peer's
identity?
 
> - IPFilterRule, 2nd to last paragraph, "An access device that is
unable to 
> interpret or apply a deny rule MUST terminate the session." Shouldn't
this 
> be dependent on whether the M bit is set or not? Ditto for the second 
> sentence (though it's probably not as important). 

Hmmmm..... this is a change that I feel uncomfortable just making as an
editorial change. What do other folks think?

> - QOSFilterRule: I thought the idea was to reference an IPFilterRule
to 
> determine what traffic should be marked or metered?

No, we decided that adding QoS to the IPFilterRule format would overload
that format.

> In 4.5: 
> - 1st paragraph, swap the ' and the . in 'Grouped.' 

Sorry, but I cannot for the life of me find the text you are referring
to :(

> - 3rd paragraph, "All AVPs included in a Grouped AVP Every Grouped AVP

> defined...". Either something is missing before the Every, or the all 
> before the Every is not needed. 

The latter

>
> In 4.5.1: 
> - in the paragraph starting with "The data for the optional AVPs...",
why 
> is is stated that "AVPs do not have to begin on 8 byte boundaries"? It
is 
> true, but I quite see why people would think it? 

Don't know why that sentence fragment was there, but it has been removed
since not only was it unnecessary, but it sure as hell was confusing.

> In 4.6: 
> - in the table, what if a flag is in neither column for a given AVP
(e.g. P 
> is often absent) 

There's another issue officially opened on this one, so I'll just ignore
this comment and address it in the formal issue, which is more complete.

> - here also, what about renumbering everything in a contiguous space? 

Sure. Next time I happen to be standing on my head for an extended
period of time. Seriously, it would be nice, but at this point I was to
minimize the changes to decrease the changes of introducing new bugs.
That would be one sure way to do it.

> - checking section references would be a good idea, but I'm way to
lazy for 
> that. 

I've done that recently, but I'll do it once more when I also check the
references.

> In 5.1: 
> - 1st paragraph, specify that this "per realm" rather than globally?
Though 
> there is the issue of proxies that serve many many realms (with
different 
> peers), and dynamically learned realms/peers. 

Paragraph changed to:

"  Although a Diameter node may have many possible peers that it is able
to
   communicate with, it may not be economical to have an established 
   connection to all of them. At a minimum, a Diameter node SHOULD have
   an established connection with a primary and secondary peer per
realm, 
   and MAY have additional connections, if it is deemed necessary. Note
that
   a given peer MAY act as a primary for a given realm, while acting as
a
   secondary for another realm."

> in 5.3: 
> - 5th paragraph, "Since the CER/CEA messages...", replace "an
upstream" 
> with "a". 

But that would be grammatically incorrect :(

> In 7.1.2: 
> - DIAMETER_LIMITED_SUCCESS: "When returned, the request was
successfully 
> completed, but additional processing is required by the application in

> order to provide service to the user." Gni? What is that? 

If you look at the Mobile IP application, it will be clear.
Unfortunately, it's rather
Difficult to describe here :( The basic idea is that the server says
yes, but you have
Additional application level stuff to do.

>
> In 7.1.3: 
> - DIAMETER_INVALID_ROUTE_RECORD is no longer needed. 

ok
> - DIAMETER_REDIRECT_INDICATION might be better off in the
informational types? 

Well, it requires local action, perhaps even by a relay (e.g. not the
NAS, but its
First hop agent).

>
> In 7.1.4/7.1.5: 
> - I'm still not quite sure of the distinction between permanent and 
> transient. e.g. DIAMETER_AUTHENTICATION_REJECTED [which is transient,
so 
> can be retried] means the request can be retried after asking the user
for 
> its password again. So it's not actually the same request, right? On
the 
> other hand, DIAMETER_AVP_UNSUPPORTED and DIAMETER_INVALID_AVP_VALUE
[which 
> are permanent, so shouldn't be retried] imply that you can retry by 
> removing the AVP or changing the value [that's actually the 
> "content-negociation" part of Diameter]. 
> Actually, we might be better off with some category that includes
anything 
> that means neither Accepted nor Refused, but rather "more to come"
(which 
> would include both the multi-round-trip auths and other capabilities 
> negociation messages). 
> Also, what if an error is detected on the answer rather than the
request? 
> Ditto for capabilities negociation in that direction (e.g. if server
says 
> "please set this feature to that value for that user" and client or
proxy 
> want to say "don't know what you're talking about/don't want to do
that").

Hmmm.... that's a whole lotta stuff, and I'm not sure exactly where you
are going with it :(

PatC

---

From: David Frascone <[email-masked]>
Date: Mon, 6 Aug 2001 09:00:36 -0500

Since the diameter port *could* get really strange traffic, I do several
other error checks in my diameter parser, that all return errors with the
error bit set:

 o Packet Length != Transport Reported Packet Length  -- Only works with SCTP.
 o Any reserved bit set.
 o AVP with reserved bit set.
 o AVP with Length > reported payload length.

When any of these things occur, I stop parsing immediately, and return an
error.

---

From: Jacques Caron <[email-masked]>
Date: Mon, 06 Aug 2001 23:21:02 +0200

>   An example is a message set used to terminate a session. The command
...*the* message set...
...command names are...

>    request that a particular action be performed, such as authorizing a

request -> ask? Not terrific, but the repetition is not terrific either...

>    user or terminating a session.

>that I follow your logic here :(

"...and a receiver that receives a message with such an AVP that is not 
supported *or the value of which is not supported* MAY simply..."

> > to *using* the AVP Base Data Formats".

Mmmm. Because the alternative is to define (verb), so it there should be a 
verb also? Really not important, though, just sounded better :-)

>us in the ... later :(

I don't quite see what Route-Record change you're talking about, but 
anyway, see the issue I posted this morning about 
DiameterIdentity/DiameterURI that should make things less confusing.

>we ever have a situation where a node will have to parse another peer's

See other issue.

>editorial change. What do other folks think?

I think we need server to client authorization AVPs negotiation. Issue 
coming up.

>No, we decided that adding QoS to the IPFilterRule format would overload

Right, but I meant:
- one defines several sets of traffic to match with multiple IPFilterRules
- then defines a QOSFilterRule that says "for traffic matching the first 
IPFilterRule, mark with this tag; for traffic matching the seconf 
IPFilterRule, mark with this other tag; etc.". Requires a way to reference 
an IPFilterRule though (unless Grouped-AVPs are used?).

>Sorry, but I cannot for the life of me find the text you are referring

1st line:
  The Diameter protocol allows AVP values of type 'Grouped.' This
->
  The Diameter protocol allows AVP values of type 'Grouped'. This

Sorry, years of proof-reading are surfacing :-/

>But that would be grammatically incorrect :(

Hu? Don't think so?
"...it is still possible that a proxy receives a message..."

>Hmmm.... that's a whole lotta stuff, and I'm not sure exactly where you

I'm going to open an issue with it :-(

Jacques.

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

---

From: Pat Calhoun <[email-masked]>
Date: Tue, 7 Aug 2001 02:42:34 -0700

> request -> ask? Not terrific, but the repetition is not terrific either...

what repetition? Is this text already covered?

> supported *or the value of which is not supported* MAY simply..."

check.

> DiameterIdentity/DiameterURI that should make things less confusing.

I meant how loop detection is now done.

> I think we need server to client authorization AVPs negotiation. Issue 

huh? This should be a very simple thing, and I don't see why any negotiation
is required. I guess I'll see your intentions in your issue filing :(

> an IPFilterRule though (unless Grouped-AVPs are used?).

That's not what we had decided to do, since the default value for processing
a packet that doesn't match an IPFilterRule set is for it to be dropped (in
some cases). That would have devestating effects if the QoSFilter was to
inherit this behavior. It seems like having this split it a much simpler 
approach, and will cause the least amount of confusion (but does admitedly
increase the document size).

> Sorry, years of proof-reading are surfacing :-/

check.

> "...it is still possible that a proxy receives a message..."

wait, you are asking me to change "... an upstream" to "...a upstream".
That is wrong.

Or did I misunderstand you?

PatC

---

From: Jacques Caron <[email-masked]>
Date: Tue, 07 Aug 2001 13:45:17 +0200

>what repetition? Is this text already covered?

"the request is sent to a peer to request..." :-(

>is required. I guess I'll see your intentions in your issue filing :(

Yep. Though I think I will file the issue first, and my solution later, 
since it seems awfully too complex :-(

>increase the document size).

Mmmm... I was thinking more like cisco(r)(tm) IOS(r)(tm)-type approach. 
Access-lists are used to classify packets into two categories: those that 
match (i.e. match a permit) and those that don't (i.e. match a deny or 
match nothing). Then depending on where the access-list is used, the two 
categories mean different things. If applied onto an interface directly, 
they mean "pass" and "drop" respectively. If applied within a QoS-type 
statement (e.g. rate-limit, or traffic-shape...), they mean "apply this QoS 
policy" and "don't do anything". Same thing for policy routing, etc.

>Or did I misunderstand you?

You did. Replace "an upstream" with "a". I.e. remove the upstream, it just 
adds confusion (it is not clear upstream from what that node is).

Jacques.