On Mon, Mar 13, 2017 at 1:20 PM, Lance Richardson <lrich...@redhat.com> wrote:
> > From: "Mickey Spiegel" <mickeys....@gmail.com> > > To: "Lance Richardson" <lrich...@redhat.com> > > Cc: "devovs" <d...@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 <lrich...@redhat.com> > > wrote: > > > > > > From: "Mickey Spiegel" <mickeys....@gmail.com> > > > > To: "Lance Richardson" <lrich...@redhat.com> > > > > Cc: "devovs" <d...@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 < > lrich...@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: > I view this proposal as a variant of the RBAC approach. I guess the RBAC approach and the root ACL table were not that far apart. This table is equivalent to the RBAC_Permissions table, with a few simplifications. > 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. > This is the "where" column in the RBAC approach, where the "CN = " part of the logical expression is implied, and only the column part is specified. When access is attempted from an ovn-controller, then the authorization rule applies. However, there are other things accessing OVN SB DB that are allowed to change these things even though they do not have a CN, or at least not one that looks like a chassis name. For example, ovn-northd. How is this controlled? This is exactly why the indirection through role is suggested, to allow access to various things without hardcoding specific logic to determine what is subject to the rules. > - 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" > > You seem to be suggesting that perhaps "owner" is not necessary, that rules based on CN are good enough for the use cases that we need to address. Mickey _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
