Skip to content

Commit 273acd5

Browse files
dyemanovdyemanov
authored
Get rid of the unnecessary permissions for implicit domains (#8937)
* Remove security class and user privileges from the domain being dropped (see #8881: Large amount of unnecessary privileges in RDB for SYSDBA) * Fix TDBB_dont_post_dfw usage for procedures/functions. Don't grant USAGE permissions (and don't create a security class) for implicit domains (see #8881: Large amount of unnecessary privileges in RDB for SYSDBA). * Correction after Alex's review * Cleanup, thanks to Alex
1 parent 4d482a2 commit 273acd5

4 files changed

Lines changed: 63 additions & 47 deletions

File tree

‎src/dsql/DdlNodes.epp‎

Lines changed: 57 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,8 +1111,16 @@ void DdlNode::storeGlobalField(thread_db* tdbb, jrd_tra* transaction, QualifiedN
11111111
Arg::Gds(isc_dsql_max_arr_dim_exceeded));
11121112
}
11131113

1114+
bool implicitDomain = false;
1115+
11141116
if (name.object.isEmpty())
1117+
{
11151118
DYN_UTIL_generate_field_name(tdbb, name);
1119+
implicitDomain = true;
1120+
}
1121+
1122+
// Avoid the security class being assigned for implicit domains
1123+
AutoSetRestoreFlag<ULONG> noSecurityClass(&tdbb->tdbb_flags, implicitDomain ? TDBB_no_security_class : 0, true);
11161124

11171125
AutoCacheRequest requestHandle(tdbb, drq_s_fld_src, DYN_REQUESTS);
11181126

@@ -1198,7 +1206,8 @@ void DdlNode::storeGlobalField(thread_db* tdbb, jrd_tra* transaction, QualifiedN
11981206
}
11991207
}
12001208

1201-
storePrivileges(tdbb, transaction, name, obj_field, USAGE_PRIVILEGES);
1209+
if (!implicitDomain)
1210+
storePrivileges(tdbb, transaction, name, obj_field, USAGE_PRIVILEGES);
12021211
}
12031212

12041213

@@ -1900,7 +1909,7 @@ void CreateAlterFunctionNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsql
19001909
}
19011910
else if (alter)
19021911
{
1903-
if (executeAlter(tdbb, dsqlScratch, transaction, false, true))
1912+
if (executeAlter(tdbb, dsqlScratch, transaction, false, false, true))
19041913
altered = true;
19051914
else
19061915
{
@@ -1915,26 +1924,22 @@ void CreateAlterFunctionNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsql
19151924

19161925
fb_assert(id);
19171926
auto* permanent = MetadataCache::newVersion<Cached::Function>(tdbb, id);
1927+
19181928
try
19191929
{
19201930
auto* prc = permanent ? permanent->getVersioned(tdbb, CacheFlag::NOCOMMIT | CacheFlag::MINISCAN) : nullptr;
19211931
{
19221932
bool dummy = false;
1923-
AutoSetRestore compiling(prc ? &(prc->compiling) : &dummy, true);
1933+
AutoSetRestore compiling(prc ? &prc->compiling : &dummy, true);
19241934

19251935
compile(tdbb, dsqlScratch);
19261936
}
19271937

1928-
{ // scope
1929-
// avoid modify routine dfw during second pass on CREATE
1930-
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, altered ? 0 : TDBB_dont_post_dfw, true);
1931-
1932-
// second pass
1933-
if (alterIndividualParameters)
1934-
executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, true, false);
1935-
else
1936-
executeAlter(tdbb, dsqlScratch, transaction, true, false);
1937-
}
1938+
// second pass
1939+
if (alterIndividualParameters)
1940+
executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, true, false);
1941+
else
1942+
executeAlter(tdbb, dsqlScratch, transaction, !altered, true, false);
19381943

19391944
if (name.package.isEmpty())
19401945
{
@@ -2034,18 +2039,15 @@ bool CreateAlterFunctionNode::executeCreate(thread_db* tdbb, DsqlCompilerScratch
20342039
if (name.package.isEmpty())
20352040
storePrivileges(tdbb, transaction, name, obj_udf, EXEC_PRIVILEGES);
20362041

2037-
// avoid modify routine dfw when execute CREATE
2038-
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, TDBB_dont_post_dfw, true);
2039-
2040-
executeAlter(tdbb, dsqlScratch, transaction, false, false);
2042+
executeAlter(tdbb, dsqlScratch, transaction, true, false, false);
20412043

20422044
return true;
20432045
}
20442046

20452047
bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
2046-
jrd_tra* transaction, bool secondPass, bool runTriggers)
2048+
jrd_tra* transaction, bool create, bool secondPass, bool runTriggers)
20472049
{
2048-
Attachment* const attachment = transaction->getAttachment();
2050+
const auto attachment = transaction->getAttachment();
20492051

20502052
bool modified = false;
20512053
unsigned returnPos = 0;
@@ -2075,6 +2077,9 @@ bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
20752077
MetadataCache::oldVersion<Cached::Function>(tdbb, id, CacheFlag::OLD_ALTER);
20762078
}
20772079

2080+
// Avoid modify routine dfw when execute CREATE
2081+
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, create ? TDBB_dont_post_dfw : 0, true);
2082+
20782083
MODIFY FUN
20792084
if (secondPass)
20802085
{
@@ -2235,14 +2240,16 @@ bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
22352240

22362241
if (!secondPass && modified)
22372242
{
2238-
// Get all comments and defaults from the old parameter list.
2243+
// Get all comments and defaults from the old parameter list
2244+
22392245
CollectedParameterMap collectedParameters;
22402246
collectParameters(tdbb, transaction, collectedParameters);
22412247

2242-
// delete all old arguments
2248+
// Delete all old arguments ...
2249+
22432250
DropFunctionNode::dropArguments(tdbb, transaction, name);
22442251

2245-
// and insert the new ones
2252+
// ... and insert the new ones
22462253

22472254
if (returnType && returnType->type)
22482255
storeArgument(tdbb, dsqlScratch, transaction, returnPos, true, returnType, NULL);
@@ -2266,7 +2273,7 @@ bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
22662273
bool CreateAlterFunctionNode::executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
22672274
jrd_tra* transaction, bool secondPass, bool runTriggers)
22682275
{
2269-
Attachment* const attachment = transaction->getAttachment();
2276+
const auto attachment = transaction->getAttachment();
22702277

22712278
bool modified = false;
22722279

@@ -2974,7 +2981,7 @@ void CreateAlterProcedureNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsq
29742981
}
29752982
else if (alter)
29762983
{
2977-
if (executeAlter(tdbb, dsqlScratch, transaction, false, true))
2984+
if (executeAlter(tdbb, dsqlScratch, transaction, false, false, true))
29782985
altered = true;
29792986
else
29802987
{
@@ -2989,26 +2996,22 @@ void CreateAlterProcedureNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsq
29892996

29902997
fb_assert(id);
29912998
auto* permanent = MetadataCache::newVersion<Cached::Procedure>(tdbb, id);
2999+
29923000
try
29933001
{
29943002
auto* prc = permanent ? permanent->getVersioned(tdbb, CacheFlag::NOCOMMIT | CacheFlag::MINISCAN) : nullptr;
29953003
{
29963004
bool dummy = false;
2997-
AutoSetRestore compiling(prc ? &(prc->compiling) : &dummy, true);
3005+
AutoSetRestore compiling(prc ? &prc->compiling : &dummy, true);
29983006

29993007
compile(tdbb, dsqlScratch);
30003008
}
30013009

3002-
{ // scope
3003-
// avoid modify routine dfw during second pass on CREATE
3004-
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, altered ? 0 : TDBB_dont_post_dfw, true);
3005-
3006-
// second pass
3007-
if (alterIndividualParameters)
3008-
executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, true, false);
3009-
else
3010-
executeAlter(tdbb, dsqlScratch, transaction, true, false);
3011-
}
3010+
// second pass
3011+
if (alterIndividualParameters)
3012+
executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, true, false);
3013+
else
3014+
executeAlter(tdbb, dsqlScratch, transaction, !altered, true, false);
30123015

30133016
if (name.package.isEmpty())
30143017
{
@@ -3101,18 +3104,16 @@ bool CreateAlterProcedureNode::executeCreate(thread_db* tdbb, DsqlCompilerScratc
31013104
if (name.package.isEmpty())
31023105
storePrivileges(tdbb, transaction, name, obj_procedure, EXEC_PRIVILEGES);
31033106

3104-
// avoid modify routine dfw when execute CREATE
3105-
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, TDBB_dont_post_dfw, true);
3106-
3107-
executeAlter(tdbb, dsqlScratch, transaction, false, false);
3107+
executeAlter(tdbb, dsqlScratch, transaction, true, false, false);
31083108

31093109
return true;
31103110
}
31113111

31123112
bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
3113-
jrd_tra* transaction, bool secondPass, bool runTriggers)
3113+
jrd_tra* transaction, bool create, bool secondPass, bool runTriggers)
31143114
{
3115-
Attachment* const attachment = transaction->getAttachment();
3115+
const auto attachment = transaction->getAttachment();
3116+
31163117
AutoCacheRequest requestHandle(tdbb, drq_m_prcs2, DYN_REQUESTS);
31173118
bool modified = false;
31183119

@@ -3141,6 +3142,9 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
31413142
MetadataCache::oldVersion<Cached::Procedure>(tdbb, id, CacheFlag::OLD_ALTER);
31423143
}
31433144

3145+
// Avoid modify routine dfw when execute CREATE
3146+
AutoSetRestoreFlag<ULONG> noDfw(&tdbb->tdbb_flags, create ? TDBB_dont_post_dfw : 0, true);
3147+
31443148
MODIFY P
31453149
if (secondPass)
31463150
{
@@ -3227,14 +3231,16 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
32273231

32283232
if (!secondPass && modified)
32293233
{
3230-
// Get all comments and defaults from the old parameter list.
3234+
// Get all comments and defaults from the old parameter list
3235+
32313236
CollectedParameterMap collectedParameters;
32323237
collectParameters(tdbb, transaction, collectedParameters);
32333238

3234-
// Delete all old input and output parameters.
3239+
// Delete all old input and output parameters ...
3240+
32353241
DropProcedureNode::dropParameters(tdbb, transaction, name);
32363242

3237-
// And insert the new ones.
3243+
// ... and insert the new ones
32383244

32393245
for (FB_SIZE_T i = 0; i < parameters.getCount(); ++i)
32403246
{
@@ -3314,7 +3320,7 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
33143320
bool CreateAlterProcedureNode::executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
33153321
jrd_tra* transaction, bool secondPass, bool runTriggers)
33163322
{
3317-
Attachment* const attachment = transaction->getAttachment();
3323+
const auto attachment = transaction->getAttachment();
33183324

33193325
bool modified = false;
33203326

@@ -10894,6 +10900,12 @@ void DropRelationNode::deleteGlobalField(thread_db* tdbb, jrd_tra* transaction,
1089410900
ARG.RDB$FIELD_SOURCE EQ FLD.RDB$FIELD_NAME)
1089510901
{
1089610902
DropDomainNode::deleteDimensionRecords(tdbb, transaction, globalName);
10903+
10904+
if (!FLD.RDB$SECURITY_CLASS.NULL)
10905+
deleteSecurityClass(tdbb, transaction, FLD.RDB$SECURITY_CLASS);
10906+
10907+
deletePrivilegesByRelName(tdbb, transaction, globalName, obj_field);
10908+
1089710909
ERASE FLD;
1089810910
}
1089910911
END_FOR

‎src/dsql/DdlNodes.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ class CreateAlterFunctionNode final : public DdlNode
442442

443443
bool executeCreate(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
444444
bool executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
445-
bool secondPass, bool runTriggers);
445+
bool create, bool secondPass, bool runTriggers);
446446
bool executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
447447
bool secondPass, bool runTriggers);
448448

@@ -590,7 +590,7 @@ class CreateAlterProcedureNode final : public DdlNode
590590
private:
591591
bool executeCreate(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
592592
bool executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
593-
bool secondPass, bool runTriggers);
593+
bool create, bool secondPass, bool runTriggers);
594594
bool executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
595595
bool secondPass, bool runTriggers);
596596

‎src/jrd/tdbb.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ inline constexpr ULONG TDBB_dfw_cleanup = 4096; // DFW cleanup phase is activ
186186
inline constexpr ULONG TDBB_repl_in_progress = 8192; // Prevent recursion in replication
187187
inline constexpr ULONG TDBB_replicator = 16384; // Replicator
188188
inline constexpr ULONG TDBB_async = 32768; // Async context (set in AST)
189+
inline constexpr ULONG TDBB_no_security_class = 65536; // don't assign a security class to the object being created
189190

190191
class thread_db final : public Firebird::ThreadData
191192
{

‎src/jrd/vio.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7183,6 +7183,9 @@ static bool set_security_class(thread_db* tdbb, Record* record, USHORT field_id)
71837183
**************************************/
71847184
dsc desc1;
71857185

7186+
if (tdbb->tdbb_flags & TDBB_no_security_class)
7187+
return false;
7188+
71867189
if (!EVL_field(0, record, field_id, &desc1))
71877190
{
71887191
const SINT64 value = DYN_UTIL_gen_unique_id(tdbb, drq_g_nxt_sec_id, SQL_SECCLASS_GENERATOR);

0 commit comments

Comments
 (0)