chan_dahdi: Fix broken hidecallerid setting. (#101)

The hidecallerid setting in chan_dahdi.conf currently
is broken for a couple reasons.

First, the actual code in sig_analog to "allow" or "block"
Caller ID depending on this setting improperly used
ast_set_callerid instead of updating the presentation.
This issue was mostly fixed in ASTERISK_29991, and that
fix is carried forward to this code as well.

Secondly, the hidecallerid setting is set on the DAHDI
pvt but not carried forward to the analog pvt properly.
This is because the chan_dahdi config loading code improperly
set permhidecallerid to permhidecallerid from the config file,
even though hidecallerid is what is actually set from the config
file. (This is done correctly for call waiting, a few lines above.)
This is fixed to read the proper value.

Thirdly, in sig_analog, hidecallerid is set to permhidecallerid
only on hangup. This can lead to potential security vulnerabilities
as an allowed Caller ID from an initial call can "leak" into subsequent
calls if no hangup occurs between them. This is fixed by setting
hidecallerid to permcallerid when calls begin, rather than when they end.
This also means we don't need to also set hidecallerid in chan_dahdi.c
when copying from the config, as we would have to otherwise.

Fourthly, sig_analog currently only allows dialing *67 or *82 if
that would actually toggle the presentation. A comment is added
clarifying that this behavior is okay.

Finally, a couple log messages are updated to be more accurate.

Resolves: #100
ASTERISK-30349 #close

diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index f97c902878b38d01a60b78c61ef0b856f6be72d3..4e7aa5afad0cd4f3165e4dbcb7dd96f9718011db 100644 (file)
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -13178,7 +13178,8 @@ static struct dahdi_pvt *mkintf(int channel, const struct dahdi_chan_conf *conf,
                                analog_p->canpark = conf->chan.canpark;
                                analog_p->dahditrcallerid = conf->chan.dahditrcallerid;
                                analog_p->immediate = conf->chan.immediate;
-                               analog_p->permhidecallerid = conf->chan.permhidecallerid;
+                               analog_p->permhidecallerid = conf->chan.hidecallerid; /* hidecallerid is the config setting, not permhidecallerid (~permcallwaiting above) */
+                               /* It's not necessary to set analog_p->hidecallerid here, sig_analog will set hidecallerid=permhidecaller before each call */
                                analog_p->pulse = conf->chan.pulse;
                                analog_p->threewaycalling = conf->chan.threewaycalling;
                                analog_p->transfer = conf->chan.transfer;

diff --git a/channels/sig_analog.c b/channels/sig_analog.c
index 5ae96d9418d2598acde993ae713ca23bebd88350..8d5ccd64fc7044865624ff7eb6ec6c2188d9c804 100644 (file)
--- a/channels/sig_analog.c
+++ b/channels/sig_analog.c
@@ -1451,6 +1451,8 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
                ast_channel_setoption(ast,AST_OPTION_TDD,&x,sizeof(char),0);
                p->callwaitcas = 0;
                analog_set_callwaiting(p, p->permcallwaiting);
+               /* In theory, the below is not necessary since we set hidecallerid = permhidecaller when calls start,
+                * but this ensures the setting is defaulted properly when channels are idle, too. */
                p->hidecallerid = p->permhidecallerid;
                analog_set_dialing(p, 0);
                analog_update_conf(p);
@@ -2133,6 +2135,19 @@ static void *__analog_ss_thread(void *data)
        case ANALOG_SIG_FXOLS:
        case ANALOG_SIG_FXOGS:
        case ANALOG_SIG_FXOKS:
+               /* Set our default presentation.
+                * This is necessary because the presentation for each call is independent
+                * (though the default may be the same).
+                * For example, if hidecallerid=yes and somebody makes a call with *82,
+                * then makes a 3-way call, the presentation for the 2nd call should still
+                * be blocked, unless that also had a *82.
+                * For this reason, setting hidecallerid = permhidecallerid on hangup
+                * is NOT sufficient, as the *82 from the first call could "leak" into
+                * subsequent ones made before a hangup, improperly leaking a number
+                * that should have been hidden.
+                */
+               p->hidecallerid = p->permhidecallerid;
+
                /* Read the first digit */
                timeout = analog_get_firstdigit_timeout(p);
                /* If starting a threeway call, never timeout on the first digit so someone
@@ -2190,18 +2205,18 @@ static void *__analog_ss_thread(void *data)
                                                res = analog_play_tone(p, idx, -1);
                                                ast_channel_lock(chan);
                                                ast_channel_exten_set(chan, exten);
-                                               if (!ast_strlen_zero(p->cid_num)) {
-                                                       if (!p->hidecallerid) {
-                                                               ast_set_callerid(chan, p->cid_num, NULL, p->cid_num);
-                                                       } else {
-                                                               ast_set_callerid(chan, NULL, NULL, p->cid_num);
-                                                       }
-                                               }
-                                               if (!ast_strlen_zero(p->cid_name)) {
-                                                       if (!p->hidecallerid) {
-                                                               ast_set_callerid(chan, NULL, p->cid_name, NULL);
-                                                       }
+
+                                               /* Properly set the presentation.
+                                                * We need to do this here as well, because p->hidecallerid might be set
+                                                * due to permanent blocking, not star-67/star-82 usage. */
+                                               if (p->hidecallerid) {
+                                                       ast_channel_caller(chan)->id.number.presentation = AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED;
+                                                       ast_channel_caller(chan)->id.name.presentation = AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED;
+                                               } else {
+                                                       ast_channel_caller(chan)->id.number.presentation = AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED;
+                                                       ast_channel_caller(chan)->id.name.presentation = AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED;
                                                }
+
                                                ast_setstate(chan, AST_STATE_RING);
                                                ast_channel_unlock(chan);
                                                analog_set_echocanceller(p, 1);
@@ -2263,9 +2278,11 @@ static void *__analog_ss_thread(void *data)
                                        ast_hangup(chan);
                                        goto quit;
                                }
-
+                       /* While the DMS-100 allows dialing as many *67s and *82s in succession as one's heart may desire,
+                        * the 5ESS does not, it only allows pure toggling (and only once!). So, it's not incorrect
+                        * to prevent people from dialing *67 if that won't actually do anything. */
                        } else if (!p->hidecallerid && !strcmp(exten, "*67")) {
-                               ast_verb(3, "Disabling Caller*ID on %s\n", ast_channel_name(chan));
+                               ast_verb(3, "Blocking Caller*ID on %s\n", ast_channel_name(chan));
                                /* Disable Caller*ID if enabled */
                                p->hidecallerid = 1;
                                ast_channel_caller(chan)->id.number.presentation = AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED;
@@ -2352,7 +2369,7 @@ static void *__analog_ss_thread(void *data)
                                        len = 0;
                                }
                        } else if (p->hidecallerid && !strcmp(exten, "*82")) {
-                               ast_verb(3, "Enabling Caller*ID on %s\n", ast_channel_name(chan));
+                               ast_verb(3, "Allowing Caller*ID on %s\n", ast_channel_name(chan));
                                /* Enable Caller*ID if enabled */
                                p->hidecallerid = 0;
                                ast_channel_caller(chan)->id.number.presentation = AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED;