[asterisk-dev] [Code Review] 2452: Fix Distributed Device State broken at sites using res_xmpp or res_jabber where Secuity Advisory AST-2012-015 is inplace.

[Alec Davis]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2452/
-----------------------------------------------------------

(Updated April 16, 2013, 9:21 p.m.)


Review request for Asterisk Developers.


Changes
-------

Addressed Matts request for a comment as to why I'm not using ast_event_get_ie_uint.

A much more intrusive fix would be to change the order of the ast_devstate_cache enumerations
enum ast_devstate_cache {
        AST_DEVSTATE_CACHABLE,      /*!< This device state is cachable, DEFAULT */
        AST_DEVSTATE_NOT_CACHABLE,  /*!< This device state is not cachable */
};

Instead of any tests like
if (cachable) {
  ...
} else {
 ..
}

The tests become
if (cachable==AST_DEVSTATE_CACHABLE) {
  do cache routines...
} else {
 ..
}

Sending the IE 'cachable' in jabber and xmpp, would require logic inversion, so that on the net <state ... cachable="1"> is still emitted, even though the value of cachable is 0 or AST_DEVSTATE_CACHABLE.

Or ast_event_get_ie_uint is passed an additional parameter
'unsigned defaultval' where the value of 'defaultval' is the returned if the IE isn't found.


Bugs: ASTERISK-21429
    https://issues.asterisk.org/jira/browse/ASTERISK-21429


Repository: Asterisk


Description
-------

The site that doesn't update it's devicestates from the remotesite is the site with ASR-2012-015 inplace.

This fixes the scenario where one site doesn't send the IE cachable=1 in the XMPP/Jabber message, to a newer system that expects to see it.

If the IE 'cachable' isn't present we need to default to AST_DEVSTATE_CACHABLE.
 Assigning the cachable variable directly from ast_event_get_ie_uint is wrong, if the IE 'cachable' isn't found ast_event_get_ie_uint() will return 0, 0 = AST_DEVSTATE_NOT_CACHABLE

We need to read the 'cachable' IE into a string first, allowing us to check that it exists, if it doesn't exist then default to cachable=AST_DEVSTATE_CACHABLE.
 If the 'cachable' IE does exist, then convert to a uint, validate value and go with it.


Caused by:
https://issues.asterisk.org/jira/browse/ASTERISK-20175


Diffs (updated)
-----

  branches/11/main/devicestate.c 385594 
  branches/11/res/res_jabber.c 385594 
  branches/11/res/res_xmpp.c 385594 

Diff: https://reviewboard.asterisk.org/r/2452/diff/


Testing
-------

Yes, change device state, at either site, and it's reflected in the other. One site is prior to ASTERISK-21075.

Tested both sites at r385594, and states correct at both sites.


Thanks,

Alec Davis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130416/41a60320/attachment.htm>