Re: [core] Benjamin Kaduk's Discuss on draft-ietf-core-senml-14: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Tue, May 08, 2018 at 06:57:12PM +0000, Ari Keränen wrote:
> 
> 
> > On 8 May 2018, at 3.01, Benjamin Kaduk <[e-mail]> wrote:
> > 
> > On Mon, May 07, 2018 at 05:55:59PM +0000, Ari Keränen wrote:
> >> 
> >> We have now submitted a new revision of the SenML draft that addresses all the IESG review comments:
> >> https://tools.ietf.org/html/draft-ietf-core-senml-15
> >> 
> >> For answers to your review comments, please see below.
> > 
> > Thanks for the updates, they are definitely improvements!  A few
> > comments inline.
> 
> Thank you for your fast reply Ben! 
> 
> I'll answer now below (mostly) just on my behalf (it takes always a bit of extra time to get consensus across multiple authors in multiple time zones and with busy schedules).

Understood.

> >>> On 19 Apr 2018, at 5.33, Benjamin Kaduk <[e-mail]> wrote:
> >>> 
> >>> ----------------------------------------------------------------------
> >>> DISCUSS:
> >>> ----------------------------------------------------------------------
> >>> 
> >>> I agree with Ben's DISCUSS.
> >> 
> >> This was addressed in the updated security considerations text:
> >> https://github.com/core-wg/senml-spec/pull/106/commits/b911090ce2d12d394d243658f636522bb3dcc335
> >> 
> >> https://tools.ietf.org/html/draft-ietf-core-senml-15#section-13
> > 
> > I would of course like to hear from Ben as well, but it would
> > probably be good to say a bit more about what "proper security
> > mechanisms" are, perhaps that they are use "to provide data
> > integrity, confidentiality, and source authentication as appropriate
> > for the usage".
> 
> This sounds good to me. I made a simple PR for this:
> https://github.com/core-wg/senml-spec/pull/126/files
> 
> I added "e.g." to point out these are not the only considerations and Cullen also pointed out that this could be better without "source" in "source authentication" since also other authentication could be important. I agree with Cullen here; is that OK change to you too?

Yes, thanks.

> >>> Additionally, I have serious reservations about introducing the
> >>> concept of "base fields" that apply to subsequent array elemnets
> >>> unless overridden.  It seems to violate an abstraction barrier for
> >>> at least some of the serialization formats, and prevents snippets
> >>> from being composable and commutable absent the
> >>> resolution/normalization process.  It does not seem like the markup
> >>> language and the document contain suffient safeguards against misuse
> >>> to prevent security holes (both sensor data and commands could be
> >>> misinterpreted).  It seems that some substantially expanded text
> >>> should be added on the hazards of the non-resolved format and giving
> >>> guidance on when resolution/normalization must be performed in order
> >>> to avoid correctness and security issues.
> >> 
> >> The base values feature is something that the WG decided is important to have. We added now text about the security implications and remedies, also taking into account Alexey's follow-ups in this thread: https://github.com/core-wg/senml-spec/pull/109/files
> > 
> > The text about use outside of a Pack is probably the minimal change
> > needed on this issue (though of course I would not object if more
> > text was added, perhaps noting that "the smallest functional quantum
> > for non-resolved SenML Records is the Pack, that provides the needed
> > context for the Base Values").  But please use your judgment on what
> > volume of discussion is appropriate given the intended usage and
> > audience.
> 
> I think the last part of the updated (in -15) security section now essentially says that:
> 
>    SenML Records are intended to be interpreted in the context of any
>    applicable base values.  If records become separated from the record
>    that establishes the base values, the data will be useless or, worse,
>    wrong.  Care needs to be taken in keeping the integrity of a Pack
>    that contains unresolved SenML Records (see Section 4.6).
> 
> (https://tools.ietf.org/html/draft-ietf-core-senml-15#section-13)

Okay.  (Now I have forgotten if I actually said or only intended to
say, do you want to have a note about how when the streaming media
type is used, the underlying transport must be reliable?)

> > The existing text seems to be pretty clear that the base time and
> > (non-base) time are first added, and then the special semantics for
> > positive and non-positive values are applied.  Things could
> > potentially get exciting in some scenarios, though, as a collection
> > of Records (whether Pack or streaming) could cross over the
> > discontinuity.  Consider, for example, a small negative base time,
> > and a series of records with increasing time values, that cross the
> > zero threshold and suddenly appear decades in the past.  Do we need
> > to say more to warn about this possibility, or is it perhaps better
> > to only give the special semantics to one of the (base time, time)
> > values and not their sum?  (I guess it would have to be something a
> > little complicated like "if base time is present, it gets the
> > special semantics and time is added to it, but if base time is not
> > present then time gets special semantics".)
> 
> Good point with the discontinuity. It seems like worth pointing out the risk or making best use of it.
> 
> I'll have a look how to best address this.

Sure, I expect it to require some thought to figure out what's best.

-Benjamin