[asterisk-dev] acl.c - option to turn off logging

[Kevin Harwell]

On Wed, Dec 4, 2019 at 5:05 AM Joshua C. Colp <jcolp at sangoma.com> wrote:

> On Wed, Dec 4, 2019 at 5:05 AM Jaco Kroon <jaco at uls.co.za> wrote:
>
>> Hi All,
>>
>> In ast_apply_acl (main/acl.c) there is two lines that's issuing a
>> LOG_WARNING when an ACL gets denied.
>>
>> The first happens if the ACL is invalid.  I'm not too worried about this
>> specific one, it's probably a good thing if this gets logged always.
>>
>> The latter, in the case of AST_SENSE_DENY is a bit problematic for me.
>> I've submitted patches now to use ACLs in res_rtp_asterisk, and with large
>> number of rejects this can quickly spam the logs, and frankly, confuse
>> consumers.
>>
>> As I see it, there are two possible solutions:
>>
>> Solution 1:
>>
>> 1.  Add AST_SENSE_INVALID as a possible return.
>> 2.  Rename the current function to ast_apply_acl_(silent|nolog), and
>> remove the logging.
>> 3.  Add a replacement ast_apply_acl function which will generate the log
>> entries as per current.
>>
>> Solution 2:
>>
>> Simply don't log at all if the purpose argument is NULL.
>>
>> Solution two is the simpler fix, but it's probably also the less ideal
>> one.
>>
>> The adding of the AST_SENSE_INVALID will also mean that the replacement
>> function will need to rewrite AST_SENSE_INVALID => AST_SENSE_DENY, or we
>> need to audit all consumers of the function (there fortunately isn't a
>> great many of these) and wherever ast_apply_acl(...) == AST_SENSE_DENY is
>> found, it should be rewritten as ast_apply_acl(...) != AST_SENSE_ALLOW.
>>
>> Would dearly like some opinions on the matter.
>>
>> PS:  The advantage for me on using ACL over HA is simply the named ACL
>> functionality, so in rtp.conf I can state ice_acl = named_acl instead of
>> having to embed the ACL into rtp.conf
>>
>
> I would prefer #1 however the existing function should behave as it does
> now. Each consumer should not need to be touched, unless they are to be
> switched to silent. We have an obligation to maintain ABI and behavior of
> API functions as best we can in case there are any outside consumers as
> well.
>

Unfortunately due to how consumer possibly use of the current return value
I think the only way to handle this is with #2. Or well by adding another
function ast_apply_acl_silent, and removing the logging that is. Your new
changes could call this function, and nothing else would be potentially
affected.

-- 
Kevin Harwell
Senior Software Developer
Sangoma Technologies
Check us out at: https://sangoma.com & https://asterisk.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20191204/9d950c8b/attachment.html>