[asterisk-dev] [Code Review] 2998: Push Security Events over AMI

[jrose]

> On Nov. 6, 2013, 7:32 p.m., Mark Michelson wrote:
> > Excellent work on this one!
> > 
> > I would suggest making a modification to the manager.conf.sample file and CHANGES file to indicate that the "security" class of events has been added.

Oh, right, good idea.


> On Nov. 6, 2013, 7:32 p.m., Mark Michelson wrote:
> > /branches/12/main/security_events.c, line 102
> > <https://reviewboard.asterisk.org/r/2998/diff/2/?file=48004#file48004line102>
> >
> >     This should not be required.

Yeah, I thought that might be the case.


> On Nov. 6, 2013, 7:32 p.m., Mark Michelson wrote:
> > /branches/12/include/asterisk/security_events.h, line 96
> > <https://reviewboard.asterisk.org/r/2998/diff/2/?file=48001#file48001line96>
> >
> >     The name of this function is a bit on the awkward side. Why is it called this?

It's called that because it's a secondary initialization function that must take place after the initialization of manager (more specifically, once its stasis topic has been created). I'm all ears on a better name for it... or just making the decision to delay the initialization of the security events framework until after manager is initialized. It probably doesn't matter since either way, nothing is logging the security events until either the forwarder is initialized or else a resource that subscribes to the security event stasis messages is loaded... and since this occurs right after manager, it's probably going to be the forwarder.


- jrose


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


On Nov. 6, 2013, 5:54 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2998/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 5:54 p.m.)
> 
> 
> Review request for Asterisk Developers, kmoore, Matt Jordan, and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Adds a to AMI handler for security event messages. I'm not 100% sure if it's proper to initialize the message forwarder in a secondary initialization step like I am, but I wasn't sure if I should move the initialization of the entire security event framework to after manager initializes instead.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/security_events.c 402500 
>   /branches/12/main/manager.c 402500 
>   /branches/12/main/asterisk.c 402500 
>   /branches/12/include/asterisk/security_events.h 402500 
>   /branches/12/include/asterisk/manager.h 402500 
> 
> Diff: https://reviewboard.asterisk.org/r/2998/diff/
> 
> 
> Testing
> -------
> 
> Tested that security events get logged over AMI.  Tested that obvious memory leaks against strings and the security event system aren't occurring with MALLOC_DEBUG.
> 
> 
> Thanks,
> 
> jrose
> 
>

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