Bug 6342 - SeMachineAccountPrivilege works same as SeAddUsersPrivilege
Summary: SeMachineAccountPrivilege works same as SeAddUsersPrivilege
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: Domain Control (show other bugs)
Version: unspecified
Hardware: x86 Linux
: P3 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-08 07:53 UTC by TAKAHASHI Motonobu
Modified: 2009-06-12 05:19 UTC (History)
2 users (show)

See Also:


Attachments
Patch for master/3.4 to test. (4.86 KB, patch)
2009-05-13 12:05 UTC, Jeremy Allison
no flags Details
Pater patch - fixes more.... (6.29 KB, patch)
2009-05-13 18:01 UTC, Jeremy Allison
no flags Details
Latest patch for master/3.4 (20.94 KB, patch)
2009-05-13 20:07 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description TAKAHASHI Motonobu 2009-05-08 07:53:10 UTC
SeMachineAccountPrivilege works same as SeAddUsersPrivilege.

In Samba domain environment,
a user granted SeMachineAccountPrivilege can modify all users' property, password, account policy, same as a user granted SeAddUsersPrivilege.

Is this by design?

P.S.
A user granted SeAddUsersPrivilege can not add a machine to the domain.
A user granted SeMachineAccountPrivilege can add a machine to the domain.

So these 2 privileges work well around joining a domain.
Comment 1 Karolin Seeger 2009-05-11 04:05:11 UTC
I guess it's by design. Creating a machine account is creating another user. Jeremy, is that assumption right?
Comment 2 Jeremy Allison 2009-05-11 19:52:40 UTC
Actually no, I don't think so. SeMachineAccountPrivilage shouldn't allow you to edit normal users. I'll look at the code in this one.
Thanks,
Jeremy.
Comment 3 Jeremy Allison 2009-05-13 12:05:36 UTC
Created attachment 4148 [details]
Patch for master/3.4 to test.
Comment 4 Jeremy Allison 2009-05-13 15:07:02 UTC
Patch passes make test, so that's something.TAKAHASHI-san, can you test this as well please ?
Jeremy.
Comment 5 Jeremy Allison 2009-05-13 18:01:54 UTC
Created attachment 4150 [details]
Pater patch - fixes more....
Comment 6 Jeremy Allison 2009-05-13 18:02:26 UTC
I meant "later" not "pater" :-).
Jeremy.
Comment 7 TAKAHASHI Motonobu 2009-05-13 19:05:05 UTC
Patch id=4150 failed for me:

-----
samba33a:/home/local/Work/samba-3.3.4/source# patch -p2 < ../patch3.txt
patching file lib/privileges_basic.c
patching file rpc_server/srv_samr_nt.c
Hunk #1 succeeded at 174 (offset -11 lines).
Hunk #2 succeeded at 2139 (offset -68 lines).
Hunk #3 FAILED at 2169.
Hunk #4 FAILED at 4661.
Hunk #5 FAILED at 4715.
Hunk #6 succeeded at 4357 with fuzz 1 (offset -575 lines).
3 out of 6 hunks FAILED -- saving rejects to file rpc_server/srv_samr_nt.c.rej
-----

This patch must be applied for 3.4 tree?
I will try to manually apply this patch for 3.3.4 later.
Comment 8 Jeremy Allison 2009-05-13 19:07:59 UTC
Sorry, this code is being developed in master/3.4 not for 3.3.
This is a large enough change I want to roll it out in 3.4 only. Guenther and I are still working on this.
Jeremy
Comment 9 Jeremy Allison 2009-05-13 20:07:09 UTC
Created attachment 4151 [details]
Latest patch for master/3.4

Ok, here is the best fix I can come up with until gd finishes his testing. The only part I'm unsure about is the part of the fix marked : 

+        * JRA - TESTME. We just created this user so we
+        * had rights to create them. Do we need to check
+        * any further access on this object ? Can't we
+        * just assume we have all the rights we need ?

which is to do with the access rights on a user we just created. Please test !
Thanks,
Jeremy.
Comment 10 Karolin Seeger 2009-05-19 04:34:49 UTC
So this one won't be fixed in 3.3?
Comment 11 Guenther Deschner 2009-05-19 05:36:41 UTC
We could of course but the patch would not be tiny :-)
Comment 12 Jeremy Allison 2009-05-19 11:56:02 UTC
No, this is way too much change for 3.3 or 3.2.
Jeremy.
Comment 13 Karolin Seeger 2009-05-20 02:22:05 UTC
Okay, so it makes sense to update the component.
Comment 14 Guenther Deschner 2009-06-12 05:19:43 UTC
This has been resolved and successfully tested for 3.4 final.