Kevin Kunkel reports that renaming a site from ADUC crashed the LDAP server with signal 6. Details are pending.
Created attachment 13606 [details] Active Directory Sites and Service MSC plugin When renaming a subnet in the Active Directory Sites and Services MSC plugin, Samba's LDAP process will crash: [2017/09/12 11:28:37.037848, 0] ../lib/util/fault.c:78(fault_report) =============================================================== [2017/09/12 11:28:37.037951, 0] ../lib/util/fault.c:79(fault_report) INTERNAL ERROR: Signal 11 in pid 10066 (4.6.7-SerNet-RedHat-11.el7) Please read the Trouble-Shooting section of the Samba HOWTO [2017/09/12 11:28:37.037973, 0] ../lib/util/fault.c:81(fault_report) =============================================================== [2017/09/12 11:28:37.037986, 0] ../lib/util/fault.c:151(smb_panic_default) PANIC: internal error [2017/09/12 11:28:37.043107, 0] ../source4/smbd/process_standard.c:127(standard_child_pipe_handler) Child 10066 (ldap) terminated with signal 6
Created attachment 13607 [details] backtrace using ldbrename of subnet Command to trigger segfault: bin/ldbrename -H st/ad_dc/private/sam.ldb CN=10.9.0.0/24,CN=Subnets,CN=Sites,CN=Configuration,... CN=10.9.0.1/24,CN=Subnets,CN=Sites,CN=Configuration,...
Fix is relatively trivial. Pass in a new parameter, DN, to the verify subnet function. Impact is restricted to those able to modify an existing subnet.
Created attachment 13611 [details] Patch and test
Created attachment 13889 [details] patch and test v2 RB+ by me with the following modification: --- a/python/samba/subnets.py +++ b/python/samba/subnets.py @@ -153,9 +153,11 @@ def rename_subnet(samdb, configDn, subnet_name, new_name): if enum == ldb.ERR_NO_SUCH_OBJECT: raise SubnetNotFound('Subnet %s does not exist' % subnet) elif enum == ldb.ERR_ENTRY_ALREADY_EXISTS: - raise SubnetAlreadyExists('A subnet with the CIDR %s already exists') + raise SubnetAlreadyExists('A subnet with the CIDR %s already exists' + % new_name) elif enum == ldb.ERR_INVALID_DN_SYNTAX: - raise SubnetInvalid("%s is not a valid subnet: %s" % (new_name, estr)) + raise SubnetInvalid("%s is not a valid subnet: %s" % (new_name, + estr)) else: raise The first -+ adds a missing name in the error string. The second -+ avoids the 80th column.
Thanks Björn, The status from my point of view is I assumed it had already gone upstream. We will get moving.
Created attachment 13890 [details] patch with BUG:, tested on master, 4.5, 4.6, 4.7 This patch applies as far back as 4.4 which introduced the feature/bug. The relevant tests pass on 4.5, 4.6, 4.7.
The patch also applies against v4-8-test.
Created attachment 13962 [details] Additional patch for master testing if non-admin users can trigger this bug Can non-admin users trigger this bug? These tests say NO. gdb confirms that access rights are checked before samldb_verify_subnet() is reached. This patch can go into master in a non urgent fashion, but NOT before the security patch. There is any point in backporting it, as far as I can see.
Reassigning to Karolin for inclusion in 4.8, 4.7 and 4.6.
(In reply to Ralph Böhme from comment #14) Pushed to autobuild-v4{6,7,8}-test.
Pushed to all branches. Closing out bug report. Thanks!