[asterisk-dev] [Code Review] 2987: ARI: Don't leak information about implementation details

[opticron]

> On Nov. 7, 2013, 3:58 p.m., David Lee wrote:
> > branches/12/main/rtp_engine.c, lines 1839-1842
> > <https://reviewboard.asterisk.org/r/2987/diff/2/?file=48011#file48011line1839>
> >
> >     Just a guess, but I think you'd want to drop the RTCP report completely if the associated channel was filtered out by the sanitizer.

This will pass a NULL into the ast_json_pack causing it to fail and return NULL if the channel snapshot is sanitized away.


> On Nov. 7, 2013, 3:58 p.m., David Lee wrote:
> > branches/12/main/stasis_bridges.c, lines 333-337
> > <https://reviewboard.asterisk.org/r/2987/diff/2/?file=48012#file48012line333>
> >
> >     Would I be right in assuming that if either bridge was filtered out, then ast_json_pack() returns NULL, effectively filtering out the message?
> >     
> >     If so, I would rather be explicitly return NULL when either snapshot is NULL. The underlying Jansson json_pack() behavior is undocumented, and could easily change in a future release.
> >     
> >     If not, then you'll end up with messages that effectively have missing fields, and it would be better to filter them out entirely.
> >     
> >     Either way, this (and several other to_json methods below) need an explicit check to filter them out if one of their fields were null.

Currently, bridges can not be entirely filtered out since the entity still exists if all channels get removed via sanitization (though there's the possibility of adding bridge sanitization at a later time). I'll go ahead and convert usage to explicit checks for channels, bridges, and endpoints.


- opticron


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


On Nov. 6, 2013, 1:46 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2987/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 1:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22744
>     https://issues.asterisk.org/jira/browse/ASTERISK-22744
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change prevents channels used as implementation details from leaking out to ARI. It does this by preventing creation of JSON blobs of channel snapshots created from those channels and sanitizing JSON blobs of bridge snapshots as they are created. This introduces a framework for excluding information from output targeted at Stasis applications on a consumer-by-consumer basis using channel sanitization callbacks which could be extended to bridges or endpoints if necessary.
> 
> This results in NULL inputs to ast_json_pack calls which generate unhelpful error messages, so that has been dealt with as well.
> 
> This also corrects a bug I noticed while investigating the issue where BridgeCreated events would not be created.
> 
> 
> Diffs
> -----
> 
>   branches/12/res/stasis/app.c 402347 
>   branches/12/res/res_stasis.c 402347 
>   branches/12/res/ari/resource_endpoints.c 402347 
>   branches/12/res/ari/resource_channels.c 402347 
>   branches/12/res/ari/resource_bridges.c 402347 
>   branches/12/main/stasis_message.c 402347 
>   branches/12/main/stasis_endpoints.c 402347 
>   branches/12/main/stasis_channels.c 402347 
>   branches/12/main/stasis_bridges.c 402347 
>   branches/12/main/rtp_engine.c 402347 
>   branches/12/main/json.c 402347 
>   branches/12/include/asterisk/stasis_endpoints.h 402347 
>   branches/12/include/asterisk/stasis_channels.h 402347 
>   branches/12/include/asterisk/stasis_bridges.h 402347 
>   branches/12/include/asterisk/stasis_app.h 402347 
>   branches/12/include/asterisk/stasis.h 402347 
> 
> Diff: https://reviewboard.asterisk.org/r/2987/diff/
> 
> 
> Testing
> -------
> 
> Manual testing with bridges and channels.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131108/e0d3d6f9/attachment-0001.html>