Ejabberd process crashes while user trying to update VCard whith LDAP backend

FiXXXeR78 commented 3 weeks ago

Environment

ejabberd version: 21.12
Erlang version: Erlang (SMP,ASYNC_THREADS) (BEAM) emulator version 13.2
OS: Linux (ALTLinux)
Installed from: distro package

Configuration


mod_vcard:
    db_type: ldap
    search: true
    allow_return_all: true
    matches: 120
    ldap_base: "ou=Accounts,dc=domain,dc=tld"
    ldap_filter: "(&(objectClass=inetOrgPerson)(pager=*jabber*)(!(postOfficeBox=*hidden*)))"
    ldap_vcard_map:
      "NICKNAME": {"%s": ["cn"]}
      "FN": {"%s": ["cn"]}
      "GIVEN": {"%s": ["givenName"]}
      "MIDDLE": {"%s": ["initials"]}
      "FAMILY": {"%s": ["sn"]}
      "EMAIL": {"%s": ["mail"]}
      "ORGUNIT": {"%s": ["ou"]}
      "ORGNAME": {"%s": ["o"]}
      "TITLE": {"%s": ["title"]}
      "TEL": {"%s": ["telephoneNumber"]}
      "PHOTO": {"%s": ["jpegPhoto"]}
    ldap_search_fields:
      "JID": "%u"
      "Full Name": "cn"
      "Name": "givenName"
      "Surname": "sn"
      "E-Mail": "mail"
    ldap_search_reported:
      "Full Name": "FN"
      "Nickname": "NICKNAME"
      "Surname": "FAMILY"
      "Name": "GIVEN"
      "Middle Name": "MIDDLE"
      "Phone": "TEL"
      "Department": "ORGUNIT"
      "Organisation": "ORGNAME"

Errors from error.log/crash.log

2024-08-15 17:51:23.914065+03:00 [error] <0.7890.0>@ejabberd_hooks:safe_apply/4:240
 Hook vcard_iq_set crashed when running mod_vcard:vcard_iq_set/1:
** exception error: no case clause matching {error,not_implemented}
   in function  mod_vcard:vcard_iq_set/1 (src/mod_vcard.erl, line 395)
   in call from ejabberd_hooks:safe_apply/4 (src/ejabberd_hooks.erl, line 236)
   in call from ejabberd_hooks:run_fold1/4 (src/ejabberd_hooks.erl, line 217)
   in call from mod_avatar:set_vcard_avatar/3 (src/mod_avatar.erl, line 378)
   in call from ejabberd_hooks:safe_apply/4 (src/ejabberd_hooks.erl, line 236)
   in call from ejabberd_hooks:run1/3 (src/ejabberd_hooks.erl, line 203)
   in call from mod_pubsub:publish_item/8 (src/mod_pubsub.erl, line 1892)
   in call from mod_avatar:publish_avatar/5 (src/mod_avatar.erl, line 275)
** Arg 1 = {iq,<<"14552188624474495279">>,set,<<>>,
.... user's VCard data here ....
               #{}}

Bug description

When mod_vcard configured to use LDAP backend, ejabberd processes crashes if user tries to update its VCard info. In mod_vcard.erl function vcard_iq_set calls set_vcard from mod_vcard_ldap.erl which returns not_implemented. vcard_iq_set does not know what to deal with it (it knows only ok and badarg return values), and process crashed. We need to correctry handle not_implemented return value.

In current 24.07 release the code looks the same, so it will behave as tested 21.12 code.

badlop commented 3 weeks ago

Looking at 3.2 Updating One's vCard it mentions:

If a user attempts to perform an IQ set on another user's vCard (i.e., by setting a 'to' address to a JID other than the sending user's bare JID), the server MUST return a stanza error, which SHOULD be or .

This is not the same case... but not-allowed looks a reasonable error response:

The recipient or server does not allow any entity to perform the action (e.g., sending to entities at a blacklisted domain); the associated error type SHOULD be "cancel".

The ejabberd response could be like this, right?

<iq xml:lang='es'
    to='test6000@localhost/tka1'
    from='test6000@localhost'
    type='error'
    id='30:478714'>
  <vCard xmlns='vcard-temp'>
    <NICKNAME>my nick</NICKNAME>
  </vCard>
  <error type='cancel'>
    <not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
    <text xml:lang='en'
    xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'>Updating the vCard is not supported by the storage backend</text>
  </error>
</iq>

FiXXXeR78 commented 3 weeks ago

I'm not a XMPP Guru, but "not-allowed" looks quite reasonable for me. Thank you!

badlop commented 3 weeks ago

Ah, other possible error condition could be 8.3.3.3. feature-not-implemented

FiXXXeR78 commented 3 weeks ago

Will this simple patch on mod_vcard.erl fix the issue? mod_vcard.erl.patch

badlop commented 3 weeks ago

Yes, the implementation generally follows that, thanks! There were two problems that I've solved and committed.

Try it, it should work perfectly now :)

processone / ejabberd