[ovs-dev] OVN: Compromised Chassis Mitigation

[Lance Richardson]

> From: "Mickey Spiegel" <mickeys.dev at gmail.com>
> To: "Lance Richardson" <lrichard at redhat.com>
> Cc: "devovs" <dev at openvswitch.org>
> Sent: Thursday, March 9, 2017 6:49:53 PM
> Subject: Re: [ovs-dev] OVN: Compromised Chassis Mitigation
> 
> On Thu, Mar 9, 2017 at 8:52 AM, Lance Richardson <lrichard at redhat.com>
> wrote:
> 
> > > From: "Mickey Spiegel" <mickeys.dev at gmail.com>
> > > To: "Lance Richardson" <lrichard at redhat.com>
> > > Cc: "devovs" <dev at openvswitch.org>
> > > Sent: Wednesday, March 8, 2017 10:41:01 PM
> > > Subject: Re: [ovs-dev] OVN: Compromised Chassis Mitigation
> > >
> > > On Wed, Mar 8, 2017 at 1:28 PM, Lance Richardson <lrichard at redhat.com>
> > > wrote:
> > >

<snip>

> 
> > BTW, one other thought was to support expressions like those used for
> > "select"
> > operations to specify ACLs.  This is probably too heavy, but I do wonder if
> > code for handling "select" expressions could be reused here (would need to
> > extend it to allow expressions involving the per-session CN name).
> >
> 
> I like this idea. I do not think that it is too heavy.
> 
> Lance and I discussed this during the OVN meeting, coming up with the
> following observations and questions.
> 
> The "RBAC_Permissions" table would have another column:
> - "where", which specifies the conditions that must be satisfied in order
>   for this permission to take effect. These conditions apply to all
> operations
>   allowed by the permission, except for "insert".
> 
> One difference from the "where" condition for "select" operations is that
> the column value needs to be compared to the CN name rather than a
> specified constant value.
> 
> Another question is whether we need to support different "where"
> conditions for different columns or different operations on the same
> table?
> If so, would we allow one role to refer to multiple "RBAC_Permissions"
> with the same value of "table"?
> This would require the addition of error handling code for the case
> where the same operation or the same column is specified in different
> permissions referred to by the same role.
> 
> Looking at the syntax for the "where" condition for "select" operations
> after the meeting, I noticed that it uses a JSON array. However,
> scanning the code, it looks to me like the JSON array syntax is not
> supported by the OVSDB IDL, so this syntax could not be used directly
> by "RBAC_Permissions"?
> Does that mean that the code in ovsdb/condition.c could not be reused
> for this purpose?
> 
> If we are OK with just one "where" clause in each permission, then we
> could just add "where_column" and "where_function". However, if we
> want to allow multiple clauses in a condition, then we would need
> something more complex. One simple but somewhat ugly approach is
> to put column name in the "key" and function in the "value".
> 
> A more complex approach is to go with a full expression syntax. A few
> design issues arise when thinking about how to define this syntax:
> - Is a string specifying the column name good enough, or would we also
>   need to support column_name:key?
>   e.g. to evaluate "options:chassis" in "Port_Binding"?
> - Is there anything other than CN name that we would compare to?
>   Do we need a corresponding symbols table?
> - Do we allow constants?
> 
> Mickey
> 
> 
> 
> >
> > I think we're getting closer to a good solution... thanks again!
> >
> > Regards,
> >
> >     Lance
> >
> 

In thinking about how to resolve some of the points above, (still planning
to respond), I wondered if the original proposal could be improved on. How
about this:

Use a single transaction ACL table, implemented as follows:

   There is one row for each table in the schema that can be modified by
   ovn-controller. Each row contains:

      - A "table" column of type "string", which is used as the index for this
        table and contains the name of a table.

      - An "authorization" column containing a set of "string" type, where
        each string is the name of a column (or column:key) that must contain
        the ID of client attempting the transaction (CN field from client
        certificate). If this set is empty, all IDs are considered to be
        authorized.  If this set contains more than one string, at least one
        must contain the client ID in order to be considered authorized. 

      - An "insert_delete" column of type boolean. If true, insertions
        are allowed by any client and deletions are allowed for rows
        meeting the authorization requirement.

      - An "update" column of type "set of strings". Each string is the
        name of a column (or column:key) for which modification is allowed
        in rows meeting the authorization requirement.

For the current implementation of the OVN_Southbound schema, the ACL 
table would contain the following rows:

   "Chassis":
      authorization: "chassis"
      insert_delete: "true"
      update:        "nb_cfg", "external_ids", "encaps", "vtep_logical_switches"
                     Modification of these columns is allowed for rows which in
                     which the authorization check passes.

   "Encap":
      authorization: "chassis"  New column containing CN ID of row creator.
      insert_delete: "true"
      update:        "type", "options", "ip"

   "Port_Binding":
      authorization: "" All chassis are considered to be authorized. In a recent 
                        live migration proposal by Russell Bryant, this column
                        would contain "options:chassis" and "options:migration-destination".
      insert_delete: "false"
      update:        "chassis"

   "MAC_Binding":
      authorization: "" All chassis are allowed to update/delete any row. The long-
                        term plan would be to eliminate this table.
      insert_delete:  "true"
      update:         "logical_port", "ip", "mac", "datapath"