[asterisk-dev] [Code Review] Security Event Framework Proposal

[Russell Bryant]

> On 2009-07-10 14:07:03, Mark Michelson wrote:
> > /trunk/doc/tex/security-events.tex, line 64
> > <https://reviewboard.asterisk.org/r/273/diff/7/?file=5703#file5703line64>
> >
> >     "can be configured"

Fixed


> On 2009-07-10 14:07:03, Mark Michelson wrote:
> > /trunk/doc/tex/security-events.tex, line 70
> > <https://reviewboard.asterisk.org/r/273/diff/7/?file=5703#file5703line70>
> >
> >     Either "the content ... is"
> 
>  wrote:
>     Ignore the word "either" there.

Fixed


> On 2009-07-10 14:07:03, Mark Michelson wrote:
> > /trunk/doc/tex/security-events.tex, line 192
> > <https://reviewboard.asterisk.org/r/273/diff/7/?file=5703#file5703line192>
> >
> >     "Call-ID"

Fixed


> On 2009-07-10 14:07:03, Mark Michelson wrote:
> > /trunk/tests/test_security_events.c, line 267
> > <https://reviewboard.asterisk.org/r/273/diff/7/?file=5712#file5712line267>
> >
> >     asdkl23478289lasdkf

Fixed


> On 2009-07-10 14:07:03, Mark Michelson wrote:
> > /trunk/doc/tex/security-events.tex, line 246
> > <https://reviewboard.asterisk.org/r/273/diff/7/?file=5703#file5703line246>
> >
> >     Get rid of the red splotch!!! Actually, there are others, too, but I won't bother to point them all out since Review Board does such a nice job of pointing this out for you.

Fixed, but this is the only one I see on new/changed lines.  Hopefully I'm not just blind, or my browser is displaying it differently or something ...


- Russell


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


On 2009-06-18 16:43:09, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/273/
> -----------------------------------------------------------
> 
> (Updated 2009-06-18 16:43:09)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a security framework for Asterisk.  This is essentially the documentation and implementation of the ideas discussed at a couple of the recent developer meetings.  The documentation resides in doc/tex/security-events.tex.
> 
> The code is an implementation of this framework.  The changes can be broken down in this way:
> 
>    1) Security event API
>       - main/security_events.c
>       - include/asterisk/security_events.h
>       - include/asterisk/security_events_defs.h
> 
>    2) Changes to the ast_event API to support security events
>       - include/asterisk/event_defs.h
>       - main/event.c
> 
>    3) A consumer of security events to produce a security log file
>       - res/res_security_log.c
> 
>    4) A completed producer of security events
>       - main/manager.c
> 
>    5) The beginning of having chan_sip produce some security events
>       ******************
>       *** NOTE: I do not propose that this part get merged now.  I think we
>       *** should handle the chan_sip mods as another patch in a second phase.
>       ******************
>       - channels/chan_sip.c
> 
>    6) A test module that generates every type of security event
>       - tests/test_security_events.c
> 
>    7) A simple test script that gets the manager interface to generate
>       one of every type of security event it emits.
>       **************
>       *** NOTE: Is this worth merging?
>       **************
>       - tests/test_ami_security_events.sh
> 
> 
> The security event API is essentially a helper API on top of the ast_event API.  I knew going in to this that there was a lot of data that we wanted in each event.  Forcing producers to use really big ast_event_new() calls, and forcing them to do the payload formatting seemed very error prone and more difficult then necessary.  So, I came up with this API that uses structure definitions of each event type and code in the core that converts these structures into events.  The code also detects if a producer of events forgot to fill in a field that was required.  Also, I put structure version fields in each of these helper structures for the sake of ABI protection.
> 
> As a final note, one notable feature that is not yet present is the ability to fire off custom security events from the dialplan.  I haven't come up with an interface for it that I am happy with just yet.  I think we can handle this as another patch later.
> 
> 
> Diffs
> -----
> 
>   /trunk/doc/tex/security-events.tex PRE-CREATION 
>   /trunk/doc/tex/asterisk.tex 201820 
>   /trunk/channels/chan_sip.c 201820 
>   /trunk/include/asterisk/event_defs.h 201820 
>   /trunk/include/asterisk/security_events.h PRE-CREATION 
>   /trunk/include/asterisk/security_events_defs.h PRE-CREATION 
>   /trunk/main/event.c 201820 
>   /trunk/main/manager.c 201820 
>   /trunk/main/security_events.c PRE-CREATION 
>   /trunk/res/res_security_log.c PRE-CREATION 
>   /trunk/tests/test_ami_security_events.sh PRE-CREATION 
>   /trunk/tests/test_security_events.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/273/diff
> 
> 
> Testing
> -------
> 
> There are two major components of the testing that has been done:
> 
>    1) There is a test module that generates every type of security event.  You can run the test CLI command and verify that the events come out in the custom security log channel, demonstrating the documented security log format:
> 
> Here is some example output:
> 
> *CLI> securityevents test generation
> 
> ...
> 
> SECURITY[17921]: res_security_log.c:125 security_event_cb: SecurityEvent="FailedACL",Service="TEST",EventVersion="1",AccountID="Username",SessionID="Session123",LocalAddress="IPV4/UDP/192.168.1.1/12121",RemoteAddress="IPV4/UDP/192.168.1.2/12345",Module="test_security_events",ACLName="TEST_ACL",SessionTV="1244131376-695232"
> 
> ...
> 
>    2) There is also a script that gets the Asterisk Manager Interface to produce at least one of every type of security event that it produces.  This has been executed and the output has been verified to be what is expected.
> 
> 
> Thanks,
> 
> Russell
> 
>