Created attachment 18618 [details] A simple way to ensure preprended zeros in GUIDs is to add .zfill(length) We have identified a small bug in: gp_cert_auto_enroll_ext.py, caused by python struct not prepending 0 (zero) to GUID. In Ubuntu the command: "adsysctl", fails due to python's struct.unpack which don't handle zero padding for GUIDs. In our AD domain registry this key exists: Computer\HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Group Policy Has data like this: {AAAAAAAA-BBBB-4EA4-8761-0CCCCCCCCCCC} (A version 4 GUID Variant 8, with last part starting with value "0C") Running "adsysctl update -a" fails on above key/data with: gensec_update_done: spnego[0x17c799f0]: NT_STATUS_OK tevent_req[0x17cc10f0/auth/gensec/spnego.c:1631]: state[2] error[0 (0x0)] state[struct gensec_spnego_update_state (0x17cc12d0)] timer[(nil)] finish[auth/gensec/spnego.c:2116] Traceback (most recent call last): File "<string>", line 142, in <module> File "<string>", line 89, in main File "<string>", line 20, in enroll File "/usr/share/adsys/python/vendor_samba/gp/gp_cert_auto_enroll_ext.py", line 517, in __enroll ca_names.extend(self.__read_cep_data(guid, ldb, TypeError: 'NoneType' object is not iterable In "gp_cert_auto_enroll_ext.py" "0CCCCCCCCCCC" is unpacked as "CCCCCCCCCCC" - as python won't pad begining/end of structs: "Padding is only automatically added between successive structure members. **No padding is added at the beginning or the end of the encoded struct.**" From: https://docs.python.org/3/library/struct.html#byte-order-size-and-alignment One solution is to zfill(length) to ensure prepended zeros: "Return a copy of the string left filled with ASCII '0' digits to make a string of length width" From: https://docs.python.org/3/library/stdtypes.html#str.zfill Diff: attached
Example test.py GUID with double prepended 0 (zeros) - demonstrating issue in original code and effect of proposed zfill change. Expected output from running attached code: test.py Mixed ldb data: b'0\xaaa\n\xbb\xbb\xa4N\x87a\x0c\xcc\xcc\xcc\xcc\xcc' Original adsys: a61aa30-bbbb-4ea4-8761-ccccccccccc Proposed zfill: 0a61aa30-bbbb-4ea4-8761-0ccccccccccc
Created attachment 18619 [details] Test script showing issue and proposed fix
Thanks. I think maybe the problems are deeper though. The %02x should zero-fill the numbers to 2 hex digits (i.e. one byte). But most of the fields are 2 byte fields, and two of them are bigger: return '%s-%s-%s-%s-%s' % ('%02x' % struct.unpack('<L', data[0:4])[0], ^^ ^^^ '%02x' % struct.unpack('<H', data[4:6])[0], '%02x' % struct.unpack('<H', data[6:8])[0], '%02x' % struct.unpack('>H', data[8:10])[0], '%02x%02x' % struct.unpack('>HL', data[10:])) ^^^^^^^ ^^^ Normally we would use: from samba.dcerpc import misc from samba.ndr import ndr_unpack def octet_string_to_objectGUID(data): return str(ndr_unpack(misc.GUID, data)) to stringify a GUID. I think that's what we want here. It works with the example: >>> b = ndr_pack(misc.GUID('AAAAAAAA-BBBB-4EA4-8761-0CCCCCCCCCCC')) >>> str(ndr_unpack(misc.GUID, b)) 'aaaaaaaa-bbbb-4ea4-8761-0ccccccccccc' >>> gp_cert_auto_enroll_ext.octet_string_to_objectGUID(b) 'aaaaaaaa-bbbb-4ea4-8761-ccccccccccc'
This bug was referenced in samba master: 47ff42232048c008a7b361a948e5ac79311b5458
Created attachment 18620 [details] patch for 4.21 and 4.22 for bugs 15839, 15829, and 15774 This contains fixes bug for 15839, bug 15829, and bug 15774. The same patch works for 4.22 and 4.21.
(In reply to Douglas Bagnall from comment #3) Good catch - thanks - setting this issue as resolved?
(In reply to cn from comment #6) Not resolved yet -- it will meander through our processes for backports etc -- but it's on the way.
Pushed to autobuild-v4-{22,21}-test.
This bug was referenced in samba v4-22-test: 21f1d226e94c150029ac1117d973cac73b1ea3a6
This bug was referenced in samba v4-21-test: 2216a4396054e1e67a0fb98cfb7965b6d20411aa
Closing out bug report. Thanks!
This bug was referenced in samba v4-22-stable (Release samba-4.22.1): 21f1d226e94c150029ac1117d973cac73b1ea3a6