<p>Joshua Colp <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18484">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Joshua Colp: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">chan_iax2: Prevent crash if dialing RSA-only call without outkey.<br><br>Currently, if attempting to place a call to a peer that only allows<br>RSA authentication, if we fail to provide an outkey when placing<br>the call, Asterisk will crash.<br><br>This exposes the broader issue that IAX2 is prone to causing a crash<br>if encryption or decryption is attempted but we never initialized<br>the encryption and decryption keys. In other words, if the logic<br>to use encryption in chan_iax2 is not perfectly aligned with the<br>decision to build keys in the first place, then a crash is not<br>only possible but probable. This was demonstrated by ASTERISK_29264,<br>for instance.<br><br>This permanently prevents such events from causing a crash by explicitly<br>checking that keys are initialized properly before setting the flags<br>to use encryption for the call. Instead of crashing, the call will<br>now abort.<br><br>ASTERISK-30007 #close<br><br>Change-Id: If925c3d86099ceac7f621804f2532baac5050c9a<br>---<br>M channels/chan_iax2.c<br>1 file changed, 23 insertions(+), 1 deletion(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c</span><br><span>index 27a7118..598e9d5 100644</span><br><span>--- a/channels/chan_iax2.c</span><br><span>+++ b/channels/chan_iax2.c</span><br><span>@@ -6379,6 +6379,18 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int invalid_key(ast_aes_decrypt_key *ecx)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < 60; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ecx->rd_key[i]) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0; /* stop if we encounter anything non-zero */</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* if ast_aes_encrypt or ast_aes_decrypt is called, then we'll crash when calling AES_encrypt or AES_decrypt */</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void build_encryption_keys(const unsigned char *digest, struct chan_iax2_pvt *pvt)</span><br><span> {</span><br><span> build_ecx_key(digest, pvt);</span><br><span>@@ -8435,7 +8447,7 @@</span><br><span> iax_ie_append_str(ied, IAX_IE_PASSWORD, secret);</span><br><span> res = 0;</span><br><span> } else</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_NOTICE, "No way to send secret to peer '%s' (their methods: %d)\n", ast_sockaddr_stringify_addr(addr), authmethods);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "No way to send secret to peer '%s' (their methods: %d)\n", ast_sockaddr_stringify_addr(addr), authmethods);</span><br><span> }</span><br><span> return res;</span><br><span> }</span><br><span>@@ -8520,12 +8532,22 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(ies->authmethods & (IAX_AUTH_MD5 | IAX_AUTH_PLAINTEXT)) && (ies->authmethods & IAX_AUTH_RSA) && ast_strlen_zero(okey)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If the only thing available is RSA, and we don't have an outkey, we can't do it... */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "Call terminated. RSA authentication requires an outkey\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ies->encmethods) {</span><br><span> if (ast_strlen_zero(p->secret) &&</span><br><span> ((ies->authmethods & IAX_AUTH_RSA) || (ies->authmethods & IAX_AUTH_MD5) || (ies->authmethods & IAX_AUTH_PLAINTEXT))) {</span><br><span> ast_log(LOG_WARNING, "Call terminated. Encryption requested by peer but no secret available locally\n");</span><br><span> return -1;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Don't even THINK about trying to encrypt or decrypt anything if we don't have valid keys, for some reason... */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If either of these happens, it's our fault, not the user's. But we should abort rather than crash. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_assert_return(!invalid_key(&p->ecx), -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_assert_return(!invalid_key(&p->dcx), -1);</span><br><span> ast_set_flag64(p, IAX_ENCRYPTED | IAX_KEYPOPULATED);</span><br><span> } else if (ast_test_flag64(iaxs[callno], IAX_FORCE_ENCRYPT)) {</span><br><span> ast_log(LOG_NOTICE, "Call initiated without encryption while forceencryption=yes option is set\n");</span><br><span></span><br></pre><div style="white-space:pre-wrap"></div><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18484">change 18484</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18484"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: If925c3d86099ceac7f621804f2532baac5050c9a </div>
<div style="display:none"> Gerrit-Change-Number: 18484 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>