Обсуждение: Rollback issue with SET ROLE
The new SET ROLE code has a bit of a problem with rollback of SET operations. For example, regression=# create user foo; CREATE ROLE regression=# create role bar; CREATE ROLE regression=# set role bar; SET regression=> show role;role ------bar (1 row) regression=> begin; BEGIN regression=> set session authorization foo; SET regression=> show role;role ------none (1 row) regression=> rollback; ROLLBACK regression=# show role;role ------none (1 row) Ideally the ROLLBACK should have restored the ROLE setting that obtained prior to BEGIN. The reason it doesn't is that the ROLLBACK effectively does a "SET SESSION AUTHORIZATION <prior-auth-value>", and that naturally clears the ROLE setting. I've been chewing on this problem for a couple hours and have come to the conclusion that it's not going to be possible to fix it without some changes to the GUC infrastructure --- there just isn't support for tracking changes to related but separate GUC variables. Even with a fix for that, there are some related nasty cases. ConsiderBEGIN;SET LOCAL SESSION AUTHORIZATION foo;SET ROLEbar;COMMIT; The SET ROLE, being nonlocal, should persist past the COMMIT. But the right to do "SET ROLE bar" would have been checked against foo's role memberships. If the outer-level session user doesn't have membership in foo, this leaves us in an illegal situation. A possible plan B is to forbid doing either SET SESSION AUTHORIZATION or SET ROLE inside a transaction block, so that none of these cases arise. This restriction is sanctioned by the SQL spec. However, we've historically allowed SET SESSION AUTHORIZATION inside a transaction block, and in fact there's a SET LOCAL SESSION AUTHORIZATION in the regression tests. Thoughts? regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Ideally the ROLLBACK should have restored the ROLE setting that obtained > prior to BEGIN. The reason it doesn't is that the ROLLBACK effectively > does a "SET SESSION AUTHORIZATION <prior-auth-value>", and that naturally > clears the ROLE setting. In this case '<prior-auth-value>' is really both the 'user' and the 'role'. Not sure that really helps though. > Even with a fix for that, there are some related nasty cases. Consider > BEGIN; > SET LOCAL SESSION AUTHORIZATION foo; > SET ROLE bar; > COMMIT; > The SET ROLE, being nonlocal, should persist past the COMMIT. But the > right to do "SET ROLE bar" would have been checked against foo's role > memberships. If the outer-level session user doesn't have membership > in foo, this leaves us in an illegal situation. To do SET SESSION AUTH, wouldn't the outer-level session user have to be superuser, and therefore you're actually fine (considering superuser is in all roles, etc)? > A possible plan B is to forbid doing either SET SESSION AUTHORIZATION > or SET ROLE inside a transaction block, so that none of these cases > arise. This restriction is sanctioned by the SQL spec. However, we've > historically allowed SET SESSION AUTHORIZATION inside a transaction > block, and in fact there's a SET LOCAL SESSION AUTHORIZATION in the > regression tests. My expectation is that they wouldn't be allowed in a transaction, I wasn't actually aware that we allowed SET SESSION AUTH in a transaction. I'm not sure I see what the use-case for it would be, to do SET SESSION AUTH you have to be superuser, aiui, and that's not something I'd expect an application to have generally (which is where I might think that dropping privileges inside a transaction might make sense, maybe...). I don't see a simple answer really. GUC variable settings still call C functions though, so I would have thought it'd be possible to track prior settings and reset them upon commit through various variables. Not exactly pretty and perhaps I'm misunderstanding how GUC deals with resetting back at a commit, but I though it called the function to deal with that. Does the function not know if it's being called for commit or rollback? Perhaps that's it, sorry I'm not of more help with this... Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> Ideally the ROLLBACK should have restored the ROLE setting that obtained >> prior to BEGIN. The reason it doesn't is that the ROLLBACK effectively >> does a "SET SESSION AUTHORIZATION <prior-auth-value>", and that naturally >> clears the ROLE setting. > In this case '<prior-auth-value>' is really both the 'user' and the > 'role'. Not sure that really helps though. Yeah, the solutions I was looking at involved various combinations of storing both values in one or both of the GUC variables. They all seemed pretty messy and fragile though. >> Even with a fix for that, there are some related nasty cases. Consider >> BEGIN; >> SET LOCAL SESSION AUTHORIZATION foo; >> SET ROLE bar; >> COMMIT; >> The SET ROLE, being nonlocal, should persist past the COMMIT. But the >> right to do "SET ROLE bar" would have been checked against foo's role >> memberships. If the outer-level session user doesn't have membership >> in foo, this leaves us in an illegal situation. > To do SET SESSION AUTH, wouldn't the outer-level session user have to be > superuser, and therefore you're actually fine (considering superuser is > in all roles, etc)? Hmm, true, but that doesn't mean you're out of the woods. If you fix the other problem by making AUTH and ROLE be effectively a single variable, then what will happen here is that SET ROLE will set the variable's value as foo/bar, and then that value will persist past COMMIT, leaving you with the wrong AUTH setting at the outer level. >> A possible plan B is to forbid doing either SET SESSION AUTHORIZATION >> or SET ROLE inside a transaction block, so that none of these cases >> arise. This restriction is sanctioned by the SQL spec. However, we've >> historically allowed SET SESSION AUTHORIZATION inside a transaction >> block, and in fact there's a SET LOCAL SESSION AUTHORIZATION in the >> regression tests. > My expectation is that they wouldn't be allowed in a transaction, I > wasn't actually aware that we allowed SET SESSION AUTH in a transaction. > I'm not sure I see what the use-case for it would be, Yeah. I actually put in code to forbid them in a transaction, but took it out again when I found the regression tests failing :-(. The offending code is in the sequence test: CREATE USER seq_user; BEGIN; SET LOCAL SESSION AUTHORIZATION seq_user; CREATE SEQUENCE seq3; SELECT nextval('seq3'); REVOKE ALL ON seq3 FROM seq_user; SELECT lastval(); ROLLBACK; DROP USER seq_user; DROP SEQUENCE seq; There isn't any amazingly good reason why this couldn't be handled without a transaction, viz SET SESSION AUTHORIZATION seq_user; ... RESET SESSION AUTHORIZATION; so unless someone comes up with a reasonable implementation plan for handling changes to both values within a transaction, I'll probably fall back to doing that. regards, tom lane