[Ecrit] Alexey Melnikov's Discuss on draft-ietf-ecrit-data-only-ea-21: (with DISCUSS and COMMENT)

[Alexey Melnikov via Datatracker <[e-mail]>]

Alexey Melnikov has entered the following ballot position for
draft-ietf-ecrit-data-only-ea-21: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ecrit-data-only-ea/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

(Updated, see the extra review comments)

I have a few a tiny point, but I think they need fixing for clarity:

1)
5.2.  The AlertMsg-Error Header Field

      error-code       = 1*3DIGIT

The text below makes it clear that the error-code is 3 digits:

   The ErrorValue contains a 3-digit error code indicating what was
   wrong with the alert in the request.

What you have above allows for 1, 2 or 3 digits. I think you meant:

      error-code       = 3DIGIT

2) As per Francesca's review: "/=" is illegal in ABNF, it should be "=/"


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I initially thought that error-codes are from the same namespace as SIP Status
Codes. I think you should explicitly say that they are not.

The first mention of HTTPS needs a normative reference.

In Section 10.1:

   Encoding considerations:  Uses XML, which can employ 8-bit
      characters, depending on the character encoding used.  See
      [RFC7303], Section 3.2.

This field of the form should say one of "7bit", "8bit", "binary" or "framed".
I think in your case it is "binary", because UTF-16 charsets are not prohibited
in your document.

**********************************************************************
* Note, that I am conducting an experiment when people aspiring to be*
* Area Directors get exposed to AD work ("AD shadowing experiment"). *
* As a part of this experiment they get to review documents on IESG  *
* telechats according to IESG Discuss criteria document and their    *
* comments get relayed pretty much verbatim to relevant editors/WGs. *
* As an AD I retain responsibility in defending their position when  *
* I agree with it.                                                   *
* Recipients of these reviews are encouraged to reply to me directly *
* about perceived successes or failures of this experiment.          *
**********************************************************************

The following comments were provided by Francesca Palombini
<[e-mail]>. Comments from me are marked with
[[Alexey:]].

Francesca would have balloted *DISCUSS* on this document. She wrote:

* Shepherd did not indicate any ABNF verification was done, I took some time to
check and:
        - message-header   /= AlertMsg-Error I am aware of =/ rule but /=?
        Don't think that is a valid ABNF op.

[[Alexey:]] well spotted.

        - Is error code 1 to 3 digits or 3 digits?

[[Alexey:]] This is the same issue as was reported by me.

        - "there MUST NOT be more than one string per error
         code." but ABNF specifies there can be 0 or more error-params which
         itself contain 1 error code text...

[[Alexey:]] While this can be fixed in ABNF, it is quite common in RFCs to
specify such restrictions in prose. So I don't think this issue is problematic.

*  Maybe not worth a discuss? But "AlertMsg-Error: 100 ; code="Cannot Process
the Alert Payload"" and following: these are error-code values not
AlertMsg-Error (as defined in the ABNF) values, I suggest changing the name so
that is clear (e.g. AlertMsg-Error-code: 100 ...)

* Security considerations need to discuss Call-Info header field usage, as that
is used, see 20.9 of RFC3261. I do not think the security considerations are
enough.

COMMENTS:

* Reference to SIP (RFC3261) is missing first time it appears in the text.
* "This scenario
   corresponds to the classic emergency services use case and the
   description in [RFC6881] is applicable." - I don’t find this sentence very
   clear. What description in RFC6881? Just needs rephrasing probably.
* I'd like either a note in terminology or a pointer to where Service URN is
described the first time it appears. * I would have liked more info about the
"author vs originator" terminology of section 4.2 * "then the <sender> element
MUST NOT contain
         the SIP URI of the user agent." MUST it contain author though? That is
         not specified.
* "the
      <expires> element is optional" Should it be OPTIONAL?
* "If the CAP message already contains an <area> element" seems to imply the
CAP message has been constructed elsewhere. Being more explicit about where
this CAP message is expected to be formed would make things clearer. * "The 425
response code is a rejection of the request due to its
   included alert content, indicating that it was malformed or not
   satisfactory for the recipient's purpose." it is unclear to me what that
   means of a request not to be "satisfactory". I think it needs to be better
   specified what can be wrong.
* "It is only appropriate to generate a 425 response when the responding
   entity has no other information in the request that is usable by the
   responder." like what other information? Again unclear imo.
* "; (message-header from 3261)" - missing ref
* Why in Fig3 is the sensor saying that it accepts these? Accept:
application/pidf+xml,application/EmergencyCallData.cap+xml * thanks for
checking XML *"An entity that has already received a CAP message within the
   indicated timeframe is able to detect a replayed message and, if the
   content of that message is unchanged, then no additional security
   vulnerability is created. " this mechanism needs to be specified in the
   document, while it is only hinted in the sec cons. If it is not in scope of
   this doc, that should be highlighted.
* It's media type not MIME type ;) (see
https://trac.ietf.org/trac/app/wiki/TypicalAppsAreaIssues#MediaTypes)