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

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

Closed
FiXXXeR78 opened this issue Aug 16, 2024 · 5 comments
Closed

Comments

@FiXXXeR78
Copy link

FiXXXeR78 commented Aug 16, 2024

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 badlop self-assigned this Aug 16, 2024
@badlop
Copy link
Member

badlop commented Aug 16, 2024

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
Copy link
Author

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

@badlop
Copy link
Member

badlop commented Aug 16, 2024

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

@FiXXXeR78
Copy link
Author

FiXXXeR78 commented Aug 16, 2024

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

@badlop
Copy link
Member

badlop commented Aug 21, 2024

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

Try it, it should work perfectly now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants