[sip-overload] AD review: draft-ietf-soc-overload-control-13

[Richard Barnes <[e-mail]>]

I have reviewed this draft, and there are several issues that I would like
to address before sending it to IETF LC.

MAJOR:

1. The fourth paragraph of Section 4.4 does not allow clients to detect
when overflow has happened.  It seems like there are two options: (1)
Define the timestamp so it cannot overflow (e.g., as a sufficiently long
timestap), or (2) define what an "appropriate base value" is so that a
client can recognize when the "oc-seq" has been reset there.  It seems like
it would be simplest to imply say that "oc-seq" MUST be an N-bit counter
value or a timestamp.  (Or better, choose one.)

2. "oc=0" seems like a bad way to signal that there's no overload going on.
 "oc-validity=0" does that.  And if there's no overload condition, on
algorithm is being applied, so no "oc" parameter value needs to be
supplied.  So it seems like you should signal support with no overload with
"oc;oc-validity=0".

3. As written, it seems like Section 5.9 could be read to require a
behavior that would exacerbate overload conditions with liveness checks.
 (For example, "periodically" with period 500ms.)  Suggest that this
section recommend a back-off algorithm, possibly with some concrete timing
paramters.  E.g., start at whatever interval you like, exponential back-off
to some floor (say 10sec).

4. The Security Considerations do a good job of describing the threats that
this mechanism introduces, but less so the mitigations to these threats.
 In particular, there is no mitigation provided for the multi-hop attack,
and the mitigation for a malicious client is not clearly stated.  Suggest:
-- Recommending that clients enforce a maximum validity period (e.g.,
3600s) in order to limit the scope of spoofing attacks (off-path or
multi-hop)
-- "Servers SHOULD monitor client behavior to determine whether they are
complying with overload control policies.  If a client is not conforming,
then the server SHOULD treat it as a non-supporting client (Section
5.10.2)."


MINOR:

1. The last paragraph of Section 2 is tautological.  ("The normative
statements...this specification.")  If an entity doesn't support this
specification, then of course the normative requirements don't apply!  Do
you mean something different from that?

2. In Section 4.2, it would help if you could say briefly why multiple
algorithms are needed.  Are some algorithms better suited to different
overload conditions?  Or could the group just not make up its mind?  (Maybe
there's a reference for this?)

3. Mandating that "loss" be included in the list of supported algorithms
seems duplicative.  Clients are already required to support "loss" and put
supported algorithms in the list.  It also seems undesirable for servers to
rely on "loss" being present, since that could cause problems if for some
reason we decide to deprecate "loss" in the future (cf. [[ SDP codec change
]])

4. It's not clear why the "oc-seq" parameter needs a fractional portion.
 Wouldn't it be simple just to make it an integer?

5. In Section 5.8, "frequent changes of overload control algorithm MUST be
avoided" -- This is not an RFC 2119 MUST.

6. In Section 5.8, "the algorithm MUST remain in effect" -> "the server
SHOULD NOT change the algorithm for that client"  It seems like you
wouldn't want to prevent the server from changing if it really needed to.

7. In Section 5.10.2, the recommendation at the end seems a little light,
and unclear how to implement.  You might recommend a simple algorithm to
apply here, e.g., just drop everything from non-supporting clients.

8. In Section 6.1, the last two paragraphs are completely redundant with
the general behavior specified above.

9. The example in Section 6.2 is not specific to the "loss" algorithm.  It
should be moved up to the top level.

10. Should Section 9 be moved to an appendix?

11. In Section 10, it's not really even necessary to use TLS to prevent
spoofing.  In many cases just using a connection-oriented protocol like TLS
or WS would be sufficient.


Thanks,
--Richard