[ovs-dev] OVN: Compromised Chassis Mitigation

Lance Richardson lrichard at redhat.com
Thu Mar 9 16:52:48 UTC 2017


> 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:
> 
> > This email (prompted by recent discussions in IRC on the subject)
> > outlines some of the options that have been discussed for securing
> > OVN_Southbound from a compromised chassis, and includes a strawman
> > proposal for an ovsdb transaction ACL implementation.
> >
> > Feedback appreciated, hopefully we can discuss in IRC tomorrow.
> >
> 
> Thanks for the proposal. Some comments and an alternative
> approach to option 3 near the bottom.

Hi Mickey,

Thanks for taking a look! More below...

> 
> I should also note that in addition to the control plane issues
> due to compromised chassis described here, there is also a
> data plane issue with respect to tunnels, or at least there was
> a couple of months ago. When a geneve tunnel packet is
> received without the geneve options field, this results in a
> cached flow that drops all packets on the tunnel, even
> subsequent packets with a valid geneve options field. I peeked
> at the OVS flow creation code for tunnels, but it was not
> immediately obvious whether there is a problem in the OVS
> code, or we just need to amend OVN coding of table 0 to
> check for presence of the geneve options field.
> 
> 
> > Regards,
> >
> >    Lance Richardson
> >
> >
> > Problem Description
> > -------------------
> > Each ovn-controller instance currently has full write access to the OVN
> > southbound database.  This means that a single compromised chassis can
> > potentially disrupt every chassis in an OVN network.
> >
> > Goals of Solution
> > -------------------
> > Limiting the potential damage that can be inflicted on an OVN network by
> > a compromised chassis will mean restricting the set of objects in
> > OVN_Southbound that can be modified by a chassis to a minimum.  In the
> > current implementation, there are a number of tables that do not need
> > to be modified by ovn-controller:
> >     SB_Global
> >     Address_Set
> >     Logical_Flow
> >     Multicast_Group
> >     Datapath_Binding
> >     DHCP_Options
> >     DHCPv6_Options
> >     Connection
> >     SSL
> >
> > Tables that do need to be modified by ovn-controller include:
> >     Chassis
> >        Rows in this table are added and updated by ovn-controller.
> >        While there have been proposals to make this table read-
> >        only for ovn-controller, note that the nb_cfg column still needs
> >        to be updated by the associated chassis in order for the
> >        "--wait=hv" option of ovn-nbctl to work.
> >
> >     Encap
> >        Rows in this table are added/deleted/modified by ovn-controller.
> >
> >     Port_Binding
> >        Rows in this table are added/deleted/modified by ovn-northd, with
> >        the exception of the "chassis" column, which is updated by
> >        ovn-controller.
> >
> >     MAC_Binding
> >        Rows in this table are inserted/deleted/modified by all chassis.
> >
> > Possible Solutions
> > -------------------
> > Several possible implementations have been proposed on the ovs-dev mailing
> > list and/or discussed in IRC, including:
> >
> >   1) Eliminate the need for writes to the southbound database by ovn-
> >      controller, adding new mechanisms for managing tables that are
> >      currently written to by ovn-controller.
> >
> >   2) Enhance ovsdb-server to support role-based (or id-based) access
> >      control mechanisms, and use these mechanisms to restrict write
> >      access to the southbound database by ovn-controller.
> >
> >   3) Eliminate all write access to the southbound database by
> > ovn-controller,
> >      adding a "trusted proxy" through which ovn-controller can request
> >      updates to the southbound database subject to ovn-specific access
> >      policies.
> >
> > (1) Eliminate Need for Writes to SB DB by ovn-controller
> > --------------------------------------------------------
> > Some work has been done for the implementation of option (1), however
> > at least two cases appear to be unsolvable:
> >
> >     - The "nb_cfg" column in the Chassis table needs to be updated by
> >       ovn-controller in order for the "--wait=hv" option of ovn-nbctl to
> >       work.
> >     - The "chassis" column of the Port_Binding table needs to be updated
> >       by ovn-controller in order for the port state "up" column in the
> >       northbound database Logical_Switch_Port table to be maintained
> >       correctly by ovn-northd.
> >
> > Because of these issues, option (1) appears to be non-viable at this
> > time.
> >
> > Related discussions and patches:
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/310825.html
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-October/324090.html
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325010.html
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325220.html
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326104.html
> >
> > (2) Introduce "Trusted Agent" for Writes to SB DB
> > -------------------------------------------------
> > The "trusted agent" solution would involve:
> >    1) Configure southbound ovsdb-server allow only read-only connections
> >       from ovn-controller.
> >    2) Add a new "trusted agent" daemon (stand-alone or integrated with
> >       ovn-northd) which would handle write requests from ovn-controller
> >       (these requests would be in json-rpc form, reusing existing code),
> >       decide whether the request is allowed, and update the southbound
> >       database appropriately.
> >    3) The "trusted agent" will determine the identity (chassis name) for
> >       each established session using SSL certificate CN field (as
> >       described for option (3)).
> >
> > Some possible issues with this approach:
> >   - If there is delay in the handling of the write requests by the
> >     "trusted agent", will ovn-controller waste cycles attempting
> >     to retry the transaction in the IDL loop? Should write requests
> >     block until the southbound db has been updated?
> >
> > Related discussions:
> > https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326096.html
> >
> > (3) Add General-Purpose Transaction ACL Support to ovsdb-server
> > -----------------------------------------------------------
> > Option (3), enhancing ovsdb-server to support role-based or id-based
> > access controls, would entail the following (strawman):
> >
> >    1) Connections between sb ovsdb-server and ovn-controller are read-
> >       only, with pssl: type. ACLs enable write access to specific
> >       database entries.
> >
> >    2) The introduction of "transaction ACL" support to ovsdb-server.
> >       Transaction ACLs are implemented using a "root" ACL table,
> >       indexed by table name, and multiple "leaf" ACL tables, indexed
> >       by chassis name, one for each table in the OVN_Southbound schema
> >       that might be written to by an ovn-controller.
> >
> >    3) The addition of a command-line option to specify that transaction
> >       ACLs are to be used and to identify the root ACL table by name.
> >
> >    4) The leaf ACL tables are indexed by chassis name, which is obtained
> >       for each SSL connection via the CN (Common Name) field in the
> >       client SSL certificate. This requires chassis SSL certificates
> >       to be generated for each chassis with CN field set to chassis name.
> >
> >    5) The root and leaf transaction ACL tables are managed by ovn-northd.
> >
> > The root transaction ACL table is implemented as follows:
> >
> >    1) One row for each table in the SB schema that can be modified by
> >       ovn-controller. It is indexed by table name, and each row contains:
> >
> >       - A column of type boolean indicating whether insertions to the
> >         associated table are allowed (if allowed, they are allowed for
> >         any chassis).
> >
> >       - A column of type boolean indicating whether deletions from the
> >         associated table are allowed for all chassis (if false, deletions
> >         may be allowed by the leaf ACL table, see below).
> >
> >       - A column of type boolean indicating whether modifications to the
> >         associated table are allowed for all chassis (if false,
> >         modifications may be allowed by the leaf ACL table, see below).
> >
> >       - A column containing a table reference to the "leaf" ACL table
> >         containing ACLs for the associated OVN_Southbound table.
> >
> > For the current implementation of the OVN_Southbound schema, the root
> > ACL table would contain the following rows:
> >
> >    "Chassis":
> >       insert:         true
> >       delete_any:     false
> >       update_any:     false
> >       leaf_acl_table: Chassis_ACL (table ref)
> >
> >    "Encap":
> >       insert:         true
> >       delete_any:     false
> >       update_any:     false
> >       leaf_acl_table: Encap_ACL
> >
> >    "Port_Binding":
> >       insert:         false
> >       delete_any:     false
> >       update_any:     false
> >       leaf_acl_table: Port_Binding_ACL
> >
> >    "MAC_Binding":
> >       insert:         true
> >       delete_any:     true
> >       update_any:     true
> >       leaf_acl_table: <empty>
> >
> > Notes:
> >    - Encap table will need a new column containing the name of
> >      the creating chassis in order to allow delete/update access
> >      controls.
> >    - Assuming MAC_Binding table will be removed in the future.
> >
> > Each leaf ACL table contains one row per chassis (indexed by chassis
> > name), each row contains the following columns:
> >    chassis_name: index, chassis name
> >    delete:       0 or more UUIDs of rows in associated table that
> >                  can be deleted by this chassis.
> >    One column for each column in associated column that can possibly
> >    be written by ovn-controller. Column name is identical to column
> >    name in associated table.
> >
> > The Chassis_ACL leaf ACL table would contain (assuming a configuration
> > with two chassis named "chassis1" and "chassis2"):
> >
> >    chassis_name: "chassis1"
> >       delete:                UUID of chassis1 row in Chassis table
> >       nb_cfg:                UUID of chassis1 row in Chassis table
> >       external_ids:          UUID of chassis1 row in Chassis table
> >       encaps:                UUID of chassis1 row in Chassis table
> >       vtep_logical_switches: UUID of chassis1 row in Chassis table
> >
> >    chassis_name: "chassis2"
> >       delete:                UUID of chassis2 row in Chassis table
> >       nb_cfg:                UUID of chassis2 row in Chassis table
> >       external_ids:          UUID of chassis2 row in Chassis table
> >       encaps:                UUID of chassis2 row in Chassis table
> >       vtep_logical_switches: UUID of chassis2 row in Chassis table
> >
> > The Encap_ACL leaf ACL table would contain:
> >
> >    chassis_name: "chassis1"
> >       delete:    UUIDs of rows created by chassis1 in Encap table
> >       type:      UUIDs of rows created by chassis1 in Encap table
> >       options:   UUIDs of rows created by chassis1 in Encap table
> >       ip:        UUIDs of rows created by chassis1 in Encap table
> >
> >    chassis_name: "chassis2"
> >       delete:    UUIDs of rows created by chassis2 in Encap table
> >       type:      UUIDs of rows created by chassis2 in Encap table
> >       options:   UUIDs of rows created by chassis2 in Encap table
> >       ip:        UUIDs of rows created by chassis2 in Encap table
> >
> > The Port_Binding_ACL leaf ACL table would contain:
> >
> >    chassis_name: "chassis1"
> >       delete:    <empty>
> >       chassis:   UUIDs of all rows in Port_Binding table
> >
> >    chassis_name: "chassis2"
> >       delete:    <empty>
> >       chassis:   UUIDs of all rows in Port_Binding table.
> >
> > Note that having all rows of Port_Binding_ACL contain all Port_Binding
> > UUIDs is unlikely to scale well.  Access to the Port_Binding table could
> > be further restricted while eliminating this scaling problem as follows:
> >
> >    1) Introduce a new option "chassis" on Logical_Switch_port in
> >       the northbound DB.
> >    2) Have ovn-northd copy the "chassis" option from Logical_Switch_Port
> >       in OVN_Northbound to the corresponding Port_Binding in
> >       OVN_Southbound.
> >    3) Change Port_Binding_ACL leaf table setup as follows:
> >
> >    chassis_name: "chassis1"
> >       delete:    <empty>
> >       chassis:   UUIDs of all rows in Port_Binding table having
> >                  "chassis" equal to "chassis1"
> >
> >    chassis_name: "chassis2"
> >       delete:    <empty>
> >       chassis:   UUIDs of all rows in Port_Binding table having
> >                  "chassis" equal to "chassis2"
> >
> > Transaction ACL Implementation:
> >
> > For each transaction request on a read-only connection, individual
> > operations (insert, delete, update, and mutate) are checked against
> > the root and appropriate leaf table (if present).  If a row corresponding
> > to the target table does not exist in the root table or if the target
> > column does not exist in the leaf table, the operation is rejected,
> > otherwise:
> >     The "insert" operation is allowed if the "insert" column in the
> >     root table contains "true".
> >
> 
> When an "insert" operation occurs, who or what updates the
> corresponding leaf ACL table row for that "chassis" with the
> UUID of the new row?
> 
> 
> >
> >     The "delete" operation is allowed if the "delete_any" column in the
> >     root table contains "true" or if the "delete" column in the leaf
> >     table for the originating chassis contains the UUID of the row to
> >     be deleted in the target table.
> >
> 
> When a "delete" operation occurs, who or what updates the
> corresponding leaf ACL table row for that "chassis" to remove
> the row UUID?
> 


The idea would be for ovn-northd to update the leaf ACL table(s) as
needed when insertions/deletions are seen. This does mean that there will
be some amount of time after an insertion during which deletions and
modifications will not be allowed, these will be retried and eventually
succeed (but at a cost, if retries are needed).

> 
> >     The "update" and "mutate" operations are allowed if the leaf table
> >     columns corresponding to the columns to be modified in the target
> >     table contain the UUIDs of the rows to be modified in the target
> >     table.
> >
> 
> I would prefer an approach that does not require dynamically
> updating permissions as rows are added and removed.

Agreed! Such an approach eliminates any timing issues with schemes
like options (2) and (3) that require another ovsdb client to
react to insertions/deletions.

> 
> I am thinking of something along the lines of traditional RBAC.
> 
> In each table, add a column "owner", which contains the
> CN (Common Name) field from the client's SSL certificate, as
> you proposed above. A user can always delete or update any
> rows that it owns. There is no need to define RBAC policies
> that explicitly allow such behavior.
> 
Do you envision the "owner" column being managed by ovsdb-server,
or would it be an ordinary column that is set as part of the
insert operation? If the latter, I think we'd need to think
about the possibility of spoofing (where owner is set to
something other than CN when the row is inserted). What
would this column be set to for non-SSL sessions?

> Define a new table "Roles", with the following columns:
> - "name", type string
> - "permissions", which refer by UUID to rows in a new
>   "RBAC_Permissions" table.
> 
> Define a new table "RBAC_Permissions" with the
> following columns:
> - "table", type string is the name of a table
> - "insert", type boolean
> - "delete", type boolean, indicating a permission to
>   delete any row in the table, even if the user does not
>   own that row.
> - "update", type boolean, indicating a permission to
>   update any column in any row in the table, even if
>   the user does not own that row.
> - "update_columns", type string, with each instance
>   specifying the name of a column in the "table" that
>   a user with this role is allowed to update, even if the
>   user does not own that row.
> 
> In the future, we might want to consider more generic
> policies. In particular, something that could support the
> specific policy of interest that you mentioned above:
> Allow update of the "chassis" column in the
> "port_binding" table if the chassis name equals the
> "chassis" option for this row.
> We would need to take a closer look at naming of the
> "chassis" option, since there are a few cases already
> where there is a "chassis" option of some sort,
> e.g. "l3_gateway" ports, "chassisredirect" ports.
> 
> In the connection table, add a column for "role".
> It seems like it is common in most RBAC
> implementations to allow for a user to have more than
> one role at a time.
> 
> There is also some question whether this should be
> generic for any OVSDB usage, as opposed to being
> OVN specific.
> 
> Mickey
> 

Assuming I understand this proposal, applying this to OVN_Southbound
would add a "Roles" table with:

    name: "ovn-controller"
    permissions: UUIDs of ovn-controller rows in RBAC_Permissions,
                 one for each table that can be modified by
                 an ovn-controller.

    name: "ovn-northd"
    permissions: UUID of ovn-northd row in RBAC_Permissions

(Maybe ovn-northd is not needed, if it is would also need to think
about ovn-sbctl and other applications modifying OVN_Southbound
directly.)

An RBAC_Permissions table would be added with:

   For ovn-controller, the following rows:
   
      table: "Chassis"
      insert: true
      delete: false
      update: false
      update_columns: "nb_cfg", "encaps", "external_ids", "vtep_logical_switches"

      table: "Encaps"
      insert: true
      delete: false
      update: false
      update_columns:

      table: "Port_Binding"
      insert: false
      delete: false
      update: false
      update_columns: "chassis"

      table: "MAC_Binding"
      insert: true
      delete: true
      update: true
      update_columns:

Overall, I like this, but I think it does provide less protection than
option (3) since (iiuc) update_columns allows any ovn-controller to modify these
columns in any row. E.g., rows in the Chassis table are "owned" by ovn-northd,
but ovn-controller needs to modify the contents of some columns, and we would
prefer to allow ovn-controller to modify columns such as nb_cfg only for rows
where the "name" column is equal to the CN name from the SSL certificate.

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 think we're getting closer to a good solution... thanks again!

Regards,

    Lance


More information about the dev mailing list