<div dir="ltr"><div dir="ltr">On Wed, Dec 4, 2019 at 5:05 AM Jaco Kroon <<a href="mailto:jaco@uls.co.za">jaco@uls.co.za</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  

    
  
  <div>
    <p>Hi All,</p>
    <p>In ast_apply_acl (main/acl.c) there is two lines that's issuing a
      LOG_WARNING when an ACL gets denied.</p>
    <p>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.</p>
    <p>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.</p>
    <p>As I see it, there are two possible solutions:</p>
    <p>Solution 1:<br>
    </p>
    <p>1.  Add AST_SENSE_INVALID as a possible return.<br>
      2.  Rename the current function to ast_apply_acl_(silent|nolog),
      and remove the logging.<br>
      3.  Add a replacement ast_apply_acl function which will generate
      the log entries as per current.</p>
    <p>Solution 2:<br>
    </p>
    <p>Simply don't log at all if the purpose argument is NULL.</p>
    <p>Solution two is the simpler fix, but it's probably also the less
      ideal one.</p>
    <p>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.</p>
    <p>Would dearly like some opinions on the matter.<br>
    </p>
    <p>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<br>
    </p>
    </div></blockquote></div><div><br></div>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.<br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div style="font-family:tahoma,sans-serif"><font color="#073763">Joshua C. Colp</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Senior Software Developer</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Sangoma Technologies</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Check us out at <a href="http://www.sangoma.com" target="_blank">www.sangoma.com</a> and <a href="http://www.asterisk.org" target="_blank">www.asterisk.org</a></font><br></div></div></div></div></div></div></div></div></div>