[asterisk-dev] [Code Review]: Generate security events in chan_sip using new Security Events Framework

[jrose]

> On Sept. 20, 2011, 12:19 p.m., jrose wrote:
> > /branches/10/configs/logger.conf.sample, lines 112-116
> > <https://reviewboard.asterisk.org/r/1362/diff/2/?file=20759#file20759line112>
> >
> >     The comments about what 'security' is don't really belong here I think.  You would put a mention of 'security' being available above this is here:
> >     
> >     ; Format is "filename" and then "levels" of debugging to be included:
> >     ;    debug
> >     ;    notice
> >     ...
> >     ;    fax
> >     +;    security
> >     
> >     Leaving the sample logging configuration though seems pretty appropriate to me.  I'd go ahead and move it to be among the others though... probably right after debug => debug, so
> >     
> >     ;debug => debug
> >     +;securitylog => security
> >     console => notice,warning,error
> >     
> >     At the moment we don't really describe what any of these loggable in here actually are... aside from suggesting that having debug logged can make logs hard to read and explode in size.  I'm thinking that might just be something that is supposed to be in the wiki.
> 
> Paul Belanger wrote:
>     I'd rather see:
>     
>     security => security
>     
>     keeping it inline with the other log files.
> 
> jrose wrote:
>     Yeah, actually I agree.  Calling it 'log' when it's just going to go into the log directory anyway is a little redundant and we don't do that for any of the other logs.
> 
> elguero wrote:
>     Sure, that can be done.  I was just following the wiki page on Asterisk Security Event Logger and in fact I have it wrong.  The wiki shows "security_log => security".  We should probably make sure to update the wiki page to match and while we are at it I saw some other areas that should be updated in the wiki when this code goes in.
>     
>     Just a side note: the logger level of "security" is only valid if the module res_security_log is in use.  This is a dynamic level that gets registered with that module.  That was why I had put the extra comments in.  I just wanted to make sure we were aware of this before making this logger level look like a built-in level.

Hmmm.  Actually, I agree that the dynamic levels should be separate from the static levels.  However, the precedent you are working with right now actually doesn't do that, and fax is listed right there with the other modules.  My suggestion for right now is to leave it as-is in your new patch.  I've already asked about separating the dynamic levels from the static levels (as well as describing the module that registers the level) in the sample and it's been approved, but I'll handle that in a separate patch.


- jrose


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


On Sept. 21, 2011, 8:25 a.m., elguero wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1362/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2011, 8:25 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Security Events Framework was added in 1.8 and support was added for AMI to generate events at that time.
> 
> This patch attempts to add support in chan_sip to generate security events.  Hopefully we can get this into Asterisk 10.
> 
> I am looking forward to hearing feedback on where this patch can be improved especially from those who have an intimate knowledge of chan_sip.
> 
> Thanks
> 
> 
> This addresses bug ASTERISK-18264.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18264
> 
> 
> Diffs
> -----
> 
>   /branches/10/CHANGES 337324 
>   /branches/10/channels/chan_sip.c 337324 
>   /branches/10/channels/sip/include/sip.h 337324 
>   /branches/10/configs/logger.conf.sample 337324 
>   /branches/10/include/asterisk/event_defs.h 337324 
>   /branches/10/include/asterisk/security_events_defs.h 337324 
>   /branches/10/main/event.c 337324 
>   /branches/10/main/security_events.c 337324 
> 
> Diff: https://reviewboard.asterisk.org/r/1362/diff
> 
> 
> Testing
> -------
> 
> Local dev machine and a softphone.  Generated events by using the wrong username, wrong password, wrong auth name, successful authentication.
> 
> 
> Thanks,
> 
> elguero
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110921/bbb4e9fa/attachment-0001.htm>