truenas
NAS-117602 / 22.12 / Privilege Management API
Time tracking failed: can't get issue ID from title 'Privilege Management API'
Don't change the schema for ldap and activedirectory. These are clustered, but your other configuration options are node specific. Maybe make it a system setting.
Is there a reason why you're still changing ldap plugin? The changes seem out of scope.
root@truenas[/mnt/dozer/middleware/src/middlewared]# midclt call privilege.update 1 '{"ds_groups": [100000513]}'
[EINVAL] privilege_update.local_groups.0: This Directory Service group does not exist
Should this work?
root@truenas[/mnt/dozer/middleware/src/middlewared]# getent group 100000513
BILLY\domain admins:x:100000513:
root@truenas[/mnt/dozer/middleware/src/middlewared]# midclt call group.query '[["gid", "=", 100000513]]' '{"extra": {"additional_information": ["DS"
]}}'
[{"id": 100100513, "gid": 100000513, "name": "BILLY\\domain admins", "group": "BILLY\\domain admins", "builtin": false, "sudo": false, "sudo_nopasswd": false, "sudo_commands": [], "users": [], "local": false, "id_type_both": true, "nt_name": null, "sid": null}]
@anodos325 fixed schema name and the actual bug in the DS groups validation
If I set an AD group in ds_groups and then delete the group from AD, it no longer appears in privilege.query results and so Administrator is unaware that the entry exists. This can lead to a security incident if GID is re-used for a different group. Contents of the list should always be visible in query results IMHO
We should have check during group creation for whether gid for new group is being used in any access rules. If it is, then I think we should raise a ValidationError (otherwise there can be unexpected privilege escalation on NAS). This can happen in two cases:
- User deletes group and then reuses gid.
- User creates a second group with the option to allow duplicate gids.
Since the situation with local users is fairly trivial to reproduce, we should probably include in our tests.
- Create group, add to local groups, delete group, query privileges and verify that orphaned gid is still present
- after (1) try to create new group reusing GID, verify fails with EINVAL.
When gid doesn't resolve, we return incomplete group entries.
{
"gid": 420
},
I'm not sure whether it would make more sense to at a minimum include group: None for consistency for consumers.
This currently causes multiple CI failures. C.F. custom run from this branch. https://ci.tn.ixsystems.net/jenkins/job/TrueNAS%20SCALE%20-%20Unstable/job/API%20Tests%20-%20TrueNAS%20SCALE%20with%20middleware%20reinstalled%20from%20a%20branch%20Incremental/34/
Primary issue may be that it breaks user deletion:
root@truenas[~]# midclt call user.delete 67
[EINVAL] exclude_user_ids: Not a list
root@truenas[~]# midclt call privilege.update 1 '{"local_groups": [545, 1000]}'
'NoneType' object is not subscriptable
Traceback (most recent call last):
File "/usr/local/lib/python3.9/dist-packages/middlewared/main.py", line 180, in call_method
result = await self.middleware._call(message['method'], serviceobj, methodobj, params, app=self)
File "/usr/local/lib/python3.9/dist-packages/middlewared/main.py", line 1284, in _call
return await methodobj(*prepared_call.args)
File "/usr/local/lib/python3.9/dist-packages/middlewared/service.py", line 931, in update
rv = await self.middleware._call(
File "/usr/local/lib/python3.9/dist-packages/middlewared/main.py", line 1284, in _call
return await methodobj(*prepared_call.args)
File "/usr/local/lib/python3.9/dist-packages/middlewared/schema.py", line 1152, in nf
res = await f(*args, **kwargs)
File "/usr/local/lib/python3.9/dist-packages/middlewared/schema.py", line 1284, in nf
return await func(*args, **kwargs)
File "/usr/local/lib/python3.9/dist-packages/middlewared/plugins/account_/privilege.py", line 129, in do_update
await self._validate("privilege_update", new, id)
File "/usr/local/lib/python3.9/dist-packages/middlewared/plugins/account_/privilege.py", line 169, in _validate
if not await self._ds_groups(groups, [ds_group_id], include_nonexistent=False):
File "/usr/local/lib/python3.9/dist-packages/middlewared/plugins/account_/privilege.py", line 221, in _ds_groups
if group["local"]:
TypeError: 'NoneType' object is not subscriptable
Current privileges are:
root@truenas[~]# midclt call privilege.query | jq
[
{
"id": 1,
"builtin_name": "LOCAL_ADMINISTRATOR",
"name": "Local Administrator",
"local_groups": [
{
"id": 91,
"gid": 545,
"group": "builtin_users",
"builtin": true,
"sudo": false,
"sudo_nopasswd": false,
"sudo_commands": [],
"smb": true,
"name": "builtin_users",
"users": [
66
],
"local": true,
"id_type_both": false,
"nt_name": null,
"sid": null
},
{
"id": 98,
"gid": 1000,
"group": "smbuser",
"builtin": false,
"sudo": false,
"sudo_nopasswd": false,
"sudo_commands": [],
"smb": false,
"name": "smbuser",
"users": [
66
],
"local": true,
"id_type_both": false,
"nt_name": null,
"sid": null
},
{
"gid": 9999,
"group": null
}
],
"ds_groups": [
{
"gid": 420,
"group": null
},
{
"gid": 100000513,
"group": null
}
],
"allowlist": [],
"web_shell": true
}
]
If I try to remove my directory services groups:
root@truenas[~]# midclt call privilege.update 1 '{"ds_groups": []}'
[EINVAL] privilege_update.local_groups.2: This local group does not exist
I'm not sure that an invalid local group should prevent removal of directory services groups.
@anodos325 in _ds_groups I call
group = await self.middleware.call(
"group.query",
[["gid", "=", gid]],
{
"extra": {"additional_information": ["DS"]},
"get": True,
},
)
It should raise MatchNotFound if the group is not found because we specify {"get": True} (all our other methods behave this way). Looks like it just returns None or am I missing something?
I'm not sure that an invalid local group should prevent removal of directory services groups.
You are still trying to save an invalid configuration. Most of our other methods behave the same: they don't allow saving invalid configuration even if you are just updating a small unrelated field. It's easier to program this way and I am not sure there'll be a good benefit of allowing to save an invalid configuration. Do you think I should fix this?
I ran the tests again https://ci.tn.ixsystems.net/jenkins/job/TrueNAS%20SCALE%20-%20Unstable/job/API%20Tests%20-%20TrueNAS%20SCALE%20with%20middleware%20reinstalled%20from%20a%20branch%20Incremental/35/#showFailuresLink the failures are the same as if we run from another branch, is any of them related to our changes?
It should raise MatchNotFound if the group is not found because we specify {"get": True} (all our other methods behave this way). Looks like it just returns None or am I missing something?
Query raises MatchNotFound so the problem isn't there. I'll look more closely at where it's failing.
It's breaking on a call where the include_nonexistent argument is False. I think your continue is over-indented.
is any of them related to our changes?
Error Message
AssertionError: assert 1000 == (1001 + 1)
+ where 1000 = call('group.get_next_gid')
Stacktrace
def test_group_next_gid():
next_gid = call("group.get_next_gid")
with mock("privilege.used_local_gids", f"""
def mock(self):
return {{{next_gid}: None}}
"""):
> assert call("group.get_next_gid") == next_gid + 1
E AssertionError: assert 1000 == (1001 + 1)
E + where 1000 = call('group.get_next_gid')
api2/test_account_privilege.py:100: AssertionError
I believe this is a test you're adding in this PR.
Once docstrings are updated, I don't see any other issues in testing. I'll defer to Caleb for approval of PR.