Fixed CORE-5892: SQL SECURITY DEFINER context is not properly evaluat…

…ed for monitoring tables (#196)

* Now we take into account the call hierarchy when use SQL SECURITY
option.
Added new context variable in SYSTEM namespace - EFFICIENT_USER which is
returns user name in which context a code works.
We change efficient user before call procedure and function, fetch a
record from selective procedure and before execute a trigger.

* Renamed new context variable to EFFECTIVE_USER. Fixed nested calls.

* Improved error messages to print effective user when there is no permission.

* Added description of new context variable EFFECTIVE_USER. Improved description of SQL SECURITY clause.

doc/sql.extensions/README.context_variables2

@@ -98,6 +98,9 @@ Usage:
    CURRENT_USER                 | Current user for the connection. Returned value is the
                                 | same as of CURRENT_USER pseudo-variable
                                 |
+   EFFECTIVE_USER               | Effective user for now. It indicates privileges of
+                                | which user is currently used to execute function, procedure, trigger.
+                                |
    CURRENT_ROLE                 | Current role for the connection. Returned value is the
                                 | same as of CURRENT_ROLE pseudo-variable
                                 |

doc/sql.extensions/README.sql_security.txt

@@ -24,14 +24,28 @@ By default INVOKER is used to keep backward compatibility. You can change this b
 with SQL STANDARD by using ALTER DATABASE SET DEFAULT SQL SECURITY statement.
 
 If INVOKER is specified a current set of privileges of the current user will be used.
-If DEFINER - a set of privileges of object owner will be used to check an access to database objects used by this object.
+If DEFINER - a set of privileges of object owner will be used to check an access to database objects
+used by this object.
 
 Trigger inherits SQL SECURITY option from TABLE but can overwrite it by explicit specifying. If SQL SECURITY option
 will be changed for table, existing triggers without explicitly specified option will not use new value immediately
 it will take effect next time trigger will be loaded into metadata cache.
 
 For procedures and functions defined in package explicit SQL SECURITY clause is prohibit.
 
+In stored procedures, functions or triggers you may check which user if really effective and which privileges
+are applying to accessed objects by using the system context variable EFFECTIVE_USER from SYSTEM namespace.
+select RDB$GET_CONTEXT('SYSTEM', 'EFFECTIVE_USER') from RDB$DATABASE;
+
+Note: now the same object may be called in different security contexts and requires different privileges.
+For example we have:
+- a stored procedure INV with SQL SECURITY INVOKER which insert records in a table T
+- a stored procedure DEF with SQL SECURITY DEFINER defined by SYSDBA
+
+If a user U calls INV an access to T will require an INSERT privile to be granted to U (and EXECUTE on INV of course).
+In this case U is EFFECTIVE_USER due INV running.
+If user U calls DEF an access to T will require an INSERT privilege to be granted to SYSDBA (end EXECUTE on DEF).
+In this case SYSDBA is EFFECTIVE_USER due INV running as well as due DEF running.
 
 Example 1. It's enough to grant only SELECT privilege to user US for table T.
 In case of INVOKER it will require also EXECUTE for function F.

lang_helpers/gds_codes.ftn

@@ -1918,6 +1918,8 @@ C --
       PARAMETER (GDS__tra_snapshot_does_not_exist      = 335545252)
       INTEGER*4 GDS__eds_input_prm_not_used          
       PARAMETER (GDS__eds_input_prm_not_used           = 335545253)
+      INTEGER*4 GDS__effective_user                  
+      PARAMETER (GDS__effective_user                   = 335545254)
       INTEGER*4 GDS__gfix_db_name                    
       PARAMETER (GDS__gfix_db_name                     = 335740929)
       INTEGER*4 GDS__gfix_invalid_sw                 

lang_helpers/gds_codes.pas

@@ -1913,6 +1913,8 @@
 	gds_tra_snapshot_does_not_exist      = 335545252;
 	isc_eds_input_prm_not_used           = 335545253;
 	gds_eds_input_prm_not_used           = 335545253;
+	isc_effective_user                   = 335545254;
+	gds_effective_user                   = 335545254;
 	isc_gfix_db_name                     = 335740929;
 	gds_gfix_db_name                     = 335740929;
 	isc_gfix_invalid_sw                  = 335740930;

src/dsql/StmtNodes.cpp

@@ -3149,6 +3149,9 @@ void ExecProcedureNode::executeProcedure(thread_db* tdbb, jrd_req* request) cons
 				Arg::Str(procedure->getName().identifier) << Arg::Str(procedure->getName().package));
 	}
 
+	UserId* invoker = procedure->invoker ? procedure->invoker : tdbb->getAttachment()->att_ss_user;
+	AutoSetRestore<UserId*> userIdHolder(&tdbb->getAttachment()->att_ss_user, invoker);
+
 	ULONG inMsgLength = 0;
 	UCHAR* inMsg = NULL;
 

src/include/gen/codetext.h

@@ -955,6 +955,7 @@ static const struct {
 	{"bad_repl_handle", 335545251},
 	{"tra_snapshot_does_not_exist", 335545252},
 	{"eds_input_prm_not_used", 335545253},
+	{"effective_user", 335545254},
 	{"gfix_db_name", 335740929},
 	{"gfix_invalid_sw", 335740930},
 	{"gfix_incmp_sw", 335740932},

src/include/gen/iberror.h

@@ -989,6 +989,7 @@ const ISC_STATUS isc_tom_chacha_key                   = 335545250L;
 const ISC_STATUS isc_bad_repl_handle                  = 335545251L;
 const ISC_STATUS isc_tra_snapshot_does_not_exist      = 335545252L;
 const ISC_STATUS isc_eds_input_prm_not_used           = 335545253L;
+const ISC_STATUS isc_effective_user                   = 335545254L;
 const ISC_STATUS isc_gfix_db_name                     = 335740929L;
 const ISC_STATUS isc_gfix_invalid_sw                  = 335740930L;
 const ISC_STATUS isc_gfix_incmp_sw                    = 335740932L;
@@ -1463,7 +1464,7 @@ const ISC_STATUS isc_trace_switch_user_only           = 337182757L;
 const ISC_STATUS isc_trace_switch_param_miss          = 337182758L;
 const ISC_STATUS isc_trace_param_act_notcompat        = 337182759L;
 const ISC_STATUS isc_trace_mandatory_switch_miss      = 337182760L;
-const ISC_STATUS isc_err_max                          = 1407;
+const ISC_STATUS isc_err_max                          = 1408;
 
 #else /* c definitions */
 
@@ -2422,6 +2423,7 @@ const ISC_STATUS isc_err_max                          = 1407;
 #define isc_bad_repl_handle                  335545251L
 #define isc_tra_snapshot_does_not_exist      335545252L
 #define isc_eds_input_prm_not_used           335545253L
+#define isc_effective_user                   335545254L
 #define isc_gfix_db_name                     335740929L
 #define isc_gfix_invalid_sw                  335740930L
 #define isc_gfix_incmp_sw                    335740932L
@@ -2896,7 +2898,7 @@ const ISC_STATUS isc_err_max                          = 1407;
 #define isc_trace_switch_param_miss          337182758L
 #define isc_trace_param_act_notcompat        337182759L
 #define isc_trace_mandatory_switch_miss      337182760L
-#define isc_err_max                          1407
+#define isc_err_max                          1408
 
 #endif
 

src/include/gen/msgs.h

@@ -958,6 +958,7 @@ Data source : @4"},		/* eds_statement */
 	{335545251, "invalid replicator handle"},		/* bad_repl_handle */
 	{335545252, "Transaction's base snapshot number does not exist"},		/* tra_snapshot_does_not_exist */
 	{335545253, "Input parameter '@1' is not used in SQL query text"},		/* eds_input_prm_not_used */
+	{335545254, "Effective user is @1"},		/* effective_user */
 	{335740929, "data base file name (@1) already given"},		/* gfix_db_name */
 	{335740930, "invalid switch @1"},		/* gfix_invalid_sw */
 	{335740932, "incompatible switch combination"},		/* gfix_incmp_sw */

src/include/gen/sql_code.h

@@ -954,6 +954,7 @@ static const struct {
 	{335545251, -901}, /* 931 bad_repl_handle */
 	{335545252, -901}, /* 932 tra_snapshot_does_not_exist */
 	{335545253, -901}, /* 933 eds_input_prm_not_used */
+	{335545254, -551}, /* 934 effective_user */
 	{335740929, -901}, /*   1 gfix_db_name */
 	{335740930, -901}, /*   2 gfix_invalid_sw */
 	{335740932, -901}, /*   4 gfix_incmp_sw */

src/include/gen/sql_state.h

@@ -954,6 +954,7 @@ static const struct {
 	{335545251, "08003"}, // 931 bad_repl_handle
 	{335545252, "0B000"}, // 932 tra_snapshot_does_not_exist
 	{335545253, "42000"}, // 933 eds_input_prm_not_used
+	{335545254, "28000"}, // 934 effective_user
 	{335740929, "00000"}, //   1 gfix_db_name
 	{335740930, "00000"}, //   2 gfix_invalid_sw
 	{335740932, "00000"}, //   4 gfix_incmp_sw

src/jrd/Attachment.cpp

@@ -207,6 +207,8 @@ Jrd::Attachment::Attachment(MemoryPool* pool, Database* dbb)
 	: att_pool(pool),
 	  att_memory_stats(&dbb->dbb_memory_stats),
 	  att_database(dbb),
+	  att_ss_user(NULL),
+	  att_user_ids(*pool),
 	  att_active_snapshots(*pool),
 	  att_requests(*pool),
 	  att_lock_owner_id(Database::getLockOwnerId()),
@@ -986,6 +988,18 @@ bool Attachment::getIdleTimerTimestamp(ISC_TIMESTAMP_TZ& ts) const
 	return true;
 }
 
+UserId* Attachment::getUserId(const MetaName& userName)
+{
+	UserId* result = NULL;
+	if (!att_user_ids.get(userName, result))
+	{
+		result = FB_NEW_POOL(*att_pool) UserId(*att_pool);
+		result->setUserName(userName);
+		att_user_ids.put(userName, result);
+	}
+	return result;
+}
+
 /// Attachment::IdleTimer
 
 void Attachment::IdleTimer::handler()

src/jrd/Attachment.h

@@ -359,6 +359,9 @@ class Attachment : public pool_alloc<type_att>
 	Database*	att_database;				// Parent database block
 	Attachment*	att_next;					// Next attachment to database
 	UserId*		att_user;					// User identification
+	UserId*		att_ss_user;				// User identification for SQL SECURITY actual user
+	Firebird::GenericMap<Firebird::Pair<Firebird::Left<
+		Firebird::MetaName, UserId*> > > att_user_ids;	// set of used UserIds
 	jrd_tra*	att_transactions;			// Transactions belonging to attachment
 	jrd_tra*	att_dbkey_trans;			// transaction to control db-key scope
 	TraNumber	att_oldest_snapshot;		// GTT's record versions older than this can be garbage-collected
@@ -566,6 +569,8 @@ class Attachment : public pool_alloc<type_att>
 		att_batches.findAndRemove(b);
 	}
 
+	UserId* getUserId(const Firebird::MetaName &userName);
+
 private:
 	Attachment(MemoryPool* pool, Database* dbb);
 	~Attachment();
@@ -608,7 +613,8 @@ class Attachment : public pool_alloc<type_att>
 
 inline bool Attachment::locksmith(thread_db* tdbb, SystemPrivilege sp) const
 {
-	return att_user && att_user->locksmith(tdbb, sp);
+	return att_user && att_user->locksmith(tdbb, sp) ||
+			att_ss_user && att_ss_user->locksmith(tdbb, sp);
 }
 
 inline jrd_tra* Attachment::getSysTransaction()

src/jrd/ExtEngineManager.cpp

@@ -732,7 +732,7 @@ ExtEngineManager::Function::~Function()
 void ExtEngineManager::Function::execute(thread_db* tdbb, UCHAR* inMsg, UCHAR* outMsg) const
 {
 	EngineAttachmentInfo* attInfo = extManager->getEngineAttachment(tdbb, engine);
-	const MetaName& userName = udf->ssDefiner.specified && udf->ssDefiner.value ? udf->owner : "";
+	const MetaName& userName = udf->invoker ? udf->invoker->getUserName() : "";
 	ContextManager<IExternalFunction> ctxManager(tdbb, attInfo, function,
 		(udf->getName().package.isEmpty() ?
 			CallerName(obj_udf, udf->getName().identifier, userName) :
@@ -786,8 +786,7 @@ ExtEngineManager::ResultSet::ResultSet(thread_db* tdbb, UCHAR* inMsg, UCHAR* out
 	  firstFetch(true)
 {
 	attInfo = procedure->extManager->getEngineAttachment(tdbb, procedure->engine);
-	const MetaName& userName = procedure->prc->ssDefiner.specified && procedure->prc->ssDefiner.value ?
-		procedure->prc->owner : "";
+	const MetaName& userName = procedure->prc->invoker ? procedure->prc->invoker->getUserName() : "";
 	ContextManager<IExternalProcedure> ctxManager(tdbb, attInfo, procedure->procedure,
 		(procedure->prc->getName().package.isEmpty() ?
 			CallerName(obj_procedure, procedure->prc->getName().identifier, userName) :
@@ -821,8 +820,7 @@ bool ExtEngineManager::ResultSet::fetch(thread_db* tdbb)
 	if (!resultSet)
 		return wasFirstFetch;
 
-	const MetaName& userName = procedure->prc->ssDefiner.specified && procedure->prc->ssDefiner.value ?
-		procedure->prc->owner : "";
+	const MetaName& userName = procedure->prc->invoker ? procedure->prc->invoker->getUserName() : "";
 	ContextManager<IExternalProcedure> ctxManager(tdbb, attInfo, charSet,
 		(procedure->prc->getName().package.isEmpty() ?
 			CallerName(obj_procedure, procedure->prc->getName().identifier, userName) :
@@ -1286,7 +1284,7 @@ void ExtEngineManager::makeFunction(thread_db* tdbb, CompilerScratch* csb, Jrd::
 	entryPointTrimmed.trim();
 
 	EngineAttachmentInfo* attInfo = getEngineAttachment(tdbb, engine);
-	const MetaName& userName = udf->ssDefiner.specified && udf->ssDefiner.value ? udf->owner : "";
+	const MetaName& userName = udf->invoker ? udf->invoker->getUserName() : "";
 	ContextManager<IExternalFunction> ctxManager(tdbb, attInfo, attInfo->adminCharSet,
 		(udf->getName().package.isEmpty() ?
 			CallerName(obj_udf, udf->getName().identifier, userName) :
@@ -1410,7 +1408,7 @@ void ExtEngineManager::makeProcedure(thread_db* tdbb, CompilerScratch* csb, jrd_
 	entryPointTrimmed.trim();
 
 	EngineAttachmentInfo* attInfo = getEngineAttachment(tdbb, engine);
-	const MetaName& userName = prc->ssDefiner.specified && prc->ssDefiner.value ? prc->owner : "";
+	const MetaName& userName = prc->invoker ? prc->invoker->getUserName() : "";
 	ContextManager<IExternalProcedure> ctxManager(tdbb, attInfo, attInfo->adminCharSet,
 		(prc->getName().package.isEmpty() ?
 			CallerName(obj_procedure, prc->getName().identifier, userName) :

src/jrd/Function.epp

@@ -221,6 +221,7 @@ Function* Function::loadMetadata(thread_db* tdbb, USHORT id, bool noscan, USHORT
 				(X.RDB$PACKAGE_NAME.NULL ? NULL : X.RDB$PACKAGE_NAME)));
 
 			function->owner = X.RDB$OWNER_NAME;
+			Nullable<bool> ssDefiner;
 
 			if (!X.RDB$SECURITY_CLASS.NULL)
 			{
@@ -241,19 +242,22 @@ Function* Function::loadMetadata(thread_db* tdbb, USHORT id, bool noscan, USHORT
 
 					// SQL SECURITY of function must be the same if it's defined in package
 					if (!PKG.RDB$SQL_SECURITY.NULL)
-						function->ssDefiner = (bool) PKG.RDB$SQL_SECURITY;
+						ssDefiner = (bool) PKG.RDB$SQL_SECURITY;
 
 				END_FOR
 			}
 
-			if (!function->ssDefiner.specified)
+			if (!ssDefiner.specified)
 			{
 				if (!X.RDB$SQL_SECURITY.NULL)
-					function->ssDefiner = (bool) X.RDB$SQL_SECURITY;
+					ssDefiner = (bool) X.RDB$SQL_SECURITY;
 				else
-					function->ssDefiner = MET_get_ss_definer(tdbb);
+					ssDefiner = MET_get_ss_definer(tdbb);
 			}
 
+			if (ssDefiner.orElse(false))
+				function->invoker = attachment->getUserId(function->owner);
+
 			size_t count = 0;
 			ULONG length = 0;
 

src/jrd/JrdStatement.cpp

@@ -54,7 +54,7 @@ JrdStatement::JrdStatement(thread_db* tdbb, MemoryPool* p, CompilerScratch* csb)
 	  accessList(*p),
 	  resources(*p),
 	  triggerName(*p),
-	  triggerOwner(*p),
+	  triggerInvoker(NULL),
 	  parentStatement(NULL),
 	  subStatements(*p),
 	  fors(*p),
@@ -402,7 +402,8 @@ void JrdStatement::verifyAccess(thread_db* tdbb)
 	SET_TDBB(tdbb);
 
 	ExternalAccessList external;
-	buildExternalAccess(tdbb, external);
+	const MetaName defaultUser;
+	buildExternalAccess(tdbb, external, defaultUser);
 
 	for (ExternalAccess* item = external.begin(); item != external.end(); ++item)
 	{
@@ -440,7 +441,7 @@ void JrdStatement::verifyAccess(thread_db* tdbb)
 			if (!relation)
 				continue;
 
-			MetaName userName;
+			MetaName userName = item->user;
 			if (item->exa_view_id)
 			{
 				jrd_rel* view = MET_lookup_relation_id(tdbb, item->exa_view_id, false);
@@ -479,7 +480,7 @@ void JrdStatement::verifyAccess(thread_db* tdbb)
 		{
 			const SecurityClass* sec_class = SCL_get_class(tdbb, access->acc_security_name.c_str());
 
-			MetaName userName;
+			MetaName userName = item->user;
 
 			if (access->acc_ss_rel_id)
 			{
@@ -488,9 +489,6 @@ void JrdStatement::verifyAccess(thread_db* tdbb)
 					userName = view->rel_owner_name;
 			}
 
-			if (userName.isEmpty() && routine->ssDefiner.specified && routine->ssDefiner.value && routine->owner.hasData())
-				userName = routine->owner;
-
 			if (routine->getName().package.isEmpty())
 			{
 				SCL_check_access(tdbb, sec_class, userName, aclType,
@@ -695,7 +693,7 @@ void JrdStatement::verifyTriggerAccess(thread_db* tdbb, jrd_rel* ownerRelation,
 
 // Invoke buildExternalAccess for triggers in vector
 inline void JrdStatement::triggersExternalAccess(thread_db* tdbb, ExternalAccessList& list,
-	TrigVector* tvec)
+	TrigVector* tvec, const MetaName& user)
 {
 	if (!tvec)
 		return;
@@ -706,34 +704,39 @@ inline void JrdStatement::triggersExternalAccess(thread_db* tdbb, ExternalAccess
 		t.compile(tdbb);
 
 		if (t.statement)
-			t.statement->buildExternalAccess(tdbb, list);
+		{
+			const MetaName& userName = (t.ssDefiner.specified && t.ssDefiner.value) ? t.owner : user;
+			t.statement->buildExternalAccess(tdbb, list, userName);
+		}
 	}
 }
 
 // Recursively walk external dependencies (procedures, triggers) for request to assemble full
 // list of requests it depends on.
-void JrdStatement::buildExternalAccess(thread_db* tdbb, ExternalAccessList& list)
+void JrdStatement::buildExternalAccess(thread_db* tdbb, ExternalAccessList& list, const MetaName &user)
 {
 	for (ExternalAccess* item = externalList.begin(); item != externalList.end(); ++item)
 	{
 		FB_SIZE_T i;
-		if (list.find(*item, i))
-			continue;
-
-		list.insert(i, *item);
 
 		// Add externals recursively
 		if (item->exa_action == ExternalAccess::exa_procedure)
 		{
 			jrd_prc* const procedure = MET_lookup_procedure_id(tdbb, item->exa_prc_id, false, false, 0);
 			if (procedure && procedure->getStatement())
-				procedure->getStatement()->buildExternalAccess(tdbb, list);
+			{
+				item->user = procedure->invoker ? procedure->invoker->getUserName() : user;
+				procedure->getStatement()->buildExternalAccess(tdbb, list, item->user);
+			}
 		}
 		else if (item->exa_action == ExternalAccess::exa_function)
 		{
 			Function* const function = Function::lookup(tdbb, item->exa_fun_id, false, false, 0);
 			if (function && function->getStatement())
-				function->getStatement()->buildExternalAccess(tdbb, list);
+			{
+				item->user = function->invoker ? function->invoker->getUserName() : user;
+				function->getStatement()->buildExternalAccess(tdbb, list, item->user);
+			}
 		}
 		else
 		{
@@ -762,9 +765,13 @@ void JrdStatement::buildExternalAccess(thread_db* tdbb, ExternalAccessList& list
 					continue; // should never happen, silence the compiler
 			}
 
-			triggersExternalAccess(tdbb, list, vec1);
-			triggersExternalAccess(tdbb, list, vec2);
+			item->user = relation->rel_ss_definer.orElse(false) ? relation->rel_owner_name : user;
+			triggersExternalAccess(tdbb, list, vec1, item->user);
+			triggersExternalAccess(tdbb, list, vec2, item->user);
 		}
+
+		if (!list.find(*item, i))
+			list.insert(i, *item);
 	}
 }