PATCHES: Password sync as active directory domain controller
Andrew Bartlett
abartlet at samba.org
Wed Jul 20 23:04:23 UTC 2016
On Thu, 2016-07-21 at 06:54 +1200, Andrew Bartlett wrote:
>
> I'll continue my review today.
>
> Thank you for your incredible patience!
I'm sorry to raise this now, but this routine needs rework:
+ def check_current_pid_conflict(pid):
+ if pid is None:
+ return False
+
+ mypid = os.getpid()
+ if pid == mypid:
+ return False
+
+ try:
+ os.kill(pid, 0)
+ except OSError as (num, msg):
+ if num != errno.ESRCH:
+ raise
+ return False
+
+ p = Popen("grep -q 'syncpasswords' /proc/%d/cmdline" %
pid, shell=True)
+ res = p.wait()
+ if res != 0:
+ return False
+ return True
We can't have a grep of files in /proc in Samba. Sorry.
If you want mutual exclusion between the scripts, can you use a
transaction lock over the ldb, or fcntl locks on another file?
Otherwise, the main thing I see is that autobuild doesn't run with --
with-gpgme.
Finally, after the https://evil32.com/ episode, we should not accept 8-
char key ids. Please update the docs to only suggest 16-char IDs, and
make the code refuse to accept < 16-char IDs.
Otherwise, I'm finally OK with all this. There is still a lot of code
here, and some of it is pretty dense, but these are the only objections
I have so far. I really appreciate the efforts you have gone to to
address my concerns.
I'm going to look over the KDC changes now, and hope to give a final
review on all this soon!
Thanks,
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list