sql: user with GRANT privilege can grant privileges they don't have

[solongordon]

Currently, if a user has the GRANT privilege on an object, they can grant any other privilege on that object, even if they don't have that privilege. @knz feels that users should only be allowed to grant privileges they already have.

Note that Postgres does not have the GRANT privilege. Instead, when a user is granted a privilege, they are then allowed to grant that privilege to other users if WITH GRANT OPTION was specified. We do not currently support this level of granularity.

Intuitively it makes sense to me that a user would not be able to grant privileges they don't have. I'm somewhat worried about breaking backward compatibility though.

@awoods187 would you like to weigh in on whether we should make this change, and if so whether we should prioritize it for 20.1?

[knz]

@aaron-crl can you have a look at this:

1. can you provide guidance about what is reasonable behavior (it looks to me like the original behavior was an oversight, maybe you have an opinion about best practices)

2. you could probably add the label A-sql-privileges to your filter for security-related issues

[aaron-crl]

Re: 1. The pg behavior referenced #45201 (comment) is a good pattern though we might emit an INFO or WARN that "not all permissions were assigned," in the event of an assignment failure. Outside of exceptional processes, users should not be able to assign privileges they do not possess.

Re: 2. Thanks! Will do.

I'm somewhat worried about breaking backward compatibility though.

• Do we have data on how GRANT is historically used by customers.
• Will this break any of our existing tests?
• Will this break any of our internal systems?

I think we should fix this regardless, we'll have to make sure we communicate it effectively and making sure that we support fixes for customers that might be using it in unsafe ways.

[jordanlewis]

@awoods187 @nstewart any opinions on this? If there are no opinions, this is likely not going to be fixed in 20.1.

[knz]

IMHO PM here is likely consumer of opinions (produced from Aaron or our security-minded staff), more than producer of opinions.

If we have every participant in this conversation sitting waiting for opinions from others, obviously nothing will happen.

In this case I'm in favor of the change. This does strengthen the security story.

[nstewart]

I am generally in favor of the occasional breaking change (with the appropriate warnings in release notes, of course) in the service of a) improving security and b) moving closer to pg compatibility. I'll defer to @aaron-crl and @knz re opinions on if this improves our security story