[Asterisk-code-review] chan_dahdi: Fix broken hidecallerid setting. (asterisk[master])
N A
asteriskteam at digium.com
Mon Dec 12 09:24:30 CST 2022
N A has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/19707 )
Change subject: chan_dahdi: Fix broken hidecallerid setting.
......................................................................
chan_dahdi: Fix broken hidecallerid setting.
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.
ASTERISK-30349 #close
Change-Id: I45d1f3d697e833fadbc45a8fcc37f569dd9063d0
---
M channels/chan_dahdi.c
M channels/sig_analog.c
2 files changed, 73 insertions(+), 15 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/07/19707/1
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 5607eb0..aab53c1 100644
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -13108,7 +13108,8 @@
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 b694a96..4b8a5d9 100644
--- a/channels/sig_analog.c
+++ b/channels/sig_analog.c
@@ -2133,6 +2133,19 @@
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
+ * (thought 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 +2203,18 @@
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);
- }
+
+ /* Properly set the presentation.
+ * We need to do this here as well, because p->hidecallerid might be set
+ * due to permanent blocking, not *67/*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;
}
- if (!ast_strlen_zero(p->cid_name)) {
- if (!p->hidecallerid) {
- ast_set_callerid(chan, NULL, p->cid_name, NULL);
- }
- }
+
ast_setstate(chan, AST_STATE_RING);
ast_channel_unlock(chan);
analog_set_echocanceller(p, 1);
@@ -2263,9 +2276,11 @@
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 +2367,7 @@
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;
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19707
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I45d1f3d697e833fadbc45a8fcc37f569dd9063d0
Gerrit-Change-Number: 19707
Gerrit-PatchSet: 1
Gerrit-Owner: N A <asterisk at phreaknet.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221212/029b539b/attachment.html>
More information about the asterisk-code-review
mailing list