Subject: [Last-Call] Genart last call review of draft-ietf-core-senml-data-ct-04

From: Christer Holmberg via Datatracker <[email-masked]>
Date: Mon, 06 Sep 2021 03:18:23 -0700

Reviewer: Christer Holmberg
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-core-senml-data-ct-04
Reviewer: Christer Holmberg
Review Date: 2021-09-06
IETF LC End Date: 2021-09-06
IESG Telechat date: Not scheduled for a telechat

Summary: I have reviewed the document. I have one technical comment, but the
rest is mostly editorial. Related to that, I do think the document could use
some editorial clean-up, e.g., when it comes to consistent terminology. I think
it is also good not to assume that the reader knows CoAP, and to make sure the
appropriate references/explanations are present when CoAP is referred to.

Major issues: N/A

Minor issues:

Q1 (TECHNICAL):

What happens if the receiver does not support the "ct" value? Is it a
server-error? If so, what response code is used? I think that should be
specified.

Nits/editorial comments:

Q2 (EDITORIAL):

The text should use consistent terminology. See below for a few examples:

The Abstract says:

   "The Sensor Measurement Lists (SenML) media type supports multiple
   types of values, from numbers to text strings and arbitrary binary
   data values.  In order to simplify processing of the data values,
   this document proposes to specify a new SenML field for indicating
   the Content-Format of the data."

First the text talks about types of values, and then suddenly the
Content-Format of the data.

Content-Format is the name of the new field - that is not what you are
indicating. You are using the new field to indicate something.

Also, "Content-Format" is also used by CoAP, so please check that it is clear
what is referred to whenever mentioned.

The text in Section 1 says:

   "To facilitate automatic interpretation it is useful to be able to
   indicate an Internet media type and content-coding right in the SenML
   Record."

...and, the test in Section 7 says:

"The indication of a media type in the data does not exempt a consuming
application from properly checking its inputs."

Now the text suddenly talks about "an Internet media type and content-coding",
when it earlier (in the Abstract) talked about value of type.

Q3 (EDITORIAL):

The text says:

"The CoAP Content-Format (Section 12.3 of [RFC7252]) provides just this
information"

I think it would be good with a little introduction on how CoAP is related to
all this.

Also "provides just this information" probably needs some re-wording.

Q4 (EDITORIAL):

Section 6 contains the ABNF for the new fields.

Is there a reason you don't define them in the same way as the basic field is
defined in RFC 8428 (there is no ABNF)?

Q5 (EDITORIAL):

The text in Section 7 says:

"The indication of a media type in the data does not exempt a consuming
application from properly checking its inputs."

I assume that by "its inputs" you refer to "received SenML data".

Shouldn't properly checking inputs be a generic CoAP security consideration?

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 15 Sep 2021 13:27:10 +0200

Hi Christer,

thank you for this review.

One interesting side note I have from reading the review is:
The CoRE WG, which produced this specification, is probably best known for maintaining the CoAP protocol, but it also has some other specifications, in particular with respect to data formats.

One example is the RFC 6690 link-format, which is well tied-in to the CoAP discovery mechanisms (RFC 7252 /.well-known/core, CoRE resource discovery).

SenML is the other major data format, with CoRAL coming up — both of these formats are really independent of CoAP, but of course have been designed to fit well with CoAP-based environments.

> server-error? If so, what response code is used? I think that should be

SenML is a data format; the specific protocol using which SenML data are passed around and how recipients of SenML would react in such a protocol to data they cannot process is not specified in SenML.  However, with respect to the extension point of adding new field names, Section 4.4 of RFC 8428 says:

  The SenML format can be extended with further custom fields.  […]   
  Implementations MUST ignore fields they don't recognize
  unless that field has a label name that ends with the "_" character,
  in which case an error MUST be generated.

So, as far as this can go while staying protocol-agnostic, the answer to this question is already fully specified in the base SenML document.

> Content-Format of the data.

Indeed, the abstract is a rather unfortunate contraction of the text in the introduction, which states explicitly that this document is about just one type of data, the “vd” field, i.e. binary data (see Section 4.2 of RFC 8428).

So we fixed the abstract:

OLD:
In order to simplify processing of the data values, this document proposes
to specify a new SenML field for indicating the Content-Format of the
data.
NEW:
In order to facilitate processing of binary data values, this document
specifies a pair of new SenML fields for indicating the
Content-Format of those binary data values, i.e., their Internet media
type including parameters as well as any Content-Coding applied.

Now in https://github.com/core-wg/senml-data-ct/commit/cc54f6b

> indicating. You are using the new field to indicate something.

The name of the new field is “ct” (or “bct” for the corresponding base field).  
It is used to indicate the Content-Format, a precise definition of which for the purposes of this document follows in Section 2.

> when it earlier (in the Abstract) talked about value of type.

Again, the abstract is excessively condensed here, see fix above.

> I think it would be good with a little introduction on how CoAP is related to

The abstract of RFC 8428 says:

…  A simple sensor, such as a
  temperature sensor, could use one of these media types in protocols
  such as HTTP or the Constrained Application Protocol (CoAP) to
  transport the measurements of the sensor or to be configured.

(talking about the SenML media types.)

> Also "provides just this information" probably needs some re-wording.

  To facilitate automatic interpretation it is useful to be able to
  indicate an Internet media type and content-coding right in the SenML
  Record.  The CoAP Content-Format (Section 12.3 of [RFC7252]) provides
  just this information; …

OK, we expanded this a bit into:

OLD:
Content-Format ({{Section 12.3 of -coap}}) provides just this
information; enclosing a Content-Format number (in this case number 60 as
defined for content-type application/cbor in {{-cbor}}) in the Record is
illustrated in {{ex-2}}. All registered CoAP Content-Formats are listed
NEW:
Content-Format ({{Section 12.3 of -coap}}) provides this
information in the form of a single unsigned integer; enclosing a Content-Format number (in this case number 60 as
defined for content-type application/cbor in {{-cbor}}) in the Record is
illustrated in {{ex-2}}. All registered CoAP Content-Formatn numbers are listed

Now in
https://github.com/core-wg/senml-data-ct/commit/09c105c
and
https://github.com/core-wg/senml-data-ct/commit/57e4713

> defined in RFC 8428 (there is no ABNF)?

Yes.  The field values for the fields in RFC 8428 have a rather simple structure (see Table 1 in Section 4.3); there was no need to provide ABNF.  A Content-Format-Spec can get complicated; there is no single standard that can be used to reference the ABNF for it from.
So we define it here.

> Shouldn't properly checking inputs be a generic CoAP security consideration?

It sure is, and this security consideration is maybe stating the obvious, but from the point of view of examining the change this spec brings to the security considerations of SenML, this is one of the significant items.

When stating the obvious, there is always a danger that this is misunderstood to mean something different, but I think the current text is innocuous here.

I have prepared a -05 on the basis of this review and the AD review; I’ll submit this once my computer has risen up from installing a mandatory security update...

Grüße, Carsten

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 16 Sep 2021 09:49:54 +0000

Hi,

I have removed the issues where I am ok, with no further comments, with the reply you gave :)

>SenML is the other major data format, with CoRAL coming up — both of these formats are really independent of CoAP, but of course have been designed to fit well with CoAP-based environments.

Sure, and there is nothing wrong with designing with some specific usage in mind.

But, I don't think the reader should have to know CoAP in order to understand SenML.

---

>> Summary: I have reviewed the document. I have one technical comment, 
>> but the rest is mostly editorial. Related to that, I do think the 
>> document could use some editorial clean-up, e.g., when it comes to 
>> consistent terminology. I think it is also good not to assume that the 
>> reader knows CoAP, and to make sure the appropriate references/explanations are present when CoAP is referred to.
>> 
>> Major issues: N/A
>> 
>> Minor issues:
>> 
>> Q1 (TECHNICAL):
>> 
>> server-error? If so, what response code is used? I think that should 
>> be specified.
>
> SenML is a data format; the specific protocol using which SenML data are passed around and how recipients of SenML would react in such a protocol to data they
> cannot process is not specified in SenML.  However, with respect to the extension point of adding new field names, Section 4.4 of RFC 8428 says:
>

So, that means that the sender of "ct", as without "ct", has no knowledge (unless obtained using some out of band mechanism) whether the receiver will be able to interpret the associated value correctly or not. Perhaps that would be good to point out.

---

> In order to simplify processing of the data values, this document proposes to specify a new SenML field for indicating the Content-Format of the data.
> NEW:
> In order to facilitate processing of binary data values, this document specifies a pair of new SenML fields for indicating the Content-Format of those binary data values, i.e., their Internet media type including parameters as well as any Content-Coding applied.
>

I don't think it is used to indicate the Content-Format. It might be used to indicate the "content format", though. No capital letters, and no dash :)

The same applies for Content-Coding. I think "content coding" would be more correct.

---

>> Q3 (EDITORIAL):
>> 
>> The text says:
>> 
>> "The CoAP Content-Format (Section 12.3 of [RFC7252]) provides just 
>> this information"
>> 
>> I think it would be good with a little introduction on how CoAP is 
>> related to all this.
>
> Content-Format ({{Section 12.3 of -coap}}) provides just this information; enclosing a Content-Format number (in this case number 60 as defined for content-type 
> application/cbor in {{-cbor}}) in the Record is illustrated in {{ex-2}}. All registered CoAP Content-Formats are listed
> NEW:
> Content-Format ({{Section 12.3 of -coap}}) provides this information in the form of a single unsigned integer; enclosing a Content-Format number (in this case number 60 as
> defined for content-type application/cbor in {{-cbor}}) in the Record is illustrated in {{ex-2}}. All registered CoAP Content-Formatn numbers are listed

Looks better.

Would it be useful to point out that CoAP Content-Format applies to the whole payload, while "ct" only applies to the "vb" element?

---

>> Is there a reason you don't define them in the same way as the basic 
>> field is defined in RFC 8428 (there is no ABNF)?
>
> Yes.  The field values for the fields in RFC 8428 have a rather simple structure (see Table 1 in Section 4.3); there was no need to provide ABNF.  
>A Content-Format-Spec can get complicated; there is no single standard that can be used to reference the ABNF for it from.
>So we define it here.

Would it be useful to point that out?

---

Regards,

Christer

---

From: Lars Eggert <[email-masked]>
Date: Mon, 20 Sep 2021 15:09:33 +0300

Christer, thank you for your review. I have entered a No Objection ballot for this document.

Lars

> On 2021-9-6, at 13:18, Christer Holmberg via Datatracker <[email-masked]> wrote:
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art