Privacy Manager suggestion

I put this in the beta forum during the beta, but did not receive any feedback on it, so I thought I would try again here. I have added this to my privacy-mgr section, but it is overwritten when changes are made in the UI. It would be nice if this were a part of the product so this change was not lost.


I have spent some time today trying to get the privacy manager to work for our line(s). I have taken a look at the information in this thread and applied the above patch to extensions.conf. However, my problem is with the logic. Maybe I am missing something, but the logic to take the callerid number and add one (excluding the + at the start of the number) seems inadequate. The line I am talking about are here:

exten => s,n(TESTRESULT),GotoIf($[“foo${TESTCID}”=“foo”]?CLEARCID:PRIVMGR)

This line assumes that the addition of 1 to the CALLERID(num) will not have any effect, resulting in an empty string if the caller ID is missing? Since the CALLERID(num) is zero in a lot of cases for a missing number, would it not make sense to also test for zero as a special case? I inserted the following line right after the KEEPCID= line to handle a caller ID number of zero.

exten => s,n,GotoIf($[${CALLERID(num)}=0]?CLEARCID)

With this line present, privacy manager is now catching calls with at least Private Number. Is there a reason that this check was not included in the code by default? Is this not a good check to include? Any insight would be very much appreciated. Thanks.

Mike

Well I have not looked at that code in a while so I’ll admit it is a bit ‘twisted’ however what I believe it is doing is simply stripping out any leading ‘+’ signs and then doing the math operation which will result in a blank callerid if it was non-numeric such as: “unknown” or “Private Party”. The reason it is doing that is because the PricacyManager() function is very ‘dumb’ if I recall correctly. It counts ‘characters’ so depending on your configuration, “Private Party” may be adequate for it to consider this ok. If a single digit like 0 is being transmitted, then you should make sure the settings in privacy.conf are adequate to filter it. The function still depends the number of digits being a minimum and in the default configuration, ‘0’ should not be adequate unless it is not functioning properly.

Philippe Lindheimer - FreePBX Project Lead
http//freepbx.org - IRC #freepbx

Well,

should work and does work presume that Asterisk is not flawed on this subject and from a couple of sanity checks followed by a little code checking Asterisk is in fact flawed or otherwise takes a different philosophy on this. If anything non-zero is passed into it, it will let it by. Only if the CID is completely missing will it then look at either the privacy.conf file or the arguments that are passed to it.

I took the liberty of making a couple of modifications to Asterisk that would make it work how I would expect. I have tested the 1.2 version and have not even tried to compile the 1.4 version but present it anyhow because it is clearly the same logic.

EDITED: well looking at the description, it looks like it is by design. It always allows the number to pass if any string is sent. So I guess that just implies we should be putting a new filter in FreePBX that lets you configure the privacy.conf parameters as well as if it should maintain this behavior.

However - the patch still seems ok if it is of interest to someone but no plans on submitting it to Asterisk as a bug.

Lightly tested Asterisk 1.2 version:

--- app_privacy.c.ORIGINAL  2007-09-16 13:19:03.000000000 -0700
+++ app_privacy.c 2007-09-16 14:12:53.000000000 -0700
@@ -100,19 +100,8 @@
  );

  LOCAL_USER_ADD (u);
- if (!ast_strlen_zero(chan->cid.cid_num)) {
-   if (option_verbose > 2)
-     ast_verbose (VERBOSE_PREFIX_3 "CallerID Present: Skipping\n");
- } else {
-   /*Answer the channel if it is not already*/
-   if (chan->_state != AST_STATE_UP) {
-     res = ast_answer(chan);
-     if (res) {
-       LOCAL_USER_REMOVE(u);
-       return -1;
-     }
-   }

+ /*------------------*/
    if (!ast_strlen_zero((char *)data))
    {
      parse = ast_strdupa(data);
@@ -161,6 +150,23 @@
          ast_log(LOG_WARNING, "Invalid min length argument\n");
      }
    }
+ /*--------------------------*/
+
+ /* changed from ast_strlen_zero() */
+ if (strlen(chan->cid.cid_num) >= minlength) {
+   if (option_verbose > 2)
+     ast_verbose (VERBOSE_PREFIX_3 "CallerID Present: Skipping\n");
+ } else {
+   /*Answer the channel if it is not already*/
+   if (chan->_state != AST_STATE_UP) {
+     res = ast_answer(chan);
+     if (res) {
+       LOCAL_USER_REMOVE(u);
+       return -1;
+     }
+   }
+
+   /*---CODE MOVED FROM HERE---*/

    /*Play unidentified call*/
    res = ast_safe_sleep(chan, 1000);

Untested and uncompiled Asterisk 1.4 version:

--- app_privacy.c.ORIGINAL  2007-09-16 14:22:49.000000000 -0700
+++ app_privacy.c 2007-09-16 14:21:21.000000000 -0700
@@ -96,19 +96,7 @@

  u = ast_module_user_add(chan);

- if (!ast_strlen_zero(chan->cid.cid_num)) {
-   if (option_verbose > 2)
-     ast_verbose (VERBOSE_PREFIX_3 "CallerID Present: Skipping\n");
- } else {
-   /*Answer the channel if it is not already*/
-   if (chan->_state != AST_STATE_UP) {
-     res = ast_answer(chan);
-     if (res) {
-       ast_module_user_remove(u);
-       return -1;
-     }
-   }
-
+ /*--------------------------*/
    if (!ast_strlen_zero(data)) {
      parse = ast_strdupa(data);

@@ -151,6 +139,23 @@
          ast_log(LOG_WARNING, "Invalid min length argument\n");
      }
    }
+ /*--------------------------*/
+
+ /* changed from ast_strlen_zero() */
+ if (strlen(chan->cid.cid_num) >= minlength) {
+   if (option_verbose > 2)
+     ast_verbose (VERBOSE_PREFIX_3 "CallerID Present: Skipping\n");
+ } else {
+   /*Answer the channel if it is not already*/
+   if (chan->_state != AST_STATE_UP) {
+     res = ast_answer(chan);
+     if (res) {
+       ast_module_user_remove(u);
+       return -1;
+     }
+   }
+
+   /*---CODE MOVED FROM HERE---*/

    /*Play unidentified call*/
    res = ast_safe_sleep(chan, 1000);

Philippe Lindheimer - FreePBX Project Lead
http//freepbx.org - IRC #freepbx

As much as I would like to try the patch, I cannot. Since I am using trixbox as my Asterisk solution, a build by me will break updates using the trixbox system. I was hoping for a solution on the FreePBX side since this I can update to my hearts content :). If you guys can find a solution for this issue it would be much appreciated. Thanks.

Mike

First to address your concern of the conf file being overwritten. Redefine the entire macro in extensions_custom.conf the way you want it and it will override what we ship.

The issue I have with adding that line is that it is a special case which I don’t know if it should be done for everyone or not. Since Privacy manager lets anything slip by and we are only filtering non-numeric data out of CALLERID(num) the appropriate answer would seem to be as follows:

  1. Create some GUI (well probably a module) to configure the PM settings (same settings you configure in the privacy.conf file but it will override those)
  2. Create the option to have it work as it does today or to have it effectively do what I did in the patch above and blank out the CID before passing to PM if too short.

This would not be too difficult but it is not necessarily high on my priority list either unless someone wants to pay for it to be done. But feel free to post it in as a feature request if it fits the bill of what is needed so it does not get lost.

Philippe Lindheimer - FreePBX Project Lead
http//freepbx.org - IRC #freepbx