[Asterisk-code-review] Revert "chan_iax2: Prevent crash if dialing RSA-only call without out... (asterisk[18])

Sean Bright asteriskteam at digium.com
Sun Jul 3 11:18:59 CDT 2022


Attention is currently required from: N A, Joshua Colp, George Joseph, Kevin Harwell.
Hello N A, Joshua Colp, George Joseph, Kevin Harwell, Friendly Automation,

I'd like you to do a code review. Please visit

    https://gerrit.asterisk.org/c/asterisk/+/18731

to review the following change.


Change subject: Revert "chan_iax2: Prevent crash if dialing RSA-only call without outkey."
......................................................................

Revert "chan_iax2: Prevent crash if dialing RSA-only call without outkey."

This reverts commit 9dc321cbcb21a243b9e6eed48c0d8ee8aa24c689.

Reason for revert: This causes a build regression by making OpenSSL a required dependency.

Change-Id: I2b8cc5c54533f3b1fc58ad4da5af0c89c51be1b2
---
M channels/chan_iax2.c
1 file changed, 1 insertion(+), 23 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/31/18731/1

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 6d76dc5..ff235af 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -6379,18 +6379,6 @@
 	}
 }
 
-static int invalid_key(ast_aes_decrypt_key *ecx)
-{
-	int i;
-	for (i = 0; i < 60; i++) {
-		if (ecx->rd_key[i]) {
-			return 0; /* stop if we encounter anything non-zero */
-		}
-	}
-	/* if ast_aes_encrypt or ast_aes_decrypt is called, then we'll crash when calling AES_encrypt or AES_decrypt */
-	return -1;
-}
-
 static void build_encryption_keys(const unsigned char *digest, struct chan_iax2_pvt *pvt)
 {
 	build_ecx_key(digest, pvt);
@@ -8447,7 +8435,7 @@
 			iax_ie_append_str(ied, IAX_IE_PASSWORD, secret);
 			res = 0;
 		} else
-			ast_log(LOG_WARNING, "No way to send secret to peer '%s' (their methods: %d)\n", ast_sockaddr_stringify_addr(addr), authmethods);
+			ast_log(LOG_NOTICE, "No way to send secret to peer '%s' (their methods: %d)\n", ast_sockaddr_stringify_addr(addr), authmethods);
 	}
 	return res;
 }
@@ -8532,22 +8520,12 @@
 		}
 	}
 
-	if (!(ies->authmethods & (IAX_AUTH_MD5 | IAX_AUTH_PLAINTEXT)) && (ies->authmethods & IAX_AUTH_RSA) && ast_strlen_zero(okey)) {
-		/* If the only thing available is RSA, and we don't have an outkey, we can't do it... */
-		ast_log(LOG_WARNING, "Call terminated. RSA authentication requires an outkey\n");
-		return -1;
-	}
-
 	if (ies->encmethods) {
 		if (ast_strlen_zero(p->secret) &&
 			((ies->authmethods & IAX_AUTH_RSA) || (ies->authmethods & IAX_AUTH_MD5) || (ies->authmethods & IAX_AUTH_PLAINTEXT))) {
 			ast_log(LOG_WARNING, "Call terminated. Encryption requested by peer but no secret available locally\n");
 			return -1;
 		}
-		/* Don't even THINK about trying to encrypt or decrypt anything if we don't have valid keys, for some reason... */
-		/* If either of these happens, it's our fault, not the user's. But we should abort rather than crash. */
-		ast_assert_return(!invalid_key(&p->ecx), -1);
-		ast_assert_return(!invalid_key(&p->dcx), -1);
 		ast_set_flag64(p, IAX_ENCRYPTED | IAX_KEYPOPULATED);
 	} else if (ast_test_flag64(iaxs[callno], IAX_FORCE_ENCRYPT)) {
 		ast_log(LOG_NOTICE, "Call initiated without encryption while forceencryption=yes option is set\n");

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18731
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I2b8cc5c54533f3b1fc58ad4da5af0c89c51be1b2
Gerrit-Change-Number: 18731
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean at seanbright.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Attention: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220703/2bd4b8d9/attachment-0001.html>


More information about the asterisk-code-review mailing list