[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:24:39 CDT 2012



> On June 11, 2012, 12:54 p.m., Paul Belanger wrote:
> > /trunk/configs/acl.conf.sample, line 42
> > <https://reviewboard.asterisk.org/r/1978/diff/1/?file=28608#file28608line42>
> >
> >     bad_combination_part2

Yeah, I caught that one a while back.  Thanks though.


> 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

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.


> On June 11, 2012, 12:54 p.m., Paul Belanger wrote:
> > /trunk/configs/acl.conf.sample, lines 20-23
> > <https://reviewboard.asterisk.org/r/1978/diff/1/?file=28608#file28608line20>
> >
> >     It would be good to also implement Tilghman's '!' negation element to negate acls (like he did with codecs[1]).
> >     
> >     ;permit=!all_acls,local_acls
> >     
> >     [1] http://svnview.digium.com/svn/asterisk?view=revision&revision=334574

I'm not sure on this one. What does that mean exactly?

Would that be like... if I have two ACLs where one is a subset of the other that some of the settings would be removed (which I don't think would work)? or that the permits/denies would be inverted (simple enough)?

Also, the option in the current form would be
acl=!all_acls
acl=local_acls

In the places I've attached this, I haven't included any support for comma separated items.


- 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/7d48dc85/attachment-0001.htm>


More information about the asterisk-dev mailing list