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

[jrose]

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

(Updated June 26, 2012, 12:25 p.m.)


Review request for Asterisk Developers, Mark Michelson, Terry Wilson, and Olle E Johansson.


Changes
-------

The whole concept has changed somewhat in the latest patch.

ACLs can be represented on a consumer by a linked list container which stores both regular ACLs specified with permit/deny as well as named ACLs. If a regular ACL is contained, it will simply be an unnamed ACL node which is the head element of the list. All named ACLs are separate elements.

Rather than building a compound ast_ha based on the order of specification of ACL, Permit, and Deny, Named ACLs are evaluated completely separately from the regular ACL when ast_appply_acl (similar to ast_apply_ha) is used. Each ACL must pass independently in order for the ACL list to return AST_SENSE_ALLOW. If any individual ACL in the list returns AST_SENSE_DENY from ast_apply_ha, ast_apply_acl will return AST_SENSE_DENY.

Some effort has been made to ensure that named ACLs will be easy to modify (add, change rules for, and delete) and have those modifications be spreadable to their consumers, but so far there are no actual methods of manipulation (such as through CLI or AMI) in the code yet since we can't ensure persistent storage of those changes. This may change after realtime is added, but for now the priority it simply to guarantee that making these changes won't require significant reworking of the named ACL subsystem.

Right now, only one consumer demonstrates use of the named ACL subsystem, and that is manager.c
Like before, it reacts very simply to changes. It subscribes to named ACl change events, and when it receives one, it forcibly reloads it's config.  It isn't elegant, but since the only way to force those changes right now is to reload the named ACL configuration, there isn't a great deal of need for an elegant solution yet.

When those changes are needed though, I've ensured that We can include specific items to react to changes in our subscriptions and also that the events are able to send specific names of ACLs that have been modified. This means enabling a consumer to make changes to a specific ACL container or even just a specific named ACL will be quite simple and there are unused functions readyf or that purpose.

There is one small issue that I think needs to be addressed though... see below.


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 (updated)
-----

  /trunk/CHANGES 369406 
  /trunk/configs/acl.conf.sample PRE-CREATION 
  /trunk/configs/manager.conf.sample 369406 
  /trunk/include/asterisk/acl.h 369406 
  /trunk/include/asterisk/event_defs.h 369406 
  /trunk/main/acl.c 369406 
  /trunk/main/asterisk.c 369406 
  /trunk/main/loader.c 369406 
  /trunk/main/manager.c 369406 
  /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/20120626/db339913/attachment-0001.htm>