Subject: Comments on draft-calhoun-diameter-18.txt

From: David Frascone <[email-masked]>
Date: Fri, 12 Jan 2001 10:35:01 -0600

Ok, here are my comments.  Some are nits, others are what *I* think needs
to change.

---------

General:

	The difference between Request/Answer and Query/Reply was never
	defined.

Section 1.0 Introduction

     Editorial/Nit
	The "required features" bullets all have a strange voice.  They're 
	clauses, but not necessarily noun clauses.  The prepositions have
	to go.  For example, I'd change the last one to:
	
	- Exchanges resource usage information, which MAY be used for
	  accounting purposes, capacity planning, etc.

Section 2.1 Header Format

>   The base DIAMETER protocol is run over SCTP [26] port 1812.
>   Implementations MAY send packets from any source port, but MUST be
>   prepared to receive packets on port 1812. When a request is received,
>   in order to send a reply, the source and destination ports in the
>   reply are reversed. Note that the source and destination addresses
>   used in request and replies MAY be any of the peer's valid IP
>   addresses.

>   A given DIAMETER process SHOULD use the same port number to send all
>   messages to aid in identifying which process sent a given message.
>   More than one DIAMETER process MAY exist within a single host, so the
>   sender's port number is needed to discriminate them.

	Since the servers MUST run on port 1812, their can *not* be multiple
	servers on a single host.  So, I think the second paragraph should
	be deleted.

PCC:
	Since Diameter no longer runs over UDP, there is no reason to keep
	PCC for transport-layer RADIUS compatibility.

	I realize that to keep the words 16 bit (at least) aligned, Diameter
	will end up with 13 bits of flags, but I think that is better than
	wasting 8 on a PCC.

2.2.1 AVP Header

>   The AVP Format is shown below and MUST be sent in network byte order.

	Nit:  It's a run-on (or strangely worded) sentence.

	*Some* fields must be sent in network byte order (i.e. data may
	or may not be).  So, I think that part needs to be removed from
	the sentence, and stated in the individual fields that *are* sent
	in network byte order.

  AVP Length

	Nit: "Attribute" and "Invalid" don't need to be capitalized.

	Perhaps "invalid attribute length" needs to be defined (i.e.
	zero, less than the maximum header size, greater than the maximum
	message size, etc)

  AVP Flags

	"Home server" is used here, and has not been defined.  I think
	we need to either define "Home server", or use a different name.
        "Logical destination" is also used to refer to a "Home server"
        Perhaps this is a better term.

2.2.2 Optional Header Elements

>   The AVP Header consists of several optional fields. These fields are
>   only present if their respective bit-flags are enabled.

	change to:

	The AVP Header contains one optional field, the Vendor-ID.  This
	field is only present if the 'V' bit is set.

2.2.3 AVP Value Formats

	"Value field" and "Data field" are used interchangeably.  We should
	pick one.

3.3 Session-Timeout AVP

	The last line in the section does not need to be there.  The zero
	case was defined in the previous paragraph.

6.4.x Proxy-State

	The Proxy-Address AVP and Proxy-Info AVP both refer to section
	6.4.4 for descriptions of them.  There are no descriptions there.

7.1 Hop-by-Hop Security

	The phrase "similar to the method employed by the RADIUS protocol."
	should be removed.  The only similarity to RADIUS is that the
	data is encrypted by moving the MD5 hash around.  Since we're probably
	going to be using AES, I think this phrase is unnecessary.

7.1.2.1.1
	
	The payload hiding is not backward compatible with RADIUS.  (The
	encryption algorithm is, but since RADIUS wouldn't know what to
	do with the packet, I think this sentence is misleading)

Ok, I think that's it.  Sorry it's so long.  I guess I'm more opinionated
that I thought.

-Dave

---

From: Pat Calhoun <[email-masked]>
Date: Fri, 12 Jan 2001 14:28:48 -0800 (PST)

> 	The difference between Request/Answer and Query/Reply was never

It is specified in the framework document.

> 	  accounting purposes, capacity planning, etc.

Thanks, those bullet items did require a rework.

> 	servers on a single host.  So, I think the second paragraph should
hmmmm.... I am not sure about removing this one. Anyone have an issue with
removing it?

> 	will end up with 13 bits of flags, but I think that is better than

I would MUCH prefer make the version field be 8 bits and the flags field be 8
bits. Anyone else?

>         Perhaps this is a better term.

Home Server will be in the Terminology section of the new Framework document.

> 	going to be using AES, I think this phrase is unnecessary.

Well, the app level security, which makes use of shared secrets, are similar
to those in RADIUS. However, given that the authentication is much stronger
than RADIUS, it probably doesn't do DIAMETER justice. I will remove it.

Thanks for your comments!

PatC