Thanks Dennis for the detailed analysis and suggestions! Here are a few questions and comments I have:
> Consider expanding the set of privilege definitions to be type-specific I like this! It seems like it solves the problem about inheritance and future grants as you said. I will think a bit more about it, update the doc accordingly, and see what others think. > we could introduce separate privileges TABLE_READ_DATA vs TABLE_READ_PROPERTIES In my definition in the doc, anything above table's data files is considered metadata, and TABLE_DESCRIBE governs all the access. There could be more fine-grained DESCRIBE that could be introduced, like TABLE_DESCRIBE_PROPERTIES, TABLE_DESCRIBE_HISTORY, TABLE_DESCRIBE_PARTITION. But once we get into that level, things might start to overlap. What if the user has TABLE_DESCRIBE_MANIFEST, but not TABLE_DESCRIBE_PARTITION? Do we show partial information about the manifest and remove partition information? I don't have a good solution to that yet, what do you think? > since "loadTable" is what the Catalog server sees, but then the engine could be satisfied with just the JSON metadata or might be intending to just crack open manifest files to select some aggregate statistics, or might be going all the way to Parquet files. My personal solution to this is to add a request context, which was prototyped in https://github.com/apache/iceberg/pull/10359. With this, an engine can describe the privileges needed when requesting table metadata. The prerequisite is that the catalog trusts the information passed by the engine through some authZ mechanism, and the engine uses the defined privileges here in the context. For example, if the engine requests table metadata for a DELETE, then the request will loadTable(table_name, context={privilege=DELETE}). Would that be something feasible to solve the concern? > mapping INSERT/DELETE/UPDATE all to TABLE_WRITE_DATA since at least for now, from the Catalog's perspective, any deletes require being able to write new manifests, and anything that can do inserts by writing new manifests can also effectively "delete" data in the newest snapshot. Yes I agree the privileges to insert, delete and update seems redundant given the writer can commit whatever manifest list eventually. I think some systems have a similar concept of just MODIFY privilege. But what if it is used under the fine-grained metadata commit proposal? ( https://docs.google.com/document/d/1OG68EtPxLWvNBJACQwcMrRYuGJCnQas8_LSruTRcHG8/edit) Then in that case an insert would result in a different action type in UpdateTable compared to update and delete. It seems like we should try to reach a consensus on the general direction of this proposal first. -Jack On Fri, Jun 28, 2024 at 8:53 PM Dennis Huo <huoi...@gmail.com> wrote: > +1, Thanks Jack and team for getting the discussion started with this > proposal! > > Much of this is well aligned with what we noticed when implementing RBAC > for Polaris Catalog, namely that even if a more complicated User/Role > structure exists outside of the catalog, that it's necessary to be able to > express some common building blocks around "grantee" roles/principals and > scoping/definitions of grants/privileges to make RBAC enforcement work well > and be more standardized across engines. > > Your suggestions about initially trying to avoid known problems with > things like "OWNER" privileges and problems depending on the "grantor" in > grant records definitely seem like good ideas. > > One thing that came up when trying to distill catalog-enforceable > privileges in Polaris was that by the nature of Iceberg's metadata model, > traditional SQL-style privileges ran into rough edges when it came to > distinguishing e.g. SELECT vs DESCRIBE, or UPDATE vs INSERT vs DELETE, > since "loadTable" is what the Catalog server sees, but then the engine > could be satisfied with just the JSON metadata or might be intending to > just crack open manifest files to select some aggregate statistics, or > might be going all the way to Parquet files. > > One way to address this is if we're willing to make privilege definitions > more closely reflect the implementation semantics, e.g. mapping > INSERT/DELETE/UPDATE all to TABLE_WRITE_DATA since at least for now, from > the Catalog's perspective, any deletes require being able to write new > manifests, and anything that can do inserts by writing new manifests can > also effectively "delete" data in the newest snapshot. > > It also seems like there's a relationship between having more > type-specific privileges, the ability to have unambiguous hierarchical > grants (e.g. granting TABLE_READ_DATA on a namespace to inherit the > privilege in all child tables), and also having a way to express > storage-credential-vending privileges under the same model. > > A few suggestions relating to this: > > - Consider expanding the set of privilege definitions to be > type-specific (beyond inferring the type-privilege from the object on which > a privilege is granted). Maybe there should still be a common convention > for all the "pure CRUDL" operations, but then types might have some > additional type-specific privileges too > - Example: NAMESPACE_CREATE, NAMESPACE_READ_PROPERTIES, > NAMESPACE_WRITE_PROPERTIES, NAMESPACE_DROP, NAMESPACE_LIST > - Allow/define a convention for inheriting grants in the securable > object hierarchy -- though it makes sense to also allow for non-inheritance > if an implementation wants to keep the model simple, if we do have > type-specific privileges, it at least mitigates one of the listed concerns > about accidental privileges. > - For example, if the privilege is only DESCRIBE, then granting > DESCRIBE on a namespace isn't clear whether it should also confer > DESCRIBE > on tables/views underneath it. But we could say > NAMESPACE_READ_PROPERTIES > on a namespace doesn't mean any kind of TABLE/VIEW privileges, while > TABLE_READ_PROPERTIES granted on a namespace would more clearly mean to > inherit the ability to read table properties underneath that namespace. > - Hierarchical grants probably also address some of the same use > cases that people might otherwise address with FUTURE GRANTS, and for > some > scenarios FUTURE GRANTS might be the more complex or error-prone > alternative > - To handle the concept of Catalog-based storage-credential vending, > we could introduce separate privileges TABLE_READ_DATA vs > TABLE_READ_PROPERTIES and the mutate counter parts TABLE_WRITE_DATA vs > TABLE_WRITE_PROPERTIES. Implementation-wise it could just mean > TABLE_READ_DATA/TABLE_WRITE_DATA enable receiving appropriately-scoped > storage credentials (e.g. read-only subscoped session token for > TABLE_READ_DATA) in things like loadTable and > createTable(stage-create=true). Whereas > TABLE_READ_PROPERTIES/TABLE_WRITE_PROPERTIES would only enable whatever the > REST Catalog server is able to handle directly in the REST > request/response. > > Would love to hear anyone's thoughts on these areas. > > Cheers, > Dennis Huo > > > On 2024/06/08 19:12:10 Walaa Eldin Moustafa wrote: > > Thanks Jack and team for working on this proposal. I went over it and it > is > > very well written. I particularly like: > > > > (1) The fact that it is adopting the SQL standard and adjusting some of > its > > semantics to fit the Iceberg model. > > > > (2) It includes views from v1. Views are a very important tool for policy > > enforcement. We have built a dynamic privacy and compliance enforcement > > catalog extension at LinkedIn using views [1], and one of the main > > improvements to that catalog extension would be securable view objects. > > Admittedly, it might require further improvements to compute engines to > > implement the permissions, but having an Iceberg spec would be the first > > step. > > > > Looking forward to the next steps of the proposal discussion and > adoption. > > > > [1] > > > https://www.slideshare.net/slideshow/viewshift-hassle-free-dynamic-policy-enforcement-for-every-data-lake/269577447 > > > > Thanks, > > Walaa. > > > > > > On Thu, May 30, 2024 at 10:35 PM Jack Ye <ye...@gmail.com> wrote: > > > > > Hi everyone, > > > > > > Me and a few colleagues at AWS would like to discuss a new proposal for > > > supporting securable objects in the Iceberg REST catalog spec. > > > > > > Here is our proposal in Google doc: > > > > https://docs.google.com/document/d/1KmIDbPuN6IYF0nWs9ostXIB9F4b8iH3zZO0hjgs1lm4/edit > > > > > > And here is the corresponding GitHub issue: > > > https://github.com/apache/iceberg/issues/10407 > > > > > > I will also paste the intro here for an overview. There are 2 main > reasons > > > for us to look into this area and draft this proposal: > > > > > > *IRC lacks clear guidelines on access management requirements:* > > > > > > This is feedback we heard frequently when interviewing AWS customers > using > > > Iceberg and considering building an IRC. Today Iceberg objects > (namespaces, > > > tables, views) are not securable within the Iceberg catalog itself, and > > > need to be secured using an auxiliary system. This means that an > > > organization building an IRC service needs to wrap many important > > > operations into custom-built APIs for downstream users to consume > (e.g. an > > > API to grant Iceberg table access on S3 needs to grant corresponding > IAM > > > users/roles the right S3 policy or ACL setting). Huge amount of effort > > > needs to be spent to figure out what are the missing APIs in IRC to > satisfy > > > enterprise level data warehouse access management requirements. > > > > > > There are some IRC products that offer vendor-specific APIs outside > IRC to > > > perform those operations, but this means that users are locked-in to > this > > > vendor’s securable object management system when using the IRC > solution, > > > and do not have the true freedom to easily switch to another solution > if it > > > offers better price-performance. > > > > > > We understand that Iceberg is not a security product, and it is not the > > > best interest of the community to dive too deep into security-related > > > domains. However, we believe that *we should at least offer the right > > > interfaces and set the right standards for how Iceberg catalog > expresses > > > securable objects and how Iceberg catalog users interact with those > objects*, > > > such that (1) users that would like to build IRC can have a clear > guideline > > > of what API constract to implement for managing access to objects in > IRC, > > > and (2) users that are on one IRC product do not need to be locked-in > due > > > to access management aspects. > > > > > > Would really appreciate any feedback on this topic and proposal! > > > > > > Best, > > > Jack Ye > > > > > >
