Re: [lmap] AD evaluation: draft-ietf-lmap-yang-10

[Juergen Schoenwaelder <[e-mail]>]

On Tue, Feb 07, 2017 at 10:31:17AM -0500, Alissa Cooper wrote:
> 
> > On Jan 26, 2017, at 5:17 AM, Juergen Schoenwaelder <[e-mail]> wrote:
> > 
> > Trimming things down...
> > 
> > On Wed, Jan 25, 2017 at 01:25:26PM -0500, Alissa Cooper wrote:
> >>> 
> >>>> (4) "A queue is internally used to pass
> >>>>                 results to another schedule."
> >>>> 
> >>>> I thought it was up to the implementation to decide how to implement this?
> >>> 
> >>> OK. I meanwhile understand that the word 'queue' has too many
> >>> connotations. Is 'buffer' a less problematic term? The key here is
> >>> that the data producer and the data consumer are in general not
> >>> running at the time and hence data needs to be stored temporarily
> >>> somewhere.
> >> 
> >> Isn’t this implied though? Even with “buffer” it still seems like specifying an implementation detail. I’d prefer to see something like your last sentence above instead.
> > 
> > So I will replace 'buffer' with 'somewhere'. Oh boy. Here is the
> > new text:
> > 
> >               A set of schedules receiving the output produced
> >               by this action. The output is stored temporarily
> >               somewhere since the destination schedules will in
> >               general not be running when output is passed to
> >               them. The behaviour of an action passing data to
> >               its own schedule is implementation specific.
> 
> I don’t think you need to say “somewhere.” The output is temporarily stored since the destination ...
>

OK
 
> > Why would the controller necessarily even know where such executables reside on the file system? And I know there are a lot of things that could go wrong if a Controller gets compromised, but it just seems like making it so trivial for an MA implementation to literally just run the executable name specified by the Controller creates unnecessary risk.
> > 
> > Perhaps we need to add more explicit text to /tasks/task saying that a
> > configured LMAP task MUST resolve to a task listed in the capabilities.
> > This is in my view what matters most.
> 
> Agree.

Here is the new text:

      list task {
        key name;
	description
          "The list of tasks configured on the LMAP agent. Note
           that a configured task must resolve to a task listed
           in the capabilities. Attempts to execute a configured
           task that is not listed in the capabilities result in
           a runtime execution error.";
 
> >> If the task name on its own is not sufficient for the MA to be able to figure out which program is suitable to run, why not have the additional field defined as ‘program-name’ with some guidance about how to populate it, so that, e.g., what you end up with in that field is “mtr” or “fping” instead of "/usr/bin/mtr” or "/usr/bin/fping”?
> > 
> > I do not really see the logic here. An implementation that blindly
> > executes a program called 'mtr' by search a search PATH may actually
> > be worse than an implementation that executes /some/path/mtr.
> 
> But at least that leaves it up to the implementation to be implemented safely, rather than building an attack vector directly into the data model design.
>

I think the above proposed new text actually takes care of this.
Something not listed in the capabilities can't be executed.

> >>>> Nits and minor comments:
> >>> 
> >>>> (2) "Implementers MUST taken care that
> >>>>  option names and values are passed literally to programs.  In
> >>>>  particular, it MUST be avoided that any shell expansions are
> >>>>  performed that may alter the option names and values."
> >>>> 
> >>>> This text strikes me as a bit odd. Surely there are a whole selection of good programming practices that are necessary to ensure that things don't go haywire when implementing LMAP -- why call out these two with normative recommendations? Why does this guidance only apply to options? I would recommend having this text be non-normative, but if you do keep it I would suggest the following:
> >>>> 
> >>>> Implementers MUST take care that
> >>>>  option names and values are passed literally to programs.  In
> >>>>  particular, shell expansions that may alter the option names and values MUST NOT be performed.
> >>> 
> >>> I have no strong opinion on MUST vs must here and I usually happily
> >>> follow the advice of IESG members on RFC 2119 keywords. ;-)
> >> 
> >> Ok, my suggestion is to use non-normative “ought to” rather than 2119 MUST.
> > 
> > Seriously?
> 
> Yes, this comes up all the time in IESG eval and I would be willing to bet that another AD will comment on the use of lowercase must.

I do what you recommend.

> > I have left the 'must' in (there are other occurances of
> > must) and I leave it to the discretion of the IESG to discuss this if
> > needed - and then I do whatever comes out of it.
> 
> Ok.

There are now three lowercase 'must' left. So IESG members searching
for musts have something left to look at. ;-) If you want me to replace
some of them with 'ought to' or other phrases, let me know.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <http://www.jacobs-university.de/>