feat(mac): sod mandatory and mac[manual-only] by kailixu · Pull Request #35121 · taosdata/TDengine

kailixu · 2026-04-13T09:25:33Z

Description

Issue(s)

Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

Is the user manual updated?
Are the test cases passed and automated?
Is there no significant decrease in test coverage?

…ement roles

…op/revoke

Copilot

Pull request overview

Copilot reviewed 86 out of 86 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T05:41:53Z

    case DB_OPTION_ALLOW_DROP:
      pDbOptions->allowDrop = taosStr2Int8(((SToken*)pVal)->z, NULL, 10);
+      if(pDbOptions->allowDrop != 0 && pDbOptions->allowDrop != 1) {
+        snprintf(pCxt->pQueryCxt->pMsg, pCxt->pQueryCxt->msgLen, "Invalid value for allow_drop, should be 0 or 1");
+        pCxt->errCode = TSDB_CODE_PAR_SYNTAX_ERROR;
+      }
+      break;
+    case DB_OPTION_SECURITY_LEVEL:
+      pDbOptions->securityLevel = taosStr2Int8(((SToken*)pVal)->z, NULL, 10);
      break;


setDatabaseOptionImpl parses ALLOW_DROP and SECURITY_LEVEL with taosStr2Int8, which can overflow/wrap (e.g., "256" becomes 0) and bypass the later range checks. Parse into a wider type (int64/int32) and validate the numeric range before narrowing to int8, similar to TABLE_OPTION_SECURITY_LEVEL handling.

Copilot · 2026-04-19T05:41:53Z

+static int32_t collectMetaKeyFromShowSecurityPoliciesStmt(SCollectMetaKeyCxt* pCxt, SShowStmt* pStmt) {
+  return reserveTableMetaInCache(pCxt->pParseCxt->acctId, TSDB_INFORMATION_SCHEMA_DB, TSDB_INS_TABLE_SECURITY_POLICIES,
+                                 pCxt->pMetaCache);
+}


There are now two static helpers for SHOW SECURITY_POLICIES meta collection: collectMetaKeyFromShowSecurityPolicies() and collectMetaKeyFromShowSecurityPoliciesStmt(). The *Stmt variant is not referenced anywhere, which can trigger -Wunused-function warnings (often treated as errors in CI). Remove the unused function or wire it into the switch if it’s intended to be used.

Copilot

Pull request overview

Copilot reviewed 87 out of 87 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T09:23:11Z

+  if (pAlterReq->hasSecurityLevel) {
+    // MAC: only superUser or user with PRIV_SECURITY_POLICY_ALTER can alter user security_level
+    SUserObj *pOperUser = NULL;
+    TAOS_CHECK_GOTO(mndAcquireUser(pMnode, RPC_MSG_USER(pReq), &pOperUser), &lino, _OVER);
+    if (!mndUserHasMacLabelPriv(pMnode, pOperUser)) {
+      mndReleaseUser(pMnode, pOperUser);
+      mError("user:%s, failed to alter security_level, operator %s lacks PRIV_SECURITY_POLICY_ALTER", pAlterReq->user, RPC_MSG_USER(pReq));
+      TAOS_CHECK_GOTO(TSDB_CODE_MND_NO_RIGHTS, &lino, _OVER);
+    }
+    // escalation prevention: only enforce under MAC mandatory; PRIV_SECURITY_LEVEL_ALTER holders are trusted principals
+    if (!pOperUser->superUser && pMnode->macActive == MAC_MODE_MANDATORY &&
+        !mndUserHasMacLabelPriv(pMnode, pOperUser) && pAlterReq->maxSecLevel > pOperUser->maxSecLevel) {
+      int8_t operMaxSecLevel = pOperUser->maxSecLevel;
+      mndReleaseUser(pMnode, pOperUser);
+      mError("user:%s, failed to alter security_level, target maxSecLevel(%d) exceeds operator %s maxSecLevel(%d)",
+             pAlterReq->user, pAlterReq->maxSecLevel, RPC_MSG_USER(pReq), operMaxSecLevel);
+      TAOS_CHECK_GOTO(TSDB_CODE_MAC_INSUFFICIENT_LEVEL, &lino, _OVER);


Same issue in ALTER USER: after rejecting operators that lack PRIV_SECURITY_POLICY_ALTER, the escalation-prevention block re-checks !mndUserHasMacLabelPriv(pMnode, pOperUser) and therefore never runs. This effectively disables the pAlterReq->maxSecLevel > pOperUser->maxSecLevel protection under MAC mandatory. Adjust the condition so the maxSecLevel comparison can be enforced for non-superuser operators when intended.

Copilot · 2026-04-19T09:23:11Z

  if (pCreate->isAudit == 1) {
    if (dbObj.cfg.daysToKeep2 < 2628000) {
      code = TSDB_CODE_AUDIT_MUST_KEEPFORCE;
      mError("db:%s, failed to create, keep not match for audit db, %d", pCreate->db, dbObj.cfg.daysToKeep2);
      TAOS_RETURN(code);
    }
    if (dbObj.cfg.walLevel != 2) {
      code = TSDB_CODE_AUDIT_MUST_WALFORCE;
      mError("db:%s, failed to create, walLevel not match for audit db, %d", pCreate->db, dbObj.cfg.walLevel);
      TAOS_RETURN(code);
    }
-    dbObj.cfg.allowDrop = TSDB_MIN_DB_ALLOW_DROP;
+  }
+  dbObj.cfg.allowDrop = (uint8_t)pCreate->allowDrop;
+


dbObj.cfg.allowDrop is now always taken from the request, even for audit databases. Previously audit DBs forced allowDrop=0, and the DROP DATABASE path only checks allowDrop (it doesn’t special-case isAudit). This means an audit DB can become droppable if the request sets ALLOW_DROP 1, which looks like a regression. Consider forcing allowDrop=0 when pCreate->isAudit==1 (or explicitly rejecting ALLOW_DROP 1 for audit DBs).

Copilot

Pull request overview

Copilot reviewed 90 out of 90 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T11:31:50Z

    pCxt->msgLen = stmtEnv_.msgBuf_.max_size();
    pCxt->async = async;
    pCxt->svrVer = "3.0.0.0";
+    pCxt->pCatalog = (SCatalog*)0x1;  // non-NULL so privilege checks go through mock


Setting pCxt->pCatalog to (SCatalog*)0x1 relies on the pointer never being dereferenced; if any parser path starts using pCatalog as an actual handle in tests, this will crash/UB. Prefer pointing to a real dummy SCatalog object (e.g., a static/stack instance) or a proper mock handle returned by the catalog mock init.

Copilot

Pull request overview

Copilot reviewed 90 out of 90 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T02:33:02Z

+static int32_t macCheckBySecLvl(SAuthCxt* pCxt, int8_t secLvl, bool checkNWD) {
+  if (secLvl <= 0) {
+    return TSDB_CODE_SUCCESS;
+  }


macCheckBySecLvl() treats secLvl <= 0 as always allowed, which effectively exempts security_level=0 objects from MAC enforcement. In this PR’s user-facing docs, level 0 is described as "Public" and still part of the 0-4 label range governed by NRU/NWD. Please either enforce MAC for level 0 as well (no special-casing), or adjust the docs/semantics to clearly state that 0 means "unlabeled / not enforced".

Copilot · 2026-04-20T02:33:03Z

+  if (pCxt->pComCxt->macMode && TSDB_CODE_SUCCESS == code && !pCxt->missCache && pStmt->pTableMeta != NULL) {
+    int8_t secLvl = pStmt->pTableMeta->secLvl;
+    if (secLvl > 0) {
+      if (pCxt->pComCxt->maxSecLevel < secLvl) {
+        code = TSDB_CODE_MAC_INSUFFICIENT_LEVEL;  // NRU violation
+      } else if (pCxt->pComCxt->minSecLevel > secLvl) {
+        code = TSDB_CODE_MAC_NO_WRITE_DOWN;  // NWD violation
+      }
+    }


In INSERT MAC enforcement, the check is only applied when secLvl > 0, so security_level=0 objects bypass both NRU and NWD checks. This conflicts with the documented 0-4 label range and can allow "write-down" into public (level 0) objects when MAC is mandatory. Please remove the secLvl > 0 guard (or document 0 as "unlabeled") and ensure normal-table inserts also consider DB security level if tables inherit it.

github-actions · 2026-04-20T04:00:40Z

+
+- MAC **未激活**时：GRANT 角色和 ALTER USER security_level 均不检查等级下限。
+- MAC **已激活**时：GRANT 角色要求用户的 `minSecLevel` 和 `maxSecLevel` 均满足该角色的下限约束，否则报错。ALTER USER security_level 不得将 minSecLevel 或 maxSecLevel 降低至当前已持有角色的下限以下。**此外，直接持有 `PRIV_SECURITY_POLICY_ALTER`（非角色继承）的用户，其 maxSecLevel 不得降至 4 以下。**
+- **受信主体豁免**：持有 `PRIV_SECURITY_LEVEL_ALTER` 权限的用户（即持有 SYSSEC 角色者）在设置安全等级时不受升级防护（escalation prevention）限制，可自由设置目标用户的安全等级。设置该权限是为了 taosX 数据同步，使用时，建议限制账户登录的 IP 白名单，除此之外，不建议为用户授予 `PRIV_SECURITY_LEVEL_ALTER` 权限.


🚫 [AutoCorrect Lint] <AutoCorrect> _{reported by reviewdog 🐶}

Suggested change

- **受信主体豁免**：持有 `PRIV_SECURITY_LEVEL_ALTER` 权限的用户（即持有 SYSSEC 角色者）在设置安全等级时不受升级防护（escalation prevention）限制，可自由设置目标用户的安全等级。设置该权限是为了 taosX 数据同步，使用时，建议限制账户登录的 IP 白名单，除此之外，不建议为用户授予 `PRIV_SECURITY_LEVEL_ALTER` 权限.

- **受信主体豁免**：持有 `PRIV_SECURITY_LEVEL_ALTER` 权限的用户（即持有 SYSSEC 角色者）在设置安全等级时不受升级防护（escalation prevention）限制，可自由设置权限。��户的安全等级。设置该权限是为了 taosX 数据同步，使用时，建议限制账户登录的 IP 白名单，除此之外，不建议为用户授予 `PRIV_SECURITY_LEVEL_ALTER` 权限.

Copilot

Pull request overview

Copilot reviewed 91 out of 91 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T04:04:23Z

+  if (pAlterReq->hasSecurityLevel) {
+    // MAC: only superUser or user with PRIV_SECURITY_POLICY_ALTER can alter user security_level
+    SUserObj *pOperUser = NULL;
+    TAOS_CHECK_GOTO(mndAcquireUser(pMnode, RPC_MSG_USER(pReq), &pOperUser), &lino, _OVER);
+    if (!mndUserHasMacLabelPriv(pMnode, pOperUser)) {
+      mndReleaseUser(pMnode, pOperUser);
+      mError("user:%s, failed to alter security_level, operator %s lacks PRIV_SECURITY_POLICY_ALTER", pAlterReq->user, RPC_MSG_USER(pReq));
+      TAOS_CHECK_GOTO(TSDB_CODE_MND_NO_RIGHTS, &lino, _OVER);
+    }
+    // escalation prevention: only enforce under MAC mandatory; PRIV_SECURITY_LEVEL_ALTER holders are trusted principals
+    if (!pOperUser->superUser && pMnode->macActive == MAC_MODE_MANDATORY &&
+        !mndUserHasMacLabelPriv(pMnode, pOperUser) && pAlterReq->maxSecLevel > pOperUser->maxSecLevel) {
+      int8_t operMaxSecLevel = pOperUser->maxSecLevel;
+      mndReleaseUser(pMnode, pOperUser);
+      mError("user:%s, failed to alter security_level, target maxSecLevel(%d) exceeds operator %s maxSecLevel(%d)",
+             pAlterReq->user, pAlterReq->maxSecLevel, RPC_MSG_USER(pReq), operMaxSecLevel);
+      TAOS_CHECK_GOTO(TSDB_CODE_MAC_INSUFFICIENT_LEVEL, &lino, _OVER);
+    }


Same issue as in create-user: in the hasSecurityLevel branch you already return if !mndUserHasMacLabelPriv(...), but the following "escalation prevention" if repeats !mndUserHasMacLabelPriv(...), so the branch can never be taken. Please remove/fix the redundant predicate so the check matches the intended behavior (or remove the dead check entirely if trusted principals are meant to bypass it).

kailixu added 30 commits February 18, 2026 13:40

feat(mac): add security_level for db/table

2a1fe31

feat(mac): security_level for user

ab72a77

feat(mac): security_level for db/vg

21e6c4e

feat(mac): security_level for user

7e90147

feat(mac): security_level for user

f04ae35

feat(mac): add test case

5ea85f0

feat(mac): show create db/stable

4713666

feat(mac): support allow_drop when create db

bb87805

feat(mac): support allow_drop when create db

09c7d31

feat(mac): support show security_polices

efb4f4a

feat(mac): support show security_policies

d696771

feat(mac): check show security_policies

e600fa6

feat(mac): alter cluster sod mandatory

fabd40a

feat(mac): check prerequisites for sod mandatory

72d1dd4

feat(mac): alter cluster sod mandatory

25d3d47

feat(mac): alter cluster sod mandatory

53c2543

feat(mac): process user disable

b88b6fc

feat(mac): process user disable

01124c1

feat(mac): enforce SoD by command line

0ea7806

feat(mac): enforce SoD by command line

abeddc3

feat(mac): enforce SoD by command line and SQL command

9900d79

feat(mac): dispatch sodInitial to client

9e0831b

feat(mac): enforce SoD mandatory

95e4c7d

feat(sod): sod mandatory

8007960

feat(sod): sod mandatory

2c0d6f3

feat(sod): sod mandatory

1f8b417

feat(sod): sod mandatory optimization

4981d40

feat(sod): realtime check in sodInitial phase when grant system manag…

87cdeff

…ement roles

feat(sod): ensure 3 management roles still satisfied after disable/dr…

3b3eca4

…op/revoke

feat(sod): ensure 3 management roles still satisfied after disable/dr…

1e454a7

…op/revoke

kailixu added 2 commits April 18, 2026 21:11

feat(mac): security_level restricts for system roles

4291511

chore: fix ci problem

b76364c

Copilot AI review requested due to automatic review settings April 19, 2026 05:37

Copilot started reviewing on behalf of kailixu April 19, 2026 05:38 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

kailixu added 2 commits April 19, 2026 15:15

chore: fix ci problem

b1f4511

Merge branch 'main' into feat/TD-6670071929-main

ba990c1

Copilot AI review requested due to automatic review settings April 19, 2026 09:16

Copilot started reviewing on behalf of kailixu April 19, 2026 09:17 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

kailixu added 2 commits April 19, 2026 18:14

chore: fix ci problem

fae72ad

chore: fix ci problem

638ed42

Copilot AI review requested due to automatic review settings April 19, 2026 11:26

Copilot started reviewing on behalf of kailixu April 19, 2026 11:27 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

kailixu added 2 commits April 20, 2026 06:13

chore: fix ci problem

8d6de2e

chore: handle ci review problem

e5ab09b

Copilot AI review requested due to automatic review settings April 20, 2026 02:26

Copilot started reviewing on behalf of kailixu April 20, 2026 02:27 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

kailixu added 2 commits April 20, 2026 11:25

chore: handle ci review problem

ef0d0ed

chore: handle ci review problem

365f3dd

Copilot AI review requested due to automatic review settings April 20, 2026 04:00

github-actions bot reviewed Apr 20, 2026

View reviewed changes

Copilot started reviewing on behalf of kailixu April 20, 2026 04:00 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

kailixu added 2 commits April 20, 2026 14:57

chore: fix ci problem

32fbe02

chore: fix ci problem

915c86b

Copilot AI review requested due to automatic review settings April 20, 2026 08:17

Copilot started reviewing on behalf of kailixu April 20, 2026 08:18 View session

Conversation

kailixu commented Apr 13, 2026

Description

Issue(s)

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants