Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR - Fixing 4-byte UTF-8 character validation #2840

Closed
389-ds-bot opened this issue Sep 13, 2020 · 25 comments
Closed

PR - Fixing 4-byte UTF-8 character validation #2840

389-ds-bot opened this issue Sep 13, 2020 · 25 comments
Labels
merged Migration flag - PR pr Migration flag - PR

Comments

@389-ds-bot
Copy link

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/49781

  • Created at 2018-06-14 20:37:22 by djpadz
  • Merged at 2018-06-18 20:14:36

When validating syntax for four-byte UTF-8 characters, the pointer wasn't being advanced, which was causing false syntax validation failures for directory strings that, for example, included pictographs.

@389-ds-bot 389-ds-bot added merged Migration flag - PR pr Migration flag - PR labels Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-14 21:23:15

Thanks for the contribution @djpadz!

This looks good to me, can you rebase it with master branch?

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-14 21:26:09

Done.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-14 21:26:23

rebased onto 2707e39

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-14 21:26:41

Do you need a test for this?

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-14 21:29:51

You read my mind :-) Yes if you have a test (written in python-lib389), or even just the exact manual steps to reproduce, that would be great.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-14 21:30:07

Okeedoke. I'll work on that :-)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-14 21:30:49

What version of 389-ds-base are you using/need this fixed in?

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-14 21:33:10

We're using it with 1.2.11.15-94.el6_9, but the backport was (obviously) easy. I'm hoping to get the servers up to EL 7 before we go live with it. I'm not sure what version is in the repos for el7.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-15 04:10:15

1 new commit added

  • test for 49781

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-15 04:12:07

@mreynolds389 - I've pushed a test, but I can't seem to get it to run. I suspect it's something environmental, since none of the tests that call modify_s() are working. In any case, here's the output. Any advice would really be appreciated.

I'm running this on a clean install of Fedora 28.

[djpadz@fedora-linux-28 389-ds-base]$ sudo py.test -s ~/389-ds-base/dirsrvtests/tests/tickets/ticket49781_test.py 
============================= test session starts ==============================
platform linux -- Python 3.6.5, pytest-3.6.1, py-1.5.3, pluggy-0.6.0
rootdir: /home/djpadz/389-ds-base, inifile:
collected 1 item                                                               

dirsrvtests/tests/tickets/ticket49781_test.py OK group dirsrv exists
OK user dirsrv exists
INFO:lib389.topologies:Instance with parameters {'ldap-port': 38901, 'ldap-secureport': 63601, 'server-id': 'standalone1', 'suffix': 'dc=example,dc=com'} was created.
CRITICAL:dirsrvtests.tests.tickets.ticket49781_test:trivial test failed!
FInstance slapd-standalone1 removed.


=================================== FAILURES ===================================
_______________________________ test_ticket49781 _______________________________

topology_st = <lib389.topologies.TopologyMain object at 0x7f54aa549ba8>

    def test_ticket49781(topology_st):
        """
            Test that four-byte UTF-8 characters are accepted by the
            directory string syntax.
        """
    
        # Add a test user
        try:
            topology_st.standalone.add_s(Entry((USER_DN,
                                                {'objectclass': ['top', 'person'],
                                                 'sn': 'sn',
                                                 'description': 'Four-byte UTF8 test',
                                                 'cn': 'test_user'})))
        except ldap.LDAPError as e:
            log.fatal('Failed to add test user')
            assert False
    
        try:
>           topology_st.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, 'cn', b'something else')])

dirsrvtests/tests/tickets/ticket49781_test.py:57: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = ('cn=test_user,dc=example,dc=com', [(2, 'cn', b'something else')])
kwargs = {}

    def inner(*args, **kwargs):
        if name == 'result':
            objtype, data = f(*args, **kwargs)
            # data is either a 2-tuple or a list of 2-tuples
            # print data
            if data:
                if isinstance(data, tuple):
                    return objtype, Entry(data)
                elif isinstance(data, list):
                    # AD sends back these search references
                    # if objtype == ldap.RES_SEARCH_RESULT and \
                    #    isinstance(data[-1],tuple) and \
                    #    not data[-1][0]:
                    #     print "Received search reference: "
                    #     pprint.pprint(data[-1][1])
                    #     data.pop() # remove the last non-entry element
    
                    return objtype, [Entry(x) for x in data]
                else:
                    raise TypeError("unknown data type %s returned by result" %
                                    type(data))
            else:
                return objtype, data
        elif name.startswith('add'):
            # the first arg is self
            # the second and third arg are the dn and the data to send
            # We need to convert the Entry into the format used by
            # python-ldap
            ent = args[0]
            if isinstance(ent, Entry):
                return f(ent.dn, ent.toTupleList(), *args[2:])
            else:
                return f(*args, **kwargs)
        else:
>           return f(*args, **kwargs)

/usr/local/lib/python3.6/site-packages/lib389-1.4.0.1-py3.6.egg/lib389/__init__.py:167: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>
dn = 'cn=test_user,dc=example,dc=com', modlist = [(2, 'cn', b'something else')]

    def modify_s(self,dn,modlist):
>     return self.modify_ext_s(dn,modlist,None,None)

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:629: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = ('cn=test_user,dc=example,dc=com', [(2, 'cn', b'something else')], None, None)
kwargs = {}

    def inner(*args, **kwargs):
        if name == 'result':
            objtype, data = f(*args, **kwargs)
            # data is either a 2-tuple or a list of 2-tuples
            # print data
            if data:
                if isinstance(data, tuple):
                    return objtype, Entry(data)
                elif isinstance(data, list):
                    # AD sends back these search references
                    # if objtype == ldap.RES_SEARCH_RESULT and \
                    #    isinstance(data[-1],tuple) and \
                    #    not data[-1][0]:
                    #     print "Received search reference: "
                    #     pprint.pprint(data[-1][1])
                    #     data.pop() # remove the last non-entry element
    
                    return objtype, [Entry(x) for x in data]
                else:
                    raise TypeError("unknown data type %s returned by result" %
                                    type(data))
            else:
                return objtype, data
        elif name.startswith('add'):
            # the first arg is self
            # the second and third arg are the dn and the data to send
            # We need to convert the Entry into the format used by
            # python-ldap
            ent = args[0]
            if isinstance(ent, Entry):
                return f(ent.dn, ent.toTupleList(), *args[2:])
            else:
                return f(*args, **kwargs)
        else:
>           return f(*args, **kwargs)

/usr/local/lib/python3.6/site-packages/lib389-1.4.0.1-py3.6.egg/lib389/__init__.py:167: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>
dn = 'cn=test_user,dc=example,dc=com', modlist = [(2, 'cn', b'something else')]
serverctrls = None, clientctrls = None

    def modify_ext_s(self,dn,modlist,serverctrls=None,clientctrls=None):
      msgid = self.modify_ext(dn,modlist,serverctrls,clientctrls)
>     resp_type, resp_data, resp_msgid, resp_ctrls = self.result3(msgid,all=1,timeout=self.timeout)

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:602: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (3,), kwargs = {'all': 1, 'timeout': -1}

    def inner(*args, **kwargs):
        if name == 'result':
            objtype, data = f(*args, **kwargs)
            # data is either a 2-tuple or a list of 2-tuples
            # print data
            if data:
                if isinstance(data, tuple):
                    return objtype, Entry(data)
                elif isinstance(data, list):
                    # AD sends back these search references
                    # if objtype == ldap.RES_SEARCH_RESULT and \
                    #    isinstance(data[-1],tuple) and \
                    #    not data[-1][0]:
                    #     print "Received search reference: "
                    #     pprint.pprint(data[-1][1])
                    #     data.pop() # remove the last non-entry element
    
                    return objtype, [Entry(x) for x in data]
                else:
                    raise TypeError("unknown data type %s returned by result" %
                                    type(data))
            else:
                return objtype, data
        elif name.startswith('add'):
            # the first arg is self
            # the second and third arg are the dn and the data to send
            # We need to convert the Entry into the format used by
            # python-ldap
            ent = args[0]
            if isinstance(ent, Entry):
                return f(ent.dn, ent.toTupleList(), *args[2:])
            else:
                return f(*args, **kwargs)
        else:
>           return f(*args, **kwargs)

/usr/local/lib/python3.6/site-packages/lib389-1.4.0.1-py3.6.egg/lib389/__init__.py:167: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>, msgid = 3, all = 1
timeout = -1, resp_ctrl_classes = None

    def result3(self,msgid=ldap.RES_ANY,all=1,timeout=None,resp_ctrl_classes=None):
      resp_type, resp_data, resp_msgid, decoded_resp_ctrls, retoid, retval = self.result4(
        msgid,all,timeout,
        add_ctrls=0,add_intermediates=0,add_extop=0,
>       resp_ctrl_classes=resp_ctrl_classes
      )

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:749: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (3, 1, -1)
kwargs = {'add_ctrls': 0, 'add_extop': 0, 'add_intermediates': 0, 'resp_ctrl_classes': None}

    def inner(*args, **kwargs):
        if name == 'result':
            objtype, data = f(*args, **kwargs)
            # data is either a 2-tuple or a list of 2-tuples
            # print data
            if data:
                if isinstance(data, tuple):
                    return objtype, Entry(data)
                elif isinstance(data, list):
                    # AD sends back these search references
                    # if objtype == ldap.RES_SEARCH_RESULT and \
                    #    isinstance(data[-1],tuple) and \
                    #    not data[-1][0]:
                    #     print "Received search reference: "
                    #     pprint.pprint(data[-1][1])
                    #     data.pop() # remove the last non-entry element
    
                    return objtype, [Entry(x) for x in data]
                else:
                    raise TypeError("unknown data type %s returned by result" %
                                    type(data))
            else:
                return objtype, data
        elif name.startswith('add'):
            # the first arg is self
            # the second and third arg are the dn and the data to send
            # We need to convert the Entry into the format used by
            # python-ldap
            ent = args[0]
            if isinstance(ent, Entry):
                return f(ent.dn, ent.toTupleList(), *args[2:])
            else:
                return f(*args, **kwargs)
        else:
>           return f(*args, **kwargs)

/usr/local/lib/python3.6/site-packages/lib389-1.4.0.1-py3.6.egg/lib389/__init__.py:167: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>, msgid = 3, all = 1
timeout = -1, add_ctrls = 0, add_intermediates = 0, add_extop = 0
resp_ctrl_classes = None

    def result4(self,msgid=ldap.RES_ANY,all=1,timeout=None,add_ctrls=0,add_intermediates=0,add_extop=0,resp_ctrl_classes=None):
      if timeout is None:
        timeout = self.timeout
>     ldap_result = self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:756: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (<built-in method result4 of LDAP object at 0x7f54a94ea170>, 3, 1, -1, 0, 0, ...)
kwargs = {}

    def inner(*args, **kwargs):
        if name == 'result':
            objtype, data = f(*args, **kwargs)
            # data is either a 2-tuple or a list of 2-tuples
            # print data
            if data:
                if isinstance(data, tuple):
                    return objtype, Entry(data)
                elif isinstance(data, list):
                    # AD sends back these search references
                    # if objtype == ldap.RES_SEARCH_RESULT and \
                    #    isinstance(data[-1],tuple) and \
                    #    not data[-1][0]:
                    #     print "Received search reference: "
                    #     pprint.pprint(data[-1][1])
                    #     data.pop() # remove the last non-entry element
    
                    return objtype, [Entry(x) for x in data]
                else:
                    raise TypeError("unknown data type %s returned by result" %
                                    type(data))
            else:
                return objtype, data
        elif name.startswith('add'):
            # the first arg is self
            # the second and third arg are the dn and the data to send
            # We need to convert the Entry into the format used by
            # python-ldap
            ent = args[0]
            if isinstance(ent, Entry):
                return f(ent.dn, ent.toTupleList(), *args[2:])
            else:
                return f(*args, **kwargs)
        else:
>           return f(*args, **kwargs)

/usr/local/lib/python3.6/site-packages/lib389-1.4.0.1-py3.6.egg/lib389/__init__.py:167: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>
func = <built-in method result4 of LDAP object at 0x7f54a94ea170>
args = (3, 1, -1, 0, 0, 0), kwargs = {}, diagnostic_message_success = None
exc_type = None, exc_value = None, exc_traceback = None

    def _ldap_call(self,func,*args,**kwargs):
      """
        Wrapper method mainly for serializing calls into OpenLDAP libs
        and trace logs
        """
      self._ldap_object_lock.acquire()
      if __debug__:
        if self._trace_level>=1:
          self._trace_file.write('*** %s %s - %s\n%s\n' % (
            repr(self),
            self._uri,
            '.'.join((self.__class__.__name__,func.__name__)),
            pprint.pformat((args,kwargs))
          ))
          if self._trace_level>=9:
            traceback.print_stack(limit=self._trace_stack_limit,file=self._trace_file)
      diagnostic_message_success = None
      try:
        try:
          result = func(*args,**kwargs)
          if __debug__ and self._trace_level>=2:
            if func.__name__!="unbind_ext":
              diagnostic_message_success = self._l.get_option(ldap.OPT_DIAGNOSTIC_MESSAGE)
        finally:
          self._ldap_object_lock.release()
      except LDAPError as e:
        exc_type,exc_value,exc_traceback = sys.exc_info()
        try:
          if 'info' not in e.args[0] and 'errno' in e.args[0]:
            e.args[0]['info'] = strerror(e.args[0]['errno'])
        except IndexError:
          pass
        if __debug__ and self._trace_level>=2:
          self._trace_file.write('=> LDAPError - %s: %s\n' % (e.__class__.__name__,str(e)))
        try:
>         reraise(exc_type, exc_value, exc_traceback)

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:329: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

exc_type = <class 'ldap.NOT_ALLOWED_ON_RDN'>
exc_value = NOT_ALLOWED_ON_RDN({'desc': 'Operation not allowed on RDN'},)
exc_traceback = <traceback object at 0x7f54aa605d48>

    def reraise(exc_type, exc_value, exc_traceback):
        """Re-raise an exception given information from sys.exc_info()
    
            Note that unlike six.reraise, this does not support replacing the
            traceback. All arguments must come from a single sys.exc_info() call.
            """
        # In Python 3, all exception info is contained in one object.
>       raise exc_value

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/compat.py:44: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lib389.DirSrv object at 0x7f54aa5c1128>
func = <built-in method result4 of LDAP object at 0x7f54a94ea170>
args = (3, 1, -1, 0, 0, 0), kwargs = {}, diagnostic_message_success = None
exc_type = None, exc_value = None, exc_traceback = None

    def _ldap_call(self,func,*args,**kwargs):
      """
        Wrapper method mainly for serializing calls into OpenLDAP libs
        and trace logs
        """
      self._ldap_object_lock.acquire()
      if __debug__:
        if self._trace_level>=1:
          self._trace_file.write('*** %s %s - %s\n%s\n' % (
            repr(self),
            self._uri,
            '.'.join((self.__class__.__name__,func.__name__)),
            pprint.pformat((args,kwargs))
          ))
          if self._trace_level>=9:
            traceback.print_stack(limit=self._trace_stack_limit,file=self._trace_file)
      diagnostic_message_success = None
      try:
        try:
>         result = func(*args,**kwargs)
E         ldap.NOT_ALLOWED_ON_RDN: {'desc': 'Operation not allowed on RDN'}

/usr/local/lib/python3.6/site-packages/python_ldap-3.1.0-py3.6-linux-x86_64.egg/ldap/ldapobject.py:313: NOT_ALLOWED_ON_RDN

During handling of the above exception, another exception occurred:

topology_st = <lib389.topologies.TopologyMain object at 0x7f54aa549ba8>

    def test_ticket49781(topology_st):
        """
            Test that four-byte UTF-8 characters are accepted by the
            directory string syntax.
        """
    
        # Add a test user
        try:
            topology_st.standalone.add_s(Entry((USER_DN,
                                                {'objectclass': ['top', 'person'],
                                                 'sn': 'sn',
                                                 'description': 'Four-byte UTF8 test',
                                                 'cn': 'test_user'})))
        except ldap.LDAPError as e:
            log.fatal('Failed to add test user')
            assert False
    
        try:
            topology_st.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, 'cn', b'something else')])
        except ldap.LDAPError as e:
            log.fatal('trivial test failed!')
>           assert False
E           assert False

dirsrvtests/tests/tickets/ticket49781_test.py:60: AssertionError
------------------------------ Captured log setup ------------------------------
topologies.py              106 INFO     Instance with parameters {'ldap-port': 38901, 'ldap-secureport': 63601, 'server-id': 'standalone1', 'suffix': 'dc=example,dc=com'} was created.
------------------------------ Captured log call -------------------------------
ticket49781_test.py         59 CRITICAL trivial test failed!
=========================== 1 failed in 7.82 seconds ===========================

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2018-06-15 07:01:51

E         ldap.NOT_ALLOWED_ON_RDN: {'desc': 'Operation not allowed on RDN'}

You're trying to replace cn, which is part of RDN of that test user. If I change test to modify description, then it works.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-15 07:41:21

1 new commit added

  • Fixed the test attribute in the test user

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-15 07:41:58

omg... I can't believe I missed that! Thanks, @vashirov! It's now fixed.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-15 09:54:10

@mreynolds389 would you like PRs for each of the release branches as well?

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-17 19:51:54

Filed 49788 so the test can be common to all branches.

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-17 19:54:39

2 new commits added

  • Test for issue 49788
  • Fixing 4-byte UTF-8 character validation

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-18 20:14:36

Pull-Request has been merged by mreynolds389

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-18 20:30:24

@djpadz cherry picked the fixes to all the branches (even 1.2.11 - although we are not building it anymore)

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-18 20:31:59

@mreynolds389 The fix isn't exactly the same between versions... I mean, it's still p++; in two places, but cherry-picking didn't run cleanly for me on all branches. Would you like PRs for each of the branches?

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-18 20:33:28

Its already done, are you saying my cherry-picks were wrong?

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-18 20:34:11

Oh, I hadn't looked at them. If you ended up putting them into the right places (it's pretty obvious where they go), then you're good. :-)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-18 20:35:56

Only applying to 1.2.11 gave me problems, but it looks right to me. Do a "git pull" and check :-)

@389-ds-bot
Copy link
Author

Comment from djpadz at 2018-06-18 20:37:42

Just did a spot-check, and it looks good :-)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-18 20:39:56

Thanks again for the contribution!!

@389-ds-bot
Copy link
Author

Patch
49781.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Migration flag - PR pr Migration flag - PR
Projects
None yet
Development

No branches or pull requests

1 participant