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

[Roman Danyliw <[e-mail]>]

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 <[e-mail]>
> Sent: Sunday, November 03, 2019 6:04 PM
> To: Roman Danyliw <[e-mail]>; The IESG <[e-mail]>
> Cc: [e-mail]; Carsten Bormann <[e-mail]>;
> [e-mail]; [e-mail]
> Subject: Re: Roman Danyliw's Discuss on draft-ietf-core-senml-etch-05: (with
> DISCUSS and COMMENT)
> 
> 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
> <[e-mail]> 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.

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.

> 
> > ----------------------------------------------------------------------
> > 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).

Thank you.  This text makes it clearer.

> > 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

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?
> 
> Good catch. Clarified this with:
> 
> If no SenML Records match, empty SenML Pack (i.e., array with no
> elements) is returned as a response.

Thanks.

> > (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.

Thanks.

Regards,
Roman