Subject: Re: [core] Roman Danyliw's Discuss on draft-ietf-core-senml-etch-05: (with DISCUSS and COMMENT)

From: Ari Keränen <[email-masked]>
Date: Sun, 3 Nov 2019 23:04:09 +0000

Hi Roman,

Thank you for your review! And apologies for late reply. Here's a PR to address your comments:
https://github.com/core-wg/senml-etch/pull/13/files

See comments and questions inline below.

Cheers,
Ari

> On 4. Sep 2019, at 21.45, Roman Danyliw via Datatracker <[email-masked]> wrote:
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> (1) Section 5.  It’s helpful that this document notes the relevance of SenML’s
> security and privacy considerations (i.e., Section 13 of RFC8428).  However,
> this references seems circular.  RFC8428 says, “SenML formats alone do not
> provide any security and instead rely on the protocol that carries them to
> provide security.”  This document seems to be that “protocol that carries them”
> implying it  should cover the security mechanisms.  Is it not appropriate to
> suggest that CoAPs can address the (per RFC8428) “confidentiality, data
> integrity, and authentication as appropriate for the usage.”? If not, what
> additional guidance can be provided?

Yes, secure CoAP would be addressing the security requirements in this case. I can make this explicit by adding to the first paragraph:

  CoAP's security mechanisms are used to provide security for the FETCH and (i)PATCH methods.

> (2) Section 5.  It seems like it would be appropriate for the server to support
> access control to restrict the client’s ability access and modify a resource
> with FETCH and (i)PATCH.  My read of “Per “In FETCH and (i)PATCH requests, the
> client can pass arbitrary names to the target resource for manipulation.  The
> resource implementer must take care to only allow access to names that are
> actually part of (or accessible through) the target resource”, doesn’t suggest
> that type of access control.

Yes, access control should be OK here. Do you have any suggestion how to make this more clear here? Note that each SenML name may or may not map to a separate resource (hence the wording "are actually part of (or accessible through) the target resource").

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> (3) Section 3.1.  A few questions about how general purpose of a query language
> is possible with the FETCH:
> 
> -- (To confirm based on the text “The SenML Records are selected by giving a
> set of names that …”) Does a name always have to be in the Fetch Request (i.e.,
> ‘at least one “n” MUST be in the Fetch Request’)?

Yes. I clarified this with:

A Fetch Pack MUST contain at least one Fetch Record. A Fetch Record MUST
contain a name and/or base name field(s).

> Using the third example in Section 5.1.2 of RFC8428 as a source:
> 
> -- The current text mentions that names and time can be in the Fetch Pack
> request.  Can other fields also be used as part of the criteria, say “v”?  For
> example, would ‘{“n”:” urn:dev:ow:10e2073a01080063”, “v”:”21.5”}’ return the
> three name+time records where the value is 21.5?

The intention was that only name and time could be used to select records; the last paragraph says that all others must be ignored. But looking at this closer I realized now that we probably need to add Unit to the selection criteria to support cases where same sensor name is used with multiple units for different types of measurements. 

But since this would be new functionality, I'll put this into a separate PR (needs likely more work):
https://github.com/core-wg/senml-etch/pull/14/files?utf8=%E2%9C%93&diff=split&w=1

> (4) Section 3.1. If there is no match for a Fetch, how is that signaled back in
> the response?  Is there any guidance to provide on error handling via CoAP
> error codes?

Good catch. Clarified this with:

If no SenML Records match, empty SenML Pack (i.e., array with no
elements) is returned as a response.

> (5) Editorial Nits:
> 
> -- Section 3.1.   s/When SenML Records contain also time values/When SenML
> Records also contain time values/

I realized this was not fully correct and rewrote that part into:
The SenML time field can be used in a Fetch Record to further narrow the
selection of matched SenML Records.

---

From: Roman Danyliw <[email-masked]>
Date: Tue, 5 Nov 2019 19:43:45 +0000

Hi Ari!

Thanks for the pull requests and explanations below.  My DISCUSS and COMMENTs are addressed by PR #13 and #14.  Details inline ...

> -----Original Message-----
> From: Ari Keränen <[email-masked]>
> Sent: Sunday, November 03, 2019 6:04 PM
> To: Roman Danyliw <[email-masked]>; The IESG <[email-masked]>
> Cc: [email-masked]; Carsten Bormann <[email-masked]>;
> [email-masked]; [email-masked]
> Subject: Re: Roman Danyliw's Discuss on draft-ietf-core-senml-etch-05: (with
> DISCUSS and COMMENT)
> 
> Hi Roman,
> 
> > (1) Section 5.  It’s helpful that this document notes the relevance of
> > SenML’s security and privacy considerations (i.e., Section 13 of
> > RFC8428).  However, this references seems circular.  RFC8428 says,
> > “SenML formats alone do not provide any security and instead rely on
> > the protocol that carries them to provide security.”  This document seems
> to be that “protocol that carries them”
> > implying it  should cover the security mechanisms.  Is it not
> > appropriate to suggest that CoAPs can address the (per RFC8428)
> > “confidentiality, data integrity, and authentication as appropriate
> > for the usage.”? If not, what additional guidance can be provided?
> 
> Yes, secure CoAP would be addressing the security requirements in this case.
> I can make this explicit by adding to the first paragraph:
>

Works for me.  Thank you.
 
> > (2) Section 5.  It seems like it would be appropriate for the server
> > to support access control to restrict the client’s ability access and
> > modify a resource with FETCH and (i)PATCH.  My read of “Per “In FETCH
> > and (i)PATCH requests, the client can pass arbitrary names to the
> > target resource for manipulation.  The resource implementer must take
> > care to only allow access to names that are actually part of (or
> > accessible through) the target resource”, doesn’t suggest that type of
> access control.
> 
> Yes, access control should be OK here. Do you have any suggestion how to
> make this more clear here? Note that each SenML name may or may not
> map to a separate resource (hence the wording "are actually part of (or
> accessible through) the target resource").

I was stumbling over the "are actually part of ..." phrase.  However, with your explanation, I understand now.  My confusion.  Consider this item resolved.

> > (3) Section 3.1.  A few questions about how general purpose of a query
> > language is possible with the FETCH:
> >
> > -- (To confirm based on the text “The SenML Records are selected by
> > giving a set of names that …”) Does a name always have to be in the
> > Fetch Request (i.e., ‘at least one “n” MUST be in the Fetch Request’)?
> 

Thank you.  This text makes it clearer.

> > -- The current text mentions that names and time can be in the Fetch
> > Pack request.  Can other fields also be used as part of the criteria,
> > say “v”?  For example, would ‘{“n”:” urn:dev:ow:10e2073a01080063”,
> > “v”:”21.5”}’ return the three name+time records where the value is 21.5?
> 
> The intention was that only name and time could be used to select records;
> the last paragraph says that all others must be ignored. But looking at this
> closer I realized now that we probably need to add Unit to the selection
> criteria to support cases where same sensor name is used with multiple units
> for different types of measurements.
> 
> https://github.com/core-wg/senml-
> etch/pull/14/files?utf8=%E2%9C%93&diff=split&w=1

OK.  No problem.  I wanted to highlight the issue.  What you have in the above PR is mostly there.  I leave it in your capable hands to polish.

> > (4) Section 3.1. If there is no match for a Fetch, how is that
> > signaled back in the response?  Is there any guidance to provide on
> > error handling via CoAP error codes?
> 

Thanks.

> selection of matched SenML Records.

Thanks.

Regards,
Roman

---

From: Ari Keränen <[email-masked]>
Date: Wed, 6 Nov 2019 20:43:41 +0000

Thank you Roman!

I merged now PR 13 so that it will be part of the next draft version. We'll still take a closer look at #14.

Cheers,
Ari

> On 5. Nov 2019, at 21.44, Roman Danyliw <[email-masked]> wrote:
> 
> Hi Ari!
> 
>> https://github.com/core-wg/senml-etch/pull/13
>> 
>> https://github.com/core-wg/senml-etch/pull/14
>