Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use security labels for table configuration. #83

Merged
merged 6 commits into from
Mar 25, 2021
Merged

Conversation

cristianberneanu
Copy link
Collaborator

Security labels are a pretty nice fit for table configuration.
We now associate the needed info directly with the objects of interest, without needing to worry about cleaning that info when the object is dropped or updating it when the object gets renamed.

For now, tables and namespaces can be marked as "sensitive" or "public". Unlabeled relations are considered public.
I wanted to add a third layer of labeling on the database level, but I didn't know how to get the database Oid, so I left it for another time.

The mechanism seems flexible enough, so additional information can be easily included into the labels.
It can also be extended for other purposes, like configuring what functions are allowed or not, for example.

This PR also does some refactoring to increase the separation between modules and drops some unused fields.

@cristianberneanu
Copy link
Collaborator Author

Ready for review.

@edongashi
Copy link
Member

@edongashi
Copy link
Member

edongashi commented Mar 25, 2021

Also appears that there's a global MyDatabaseId https://github.com/postgres/postgres/blob/f2c7ce64ae9ba292c1846ae864cef1b8b37af1f3/src/include/miscadmin.h#L187

We'll handle this later.

@cristianberneanu
Copy link
Collaborator Author

get_database_oid in dbcommands.h could be relevant.

I found that one, but it requires a name, which I don't have.

Also appears that there's a global MyDatabaseId

This is interesting, thanks!

src/auth.c Outdated

if (seclabel == NULL)
{
ObjectAddress object = {.classId = NamespaceRelationId, .objectId = namespace_oid, .objectSubId = 0};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea of permission inheritance!

Could we avoid shadowing and instead call these something like relation_object and namespace_object?

else if (is_public_label(seclabel))
return false;
else
FAILWITH_CODE(ERRCODE_INVALID_NAME, "'%s' is not a valid label", seclabel);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the compiler complain about a missing return in this branch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't. I think it detects that it performs an abort(), so it is a noreturn function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants