python-ldap · encukou · Jun 18, 2020 · Jun 8, 2020 · Jun 8, 2020 · Jun 9, 2020
diff --git a/Lib/ldap/syncrepl.py b/Lib/ldap/syncrepl.py
@@ -314,34 +314,35 @@ def __init__(self, encodedMessage):
         self.refreshPresent = None
         self.syncIdSet = None
 
-        for attr in ['newcookie', 'refreshDelete', 'refreshPresent', 'syncIdSet']:
-            comp = d[0].getComponentByName(attr)
-
-            if comp is not None and comp.hasValue():
-
-                if attr == 'newcookie':
-                    self.newcookie = str(comp)
-                    return
+        # Due to the way pyasn1 works, refreshDelete and refreshPresent are both
+        # valid in the components as they are fully populated defaults. We must
+        # get the component directly from the message, not by iteration.
+        attr = d[0].getName()
+        comp = d[0].getComponent()
+
+        if comp is not None and comp.hasValue():
+            if attr == 'newcookie':
+                self.newcookie = str(comp)
+                return
 
-                val = {}
+            val = {}
 
-                cookie = comp.getComponentByName('cookie')
-                if cookie.hasValue():
-                    val['cookie'] = str(cookie)
+            cookie = comp.getComponentByName('cookie')
+            if cookie.hasValue():
+                val['cookie'] = str(cookie)
 
-                if attr.startswith('refresh'):
-                    val['refreshDone'] = bool(comp.getComponentByName('refreshDone'))
-                elif attr == 'syncIdSet':
-                    uuids = []
-                    ids = comp.getComponentByName('syncUUIDs')
-                    for i in range(len(ids)):
-                        uuid = UUID(bytes=bytes(ids.getComponentByPosition(i)))
-                        uuids.append(str(uuid))
-                    val['syncUUIDs'] = uuids
-                    val['refreshDeletes'] = bool(comp.getComponentByName('refreshDeletes'))
+            if attr.startswith('refresh'):
+                val['refreshDone'] = bool(comp.getComponentByName('refreshDone'))
+            elif attr == 'syncIdSet':
+                uuids = []
+                ids = comp.getComponentByName('syncUUIDs')
+                for i in range(len(ids)):
+                    uuid = UUID(bytes=bytes(ids.getComponentByPosition(i)))
+                    uuids.append(str(uuid))
+                val['syncUUIDs'] = uuids
+                val['refreshDeletes'] = bool(comp.getComponentByName('refreshDeletes'))
 
-                setattr(self, attr, val)
-                return
+            setattr(self, attr, val)
 
 
 class SyncreplConsumer:

diff --git a/Tests/t_ldap_syncrepl.py b/Tests/t_ldap_syncrepl.py
@@ -10,6 +10,7 @@
 import shelve
 import sys
 import unittest
+import binascii
 
 if sys.version_info[0] <= 2:
     PY2 = True
@@ -21,7 +22,7 @@
 
 import ldap
 from ldap.ldapobject import SimpleLDAPObject
-from ldap.syncrepl import SyncreplConsumer
+from ldap.syncrepl import SyncreplConsumer, SyncInfoMessage
 
 from slapdtest import SlapdObject, SlapdTestCase
 
@@ -445,6 +446,47 @@ def setUp(self):
         )
         self.suffix = self.server.suffix.encode('utf-8')
 
+class DecodeSyncreplProtoTests(unittest.TestCase):
+    """
+    Tests of the ASN.1 decoder for tricky cases or past issues to ensure that
+    syncrepl messages are handled correctly.
+    """
+
+    def test_syncidset_message(self):
+        """
+        A syncrepl server may send a sync info message, with a syncIdSet
+        of uuids to delete. A regression was found in the original
+        sync info message implementation due to how the choice was
+        evaluated, because refreshPresent and refreshDelete were both
+        able to be fully expressed as defaults, causing the parser
+        to mistakenly catch a syncIdSet as a refreshPresent/refereshDelete.
+
+        This tests that a syncIdSet request is properly decoded.
+
+        reference: https://tools.ietf.org/html/rfc4533#section-2.5
+        """
+
+        # This is a dump of a syncidset message from wireshark + 389-ds
+        msg = """
+        a36b04526c6461706b64632e6578616d706c652e636f6d3a333839303123636e
+        3d6469726563746f7279206d616e616765723a64633d6578616d706c652c6463
+        3d636f6d3a286f626a656374436c6173733d2a2923330101ff311204108dc446
+        01a93611ea8aaff248c5fa5780
+        """.replace(' ', '').replace('\n', '')
+
+        msgraw = binascii.unhexlify(msg)
+        sim = SyncInfoMessage(msgraw)
+        self.assertEqual(sim.refreshDelete, None)
+        self.assertEqual(sim.refreshPresent, None)
+        self.assertEqual(sim.newcookie, None)
+        self.assertEqual(sim.syncIdSet,
+            {
+                'cookie': 'ldapkdc.example.com:38901#cn=directory manager:dc=example,dc=com:(objectClass=*)#3',
+                'syncUUIDs': ['8dc44601-a936-11ea-8aaf-f248c5fa5780'],
+                'refreshDeletes': True
+            }
+        )
+
 
 if __name__ == '__main__':
     unittest.main()