Immediately after upgrading an AD DC from 4.8.5 to 4.9.0, running "samba-tool dbcheck" produced the following error message: Checking 511 objects ERROR(<type 'exceptions.UnicodeEncodeError'>): uncaught exception - 'ascii' codec can't encode character u'\xe1' in position 133: ordinal not in range(128) File "/usr/local/samba/lib64/python2.7/site-packages/samba/netcmd/__init__.py", line 177, in _run return self.run(*args, **kwargs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/netcmd/dbcheck.py", line 157, in run controls=controls, attrs=attrs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/dbchecker.py", line 221, in check_database error_count += self.check_object(object.dn, attrs=attrs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/dbchecker.py", line 2230, in check_object error_count += self.check_dn(obj, attrname, syntax_oid) File "/usr/local/samba/lib64/python2.7/site-packages/samba/dbchecker.py", line 1182, in check_dn dsdb_dn = dsdb_Dn(self.samdb, val.decode('utf8'), syntax_oid) File "/usr/local/samba/lib64/python2.7/site-packages/samba/common.py", line 101, in __init__ self.dn = ldb.Dn(samdb, self.dnstring) The error seems related to handling of Unicode characters. After going back to Samba 4.8.5 the error disapears. The underlying OS was CentOS 7.4. I am not sure about the severity of this bug. I classified it as major only because (at least in my case) it prevented the use of samba 4.9.0 as AD DC.
Based on my testing this only happens if you have DNs which are saved as Base64 and contain therefore utf-8 data. (e.g. polish names) I fixed it locally with the added patch. The offending commit is this one: https://git.samba.org/?p=samba.git;a=commitdiff;h=13280d3db9fdbbd87c6ed2faa69b24bc7660674d
Created attachment 14493 [details] Patch to remove utf-8 decode from dbchecker.py
Noel, Can you look into this, add tests etc? Thanks,
(In reply to Andrew Bartlett from comment #3) Hmm, it seems what is happening here is that by default data in python2 passed to dsdb_Dn is now passed as unicode whereas previously it was passed as 'str'. c python interface for ldb.Dn uses 's' format for the dn string passed to it, in this case python2 'str' is treated more like raw bytes and unicode is subject to handling by the default encoder e.g. ('ascii') I am assuming that the previous change to ensure the second param to dsdb_Dn is correct, that it is assumed a string value is expected, that it is in utf8 format. https://msdn.microsoft.com/en-us/library/cc223180.aspx would seem to indicate it should be, Andrew ? do you agree ? So it would appear that the c interface to ldb.Dn is the problem, it should accept not only byte string but also utf8 unicode. The alternative would be to adjust the code from the bulk commit below to be more forgiving (e.g. use the new python samba.compat get_string function) this would ensure that python2 code would be unaffected and only future python3 code should decode.
patch branch https://gitlab.com/samba-team/devel/samba/commits/npower_ldb_ut8 CI: https://gitlab.com/samba-team/devel/samba/pipelines/31045930
Created attachment 14499 [details] backport of patch to 4.9
Created attachment 14500 [details] patch that implements fix (& test) Version of patch accepted into master (for comparison with backport)
Comment on attachment 14493 [details] Patch to remove utf-8 decode from dbchecker.py superseded by newer version (only different in addition of tests)
Comment on attachment 14499 [details] backport of patch to 4.9 An ldb release needs to be made in the 4.9 series for this to be backported. Just bump the version in the lib/ldb/wscript and then commit with the new ABI files (ensure you build with --extra-python).
(In reply to Andrew Bartlett from comment #9) thanks, just saw this, wasn't aware of this procedure, I keep forgetting things like ldb are sort of separate
(In reply to Noel Power from comment #10) hmm, I don't see any abi changes ? (not strict ones anyway) The python c interface hasn't changes (at least the build doesn't detect any change) sure, how the python c handles the input has changed (more forgiving, e.g. will accept ut8 encoded unicode) Is there really a need to bump the ldb version for this ?
Created attachment 14501 [details] backport of patches to 4.9 (including version bump) Not being sure of the procedure I also add a version here with ldb version bump (can obsolete whichever version when I know what is what) Note: no abi change necessary
Created attachment 14565 [details] backport of patches to 4.9 with corrected version bump patch
Karolin, Please select for 4.9.next. Thanks! Andrew Bartlett
(In reply to Andrew Bartlett from comment #14) Pushed to autobuild-v4-9-test.
(In reply to Karolin Seeger from comment #15) Pushed to v4-9-test. Closing out bug report. Thanks!
Looks like the bug is still there with Samba 4.9.2: Checking 511 objects ERROR(<type 'exceptions.UnicodeDecodeError'>): uncaught exception - 'ascii' codec can't decode byte 0xc3 in position 25: ordinal not in range(128) File "/usr/local/samba/lib64/python2.7/site-packages/samba/netcmd/__init__.py", line 177, in _run return self.run(*args, **kwargs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/netcmd/dbcheck.py", line 157, in run controls=controls, attrs=attrs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/dbchecker.py", line 222, in check_database error_count += self.check_object(object.dn, attrs=attrs) File "/usr/local/samba/lib64/python2.7/site-packages/samba/dbchecker.py", line 2315, in check_object expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn)) The error disappears after reverting to Samba 4.8.6.
I had a look at this, I replied with details also to a mail about the same (but not sure if that actually went anywhere as it at least bounced from some (or maybe all) recipients. The gist of it is that the previous change to the ParseTuple format to 'es' seemed the correct thing to do. However there is a side-affect of doing this that wasn't immediately obvious to me from reading the documentation with previous format 's' behaviour is as follows: python2 (string) basically is passed through unencoded (this is probably bad but it is the default way python2 handles things) (unicode) unicode is encoded to the default encoding ('ascii') (bad because we don't expect or want that) python3 (unicode/string) is encoded to the default encoding ("utf8") with new format we added as a 'es' (and specified encoding as 'utf8') python2 (string) is re-encoded to 'utf8' (this is both unexpected and bad since effectively this operation will be string.decode('ascii').encode('utf8')) This first decode is the root cause of the current problem. (unicode) is encoded to 'utf8' (this is good) python3 (string/unicode) is encoded with the specified encoding ('utf8') (bytes) will not be accepted What can we do ? a) we could ensure every call to Ldb.Dn passes unicode This would be alot of changes, and always the risk we miss some and also going forward everyone needs to remember when writing any code that they *need* to pass unicode. Seems unworkable b) we could use the format 'et' which as far as I understand behaves as follows: python2 (string) basically is passed through unencoded (same behaviour as old python2 code) (unicode) encodes to specified format 'utf8' (this is good) (bytes) will not be accepted python3 (string/uncode) encodes to specified format ('utf8') in our case (bytes) will be accepted (with the assumption the bytes contains bytes encoded in the specified encoding) This is might be problematic in that previously we would have gotten an error attempting to pass bytes (and also the bytes might not be encoded with the correct encoding) Obviously b seems the saner option, but... there is some risk with python3 (accepting the bytes) I think the best option is a hybrid approach to use the 'et' format with python2 and the 'es' format with python3. This is yet another attempt to fix the same problem again so I really would like another pair (or more) of eyes and/or opinions on this. I'll attach master version of patch and also a 4.9 version (without the ldb version related bits), I'll add those if/when we are happy) And if there is consensus this is the correct approach we should change the other instances of use of 'es' with ParseTuple in the code too
Created attachment 14650 [details] proposed fix for master
Created attachment 14651 [details] partial fix for 4.9 (missing ldb versioning related pieces)
Comment on attachment 14650 [details] proposed fix for master fails CI (due to some issues with test)
Comment on attachment 14651 [details] partial fix for 4.9 (missing ldb versioning related pieces) issue with test
Created attachment 14652 [details] proposed new fix for master
Created attachment 14653 [details] proposed new partial fix for 4.9
CI is running here https://gitlab.com/samba-team/devel/samba/pipelines/36415402
Hello, I upgraded from 4.7.3 to 4.9.2 on Debian Buster and first I experienced internal DNS issue. After some googling I found this bug. I tried samba-tool dbcheck and it resulted in Python exception. I fixed issue the same way as earlier patches. After successful dbcheck and samba start, dns subsystem started and samba works as expected. This is my patch: --- /usr/lib/python2.7/dist-packages/samba/dbchecker.py 2018-12-29 22:12:17.058956118 +0100 +++ /tmp/dbchecker.py 2018-12-29 21:28:18.034954555 +0100 @@ -2312,7 +2312,7 @@ parent_dn = deleted_objects_dn if parent_dn is None: parent_dn = obj.dn.parent() - expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn)) + expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (str(parent_dn).decode('utf8'))) expected_dn.set_component(0, obj.dn.get_rdn_name(), name_val) if obj.dn == deleted_objects_dn: Best regards, ivan
Created attachment 14768 [details] follow patch (master version)
Created attachment 14769 [details] follow patch 4.9 version) Patch for 4.9 included needed ldb version bump
Comment on attachment 14769 [details] follow patch 4.9 version) Does the 4.9 patch need to bump the ldb version? None of the .sig files have changed from the 1.4.3 ones (and they shouldn't have). The Python-level API hasn't *really* changed has it? -- still taking str or unicode but failing in fewer cases. Do we bump the version to trigger distro action? In any case, the message is a bit lacking: > Python: Ensure ldb.Dn can doesn't rencoded str with py2 (bug 13616) Otherwise RB+.
(In reply to Douglas Bagnall from comment #29) Hi Douglas, Afaiu we need to trigger an ldb release in order to associate the ldb version with the samba version it needs to be released with (not sure if this actually ends up in the build dependencies somehow or is like you say it is a trigger for distros to make that dependency) If we bump the version we need to provide the new sigs (even if the api hasn't *really* changed) see comment #9 and following for my similar questions to yours :-) However I have to redo this patch, I just noticed that I include the patch to enable the test for py3 in the series. However, this does not work in 4.9. Well to be precise the test will work with py3 (if run manually) but the make test TEST=blah will not work properly due to some missing fixes to the some test related infrastructure code for py3. I will have to get you to rb again, sorry :-)
Created attachment 14786 [details] follow patch 4.9 version Also added Bug tag to all the patches for completeness (bug tag was only on those patches that specifically addressed the bug and not the related tests)
Re-assigning to Karolin for 4.9.next (and whatever else) p.s. just check that I needed to bump ldb version, I'm pretty sure this is needed but wanted to mention it anyway
(In reply to Noel Power from comment #32) Thanks Noel! Pushed both patches to autobuild-v4-9-test.
(In reply to Karolin Seeger from comment #33) Pushed to v4-9-test. Closing out bug report. Thanks!
This bug is not fixed yet. I just installed samba 4.9.4 as AD DC and running "samba-tool db check" still produces the following error message: ERROR(<type 'exceptions.UnicodeDecodeError'>): uncaught exception - 'ascii' codec can't decode byte 0xc3 in position 25: ordinal not in range(128) File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/__init__.py", line 177, in _run return self.run(*args, **kwargs) File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/dbcheck.py", line 157, in run controls=controls, attrs=attrs) File "/usr/local/samba/lib64/python2.6/site-packages/samba/dbchecker.py", line 222, in check_database error_count += self.check_object(object.dn, attrs=attrs) File "/usr/local/samba/lib64/python2.6/site-packages/samba/dbchecker.py", line 2315, in check_object expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn)) Reverting to Samba 4.8.9 makes the error disappear. This particular server is running CentOS 6.10. Thank you.
Re-assigning to Noel as it still seems to be an issue.
(In reply to Karolin Seeger from comment #36) The code afaics isn't in 4.9.4 Author: Karolin Seeger <kseeger@samba.org> Date: Thu Dec 20 09:23:46 2018 +0100 VERSION: Disable GIT_SNAPSHOT for the 4.9.4 release. but this only hit 4.9-test on 2019-01-31 09:41:52 UTC should I reassign back to you Karolin ? and change the status back to resolved would be great if this could be verified too @Miguel Medalha are you in a position to test from the v4-9-test branch from source?
The release notes for 4.9.2 contain the following: o Noel Power <noel.power@suse.com> * BUG 13616: ldb: Bump ldb version to 1.4.3, Python: Ensure ldb.Dn can accept utf8 encoded unicode. I assumed that this bug had been considered as solved on that version. That's why I reopened the bug when I found the problem again in version 4.9.4. I am going to build from 4.9-test and will report later. Thank you
OK. It built as Version 4.9.5-GIT-47fb4ba. "samba-tool dbcheck" gives 0 errors, so it looks like the problem is gone. I hope that the solution will be included in Release 4.9.5. Thank you!
(In reply to Miguel Medalha from comment #38) yeah, there was an initial fix (which didn't quite go far enough), then there was a secondary fix (which is still in the pipeline, it will be in the next release)
(In reply to Miguel Medalha from comment #39) thanks for testing, I appreciate that. Good to hear it is working for you now
I don't know if this is somehow related to the work done to fix this bug: I just discovered that on the version I built from samba 4.9-test, if I run samba-tool without parameters, or with the -h or --help parameters, it produces the following output: ERROR(<type 'exceptions.SyntaxError'>): uncaught exception - invalid syntax (graph.py, line 91) File "/usr/local/samba/bin/samba-tool", line 45, in <module> retval = cmd._run("samba-tool", subcommand, *args) File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/__init__.py", line 247, in _run cmd = self.subcommands[cmd_name] File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/main.py", line 35, in __getitem__ fromlist=['cmd_%s' % attr]), File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/visualize.py", line 33, in <module> from samba.graph import dot_graph Samba 4.8.9 produces the normal information about available commands.
(In reply to Miguel Medalha from comment #42) Thanks, this is a different issue and needs a new bug. I would warn that Python 2.6 is essentially found to be broken after every new Samba release so it would be best to just avoid it.
My problem is that on a particular server I still need to use CentOS 6.10. Is it possible to build Samba against a newer Python version instead of the CentOS 6.x default, without disrupting the OS? Looking at the error message again, could it be related to the "visualize" command? "samba-tool visualize --help" gives a similar error. All the other samba-tool sub-commands behave correctly with --help.
Created attachment 14906 [details] patch for comment 42 Miguel: the attached patch should fix the problem, but if you want it backported to 4.9 you'll need to open another bug. I'm afraid I don't know much about using modern Python on Centos 6, though I believe it has been done.
Thank you for the patch. Unfortunately, it does not solve the problem and now "samba-tool dbcheck" is giving errors again. I will revert to Samba 4.8.9 for now until this situation is clarified. Thank you again.
(In reply to Miguel Medalha from comment #46) What are the errors?
(In reply to Douglas Bagnall from comment #47) I am not at work now, so I don't have them here. I am now concentrating on building Samba against Python 3.x on CentOS 6.10. Building right now...
(In reply to Douglas Bagnall from comment #47) Both "samba-tool" and "samba-tool --help" produce teh following error: ERROR(<type 'exceptions.SyntaxError'>): uncaught exception - invalid syntax (graph.py, line 91) File "/usr/local/samba/bin/samba-tool", line 45, in <module> retval = cmd._run("samba-tool", subcommand, *args) File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/__init__.py", line 247, in _run cmd = self.subcommands[cmd_name] File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/main.py", line 35, in __getitem__ fromlist=['cmd_%s' % attr]), File "/usr/local/samba/lib64/python2.6/site-packages/samba/netcmd/visualize.py", line 33, in <module> from samba.graph import dot_graph
(In reply to Miguel Medalha from comment #49) Please discuss this on a new bug.
(In reply to Andrew Bartlett from comment #50) Filed as Bug 13837. https://bugzilla.samba.org/show_bug.cgi?id=13837