[Home]

Summary:ASTERISK-24780: [patch] - Buddies are always auto-registered when processing the roster
Reporter:Simon Arlott (lp0)Labels:
Date Opened:2015-02-11 14:01:00.000-0600Date Closed:2015-03-25 10:30:42
Priority:MajorRegression?Yes
Status:Closed/CompleteComponents:Resources/res_xmpp
Versions:11.16.0 13.1.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-13.1.0-24780.patch
Description:In both xmpp_roster_hook and xmpp_client_create_buddy, it ignores the XMPP_AUTOREGISTER setting and sets "buddy->subscribe = 1" if there is no subscription.

This is a regression as it was already fixed in ASTERISK-14233. It is extremely inconvenient to have Asterisk send "Greetings! I am the Asterisk Open Source PBX and I want to subscribe to your presence" to everyone in your contact list.

I am sharing my own XMPP account with Asterisk (at a negative priority) and this needs to be a recognised use case where Asterisk should not do inappropriate things.
Comments:By: Matt Jordan (mjordan) 2015-02-11 14:15:29.125-0600

Actually, the code in {{res_xmpp}} is already attempting to honor the autoregister feature:

{code}
/* Determine if we need to subscribe to their presence or not */
if (!iks_strcmp(iks_find_attrib(item, "subscription"), "none") ||
   !iks_strcmp(iks_find_attrib(item, "subscription"), "from")) {
buddy->subscribe = 1;
} else {
buddy->subscribe = 0;
}
{code}

We then invoke an {{ao2_callback}} only if {{autoregister}} is enabled:

{code}
/* If autoregister is enabled we need to go through every buddy that we need to subscribe to and do so */
if (ast_test_flag(&clientcfg->flags, XMPP_AUTOREGISTER)) {
ao2_callback(client->buddies, OBJ_NODATA | OBJ_MULTIPLE, xmpp_client_subscribe_user, client);
}
{code}

Which actually does the subscription.

Note that {{autoregister}} is on by default, and is configured in the {{general}} section. Can you attach your {{xmpp.conf}}, as well as a DEBUG log with XMPP debugging enabled?

By: Simon Arlott (lp0) 2015-02-11 14:42:36.394-0600

{code}
[general]
debug=no                                ;;Turn on debugging by default.
autoprune=no                            ;;Auto remove users from buddy list.
autoregister=no                         ;;Auto register users from buddy list.
;autoaccept=no
auth_policy=deny

[asterisk]
type=client
serverhost=...
username=...
secret=...
priority=-100
status=xaway
statusmessage="..."
{code}

I can't now see where in the code Asterisk is making the subscription requests from, but I've always had autoregister=no set in the global section:
^9a6a589 jabber.conf (proxima.lp0.eu 2010-07-13 20:30:35 +0100  4) autoregister=no                              ;;Auto register users from buddy list.

After upgrading from 1.6 there was an "autoaccept=no" line but that appears to cause the module to not load:
{code}
10 19:46:39.703613 VERBOSE[8919]: config.c:2045 in config_text_file_load: Parsing '/etc/asterisk/xmpp.conf': Found
10 19:46:39.703807 ERROR[8919]: config_options.c:483 in process_category: In xmpp.conf: Processing options for general failed
{code}

I can't provide a DEBUG log with XMPP debugging, it'd take too long to sanitise. I've removed the offending code from res_xmpp.c.

By: Simon Arlott (lp0) 2015-02-11 14:54:59.381-0600

{code}
Breakpoint 2, xmpp_roster_hook (data=0xb4a37bec, pak=0x88721b4) at res_xmpp.c:2374
(gdb) p clientcfg->flags
$1 = {flags = 182}
{code}

These flags correspond to XMPP_AUTOREGISTER | XMPP_AUTOACCEPT | XMPP_USETLS | XMPP_USESASL | XMPP_KEEPALIVE, so none of the changes to the global settings are being applied to the client settings.


If I put a breakpoint at the end of xmpp_client_config_post_apply, I get the default flags:
{code}
(gdb) p ((struct ast_xmpp_client_config *)obj)->flags->flags
$42 = 182
{code}
unless I explicitly set a flag in the client section, and then:
{code}
(gdb) p ((struct ast_xmpp_client_config *)obj)->flags->flags
$43 = 180
{code}

By: Simon Arlott (lp0) 2015-02-14 10:09:35.022-0600

I can change the {{aco_option_register_custom}} parameters for the {{client_options}} that should be inherited from the {{global_options}} to remove the default value, but there doesn't appear to be a way to load the {{global_options}} first and then use them to initialise the {{client_options}}. It looks like using {{item_pre_process}} would only work if the config file had the {{general}} section first.

Alternatively, the {{client_options}} would need to keep track of which flags had been modified and use that to merge them with the {{global_options}} in {{xmpp_client_config_post_apply}}.


By: Simon Arlott (lp0) 2015-02-14 12:12:32.591-0600

The attached patch fixes this issue by correctly merging the XMPP_AUTO* client options with the defaults and the global options.

Note: {{debug=yes}} in {{global_option}} continues to override {{debug=no}} in {{client_option}}.

By: Matt Jordan (mjordan) 2015-02-16 12:18:04.733-0600

Copying over the flags in the {{xmpp_client_config_post_apply}} looks correct to me. Thanks for the patch!

If you'd like this to get merged over faster, feel free to put the patch up for peer review:

https://wiki.asterisk.org/wiki/display/AST/Code+Review

By: Matt Jordan (mjordan) 2015-03-14 10:22:37.457-0500

I went and threw this up for code review at https://reviewboard.asterisk.org/r/4496/. While the patch is pretty straight forward, I wanted to get another pair of eyes on it.

Thanks for the contribution and fixing this regression!