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

[Ari Keränen <[e-mail]>]

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 <[e-mail]> wrote:
> 
> 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
>> 
>> 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
> 
> 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
>