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

From: Alissa Cooper <[email-masked]>
Date: Tue, 7 Feb 2017 10:31:17 -0500

> On Jan 26, 2017, at 5:17 AM, Juergen Schoenwaelder <[email-masked]> 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 ...

> 
>>> I will replace queue with buffer for now.
>>> 
>>>> (5) I don't understand why the 'program' elements are included in the task configurations and capabilities. It doesn't seem wise to allow the controller to tell the MA which program it needs to use to complete a task, and I don't understand why the MA would need to communicate that information to the controller either. I thought the recent list discussion indicated that if an MA was not capable of performing an action, that action would simply fail, which seems like all that is needed.
>>> 
>>> There are multiple things:
>>> 
>>> (a) How do I tell which task I want to have executed? The information
>>>   model assumes that this can be done with the help of a registry.
>>>   The YANG data model, in addition, allows to use a simple 'program
>>>   name'. Note that this is a choice, i.e., you either use a registry
>>>   or a program name, but not both.
>>> 
>>>   The registry at the end is just some level of indirection - but
>>>   this indirection also requires to have tasks registered in a
>>>   registry. Right now, the implementation I know of only supports
>>>   program names.
>>> 
>>> (b) The task list in the capabilities branch serves as an inventory,
>>>   i.e., it tells the controller which tasks are supported by a given
>>>   implementation. The other task list defined options that are used
>>>   when a certain task is invoked (the Task Configuration in the
>>>   information model). If a controller configures a task that the
>>>   agent does support (i.e., it is not listed in the capability
>>>   tasks), it will not be executed.
>>> 
>>> Note that a capability task name 'traceroute' exposed by the LMAP
>>> agent does not necessarily mean that there is a program called
>>> traceroute at the operating system level. In fact, an implementation
>>> could choose to run traceroute internally without an explicit system
>>> level process (like RIPE Atlas did everything in a big event loop, not
>>> sure whether this is still the case).
>> 
>> Ok, I can understand that under circumstances where there is no registry, there needs to be some identifier to indicate which task to run, and which tasks the MA is capable of running. But the examples seem to imply that ‘program’ is the path and file name of the actual executable on the MA, which is what seems dangerous and unnecessary to me.
> 
> I can take out the path if that helps.

Yes.

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

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

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

Alissa

> 
> /js
> 
> -- 
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 8 Feb 2017 12:35:02 +0100

> I don’t think you need to say “somewhere.” The output is temporarily stored since the destination ...

OK
 
> > This is in my view what matters most.

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.";
 
> 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.

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

> > needed - and then I do whatever comes out of it.

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

---

From: "MORTON, ALFRED C (AL)" <[email-masked]>
Date: Wed, 8 Feb 2017 13:09:09 +0000

> -----Original Message-----
> From: lmap [mailto:[email-masked]] On Behalf Of Juergen
...
> 
> > > 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.
> > >
[ACM] 
The second sentence is a requirement on implementations, 
for the Controller to obey and for the MA to evaluate, so
s/must/MUST/
in the new text?  Or, "ought to" ?

Al

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 8 Feb 2017 14:15:02 +0100

> in the new text?  Or, "ought to" ?

I am fine with MUST but I always happily follow advice of IESG members
when it comes to RFC 2119 language. So will use MUST for now.

/js

---

From: Alissa Cooper <[email-masked]>
Date: Wed, 8 Feb 2017 08:59:51 -0500

> On Feb 8, 2017, at 8:15 AM, Juergen Schoenwaelder <[email-masked]> wrote:
> 

Agree that MUST is better here.

The rest of the changes look good to me.

Thanks,
Alissa

> Fax:   +49 421 200 3103         <http://www.jacobs-university.de/ <http://www.jacobs-university.de/>>