[asterisk-dev] [Code Review]: named_acls: Named ACLs - a system for creating and applying ACLs with named profiles which can be shared

jrose reviewboard at asterisk.org
Mon Jun 11 13:33:48 CDT 2012



> On June 11, 2012, 12:54 p.m., Paul Belanger wrote:
> > /trunk/configs/acl.conf.sample, lines 37-44
> > <https://reviewboard.asterisk.org/r/1978/diff/1/?file=28608#file28608line37>
> >
> >     I'm curious why you choose to use 'back to back' contents, rather then allowing templating.
> >     
> >     To me, it makes more sense to build up your settings in templates, rather then appending each new value in the config file. Since other asterisk configs allow you to do this.
> >     
> >     EG: 
> >     [bad_combination_part1](!)
> >     ;deny=0.0.0.0/0.0.0.0
> >     ;permit=10.24.20.1/255.255.255.0
> >     
> >     ;[bad_combination)part2](bad_combination_part1)
> >     ;deny=0.0.0.0/0.0.0.0
> >     ;permit=10.24.20.2/255.255.255.0
> >     
> >     Also, does the following work?
> >     
> >     [bad_acl]
> >     ;deny=0.0.0.0/0.0.0.0
> >     ;permit=10.24.20.2/255.255.255.0
> >     
> >     [bad_acl](+)
> >     permit=192.168.1.100/255.255.255.255
> 
> jrose wrote:
>     I actually have no idea what all it supports since much of the functionality comes from the new configuration framework Terry made.
>     
>     I tried this out though:
>     
>     [bad_acl]
>     deny=0.0.0.0/0.0.0.0
>     permit=10.24.20.2/255.255.255.0
>     
>     [bad_acl](+)
>     permit=192.168.1.100/255.255.255.255
>     
>     
>     And got the following ACL:
>     
>     ACL: bad_acl
>     ---------------------------------------------
>       0:  deny - 0.0.0.0/0.0.0.0
>       1: allow - 10.24.20.0/255.255.255.0
>       2: allow - 192.168.1.100/255.255.255.255
>     
>     
>     So if that's what's supposed to happen, cool.
>     
>     
>     
>     And I tried this one as well:
>     [bad_combination_part1](!)
>     deny=0.0.0.0/0.0.0.0
>     permit=10.24.20.1/255.255.255.0
>     
>     [bad_combination_part2](bad_combination_part1)
>     deny=0.0.0.0/0.0.0.0
>     permit=10.24.20.2/255.255.255.0
>     
>     And what ended up happening is bad_combination_part1 didn't get created while bad_combination_part2 was a combined ACL like so:
>     
>     ACL: bad_combination_part2
>     ---------------------------------------------
>       0:  deny - 0.0.0.0/0.0.0.0
>       1: allow - 10.24.20.0/255.255.255.0
>       2:  deny - 0.0.0.0/0.0.0.0
>       3: allow - 10.24.20.0/255.255.255.0
>     
>     I'm not a configuration expert (most of the ones I work with are as simple as I can make them), but I'm guessing that is what's supposed to happen.
>     
>     The reason I went with back to back contents is because that's the most minimally invasive way to implement this feature, and minimal invasiveness is one of the things I've been going for with the Asterisk 11 features I'm working on. Also having more named profiles with the full contents of every ACL means more memory usage, but that's a fairly minor concern seeing as uses of named_ACLs involve duplicated the ast_ha's associated with them anyway. Not sure if that's going to change with the next few steps yet.
>     
>     If the above looks like the right behavior by the way, I could go ahead and add these examples to my sample configuration.

Actually, the second example is nicer when not using the bad masks that mmichelson mentioned before:

[bad_combination_part1](!)
deny=0.0.0.0/0.0.0.0
permit=10.24.20.1/255.255.255.255

[bad_combination_part2](bad_combination_part1)
deny=0.0.0.0/0.0.0.0
permit=10.24.20.2/255.255.255.255

ACL: bad_combination_part2
---------------------------------------------
  0:  deny - 0.0.0.0/0.0.0.0
  1: allow - 10.24.20.1/255.255.255.255
  2:  deny - 0.0.0.0/0.0.0.0
  3: allow - 10.24.20.2/255.255.255.255


- jrose


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


On June 8, 2012, 2:04 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1978/
> -----------------------------------------------------------
> 
> (Updated June 8, 2012, 2:04 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, Terry Wilson, and Olle E Johansson.
> 
> 
> Summary
> -------
> 
> This feature is based on oej's deluxepine (or something like that) branch with a similarly named feature.  ACLs are defined in acl.conf and can be used by pretty much anything that has ACL options permit/deny (acl='aclname').  acl= works similarly to permit= and deny= in that it simply appends to the working ACL, so they can be combined with other uses of permit/deny/acl.
> 
> Also in use in this patch are twilson's new config options.
> 
> Since named acls are duplicated when used in another configuration, configurations that use named acls need to be updated if acl.conf is reloaded. This is accomplished with a new event type and the consumption of that event is demonstrated currently only in manager.conf
> If this seems like a proper approach to this problem, that will be replicated across other consumers of named acls.
> 
> NOTE: This code is very much WIP and not intended for merging.
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 368662 
>   /trunk/channels/chan_h323.c 368662 
>   /trunk/channels/chan_iax2.c 368662 
>   /trunk/channels/chan_mgcp.c 368662 
>   /trunk/channels/chan_sip.c 368662 
>   /trunk/channels/chan_skinny.c 368662 
>   /trunk/channels/chan_unistim.c 368662 
>   /trunk/configs/acl.conf.sample PRE-CREATION 
>   /trunk/configs/iax.conf.sample 368662 
>   /trunk/configs/manager.conf.sample 368662 
>   /trunk/configs/sip.conf.sample 368662 
>   /trunk/configs/skinny.conf.sample 368662 
>   /trunk/include/asterisk/acl.h 368662 
>   /trunk/include/asterisk/event_defs.h 368662 
>   /trunk/main/acl.c 368662 
>   /trunk/main/asterisk.c 368662 
>   /trunk/main/manager.c 368662 
>   /trunk/main/named_acl.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1978/diff
> 
> 
> Testing
> -------
> 
> Various tests for configuring and using named acls were performed, and a task for writing comprehensive testsuite tests is in the queue.  Additionally, various means of reloading the configuration have been performed, and so far they pan out aside from a bug with an unchanged acl.conf which is a generic problem against config options accidentally introduced a little while back.
> 
> 
> Thanks,
> 
> jrose
> 
>

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


More information about the asterisk-dev mailing list