Summary:	ASTERISK-22820: [patch] Plaintext auth is still supported in IAX2
Reporter:	Eugene (varnav)	Labels:
Date Opened:	2013-11-03 03:07:13.000-0600	Date Closed:	2016-12-20 13:59:51.000-0600
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Channels/chan_iax2
Versions:	SVN 12.0.0 13.0.0	Frequency of Occurrence	Constant
Related Issues:
Environment:		Attachments:	( 0) asterisk-12-chan_iax2-plaintext-auth-deprecated-v2.diff
Description:	Starting from draft 2 of RFC 5456 (October 23, 2006) plaintext auth is not supported in IAX2 protocol. Please refer to section 8.6.13 of RFC 5456. But plaintext auth is still supported by Asterisk implementation of IAX2. This support should be dropped. Attached patch, based on asterisk-dev discussion, adds deprecation warning on startup if 'auth' is set to 'plaintext', changes default values of 'auth' from 'md5, plaintext' to 'md5', and adds note to UPGRADE.txt Patch is safe in terms of backwards compatibility, will work even if remote peers have auth=plaintext and we have defaults. auth=plaintext setting will remain deprecated in Asterisk 14 and 15, and IAX2 plaintext support will be removed in Asterisk 16.
Comments:	By: Michael L. Young (elguero) 2013-11-03 07:59:05.938-0600 Do you have a patch? You should bring this up on the asterisk-dev mailing list, not here on the issue tracker. By: Eugene (varnav) 2013-11-03 09:23:23.840-0600 Is noncompliance with RFC an issue or not? I don't have a patch yet. By: Eugene (varnav) 2013-11-03 14:41:44.282-0600 Patch for 12 and trunk versions of chan_iax2.c Removes plaintext auth support in IAX2 for RFC5456 compliance. By: Michael L. Young (elguero) 2013-11-03 15:30:13.929-0600 First, thanks for the contribution.  It is appreciated. My comment above was based on, to me, that it can be based on different point of views in regards to whether it needs to be taken out or not.  On one hand you have something that perhaps doesn't match the RFC but on the other hand it might still be there for backwards compatibility.  That is why I didn't think we should just assume that it needs to be taken out without some discussion and getting what the consensus was.  Hence my comment to mention it on the mailing list in order to find out if it was an issue that needed to be taken on or not. Again, thanks for the contribution. By: Eugene (varnav) 2013-11-04 01:43:38.877-0600 Well, to be honest, I'm just confused with these mailing lists and how to use them. Now, when we have the patch, maybe it worth to get it throught reviewboard? Or it is still a good idea to discuss this on asterisk-dev? Leaving this feature will, of course, leave some backwards compatibility with systems that are configured to use plaintext (what is not recommended and never was). It's not about backwards compatibility with older versions, as MD5 and RSA auth methods are in IAX2 protocol since the very first draft of specification. Removing this feature will make asterisk more secure and will make IAX2 implementation RFC compliant. And, of course, it's not the only solution to remove plaintext support. It is possible to do partial removal, like "accept incoming plaintext auth but never send plaintext password" or, minimalistic solution, just add log messages that plaintext auth for IAX2 is deprecated. By: Matt Jordan (mjordan) 2013-11-04 11:13:55.785-0600 JIRA issues are to track issues and provide a place for people to get patches. Review for patches that are small can also occur on the JIRA issue. Review board is typically reserved for people who propose patches often and have shown that they can follow the [Coding Guidelines|https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines]. Patches are put up on Review Board when they warrant more discussion than what takes place on the JIRA issues. Often, folks in the developer community pick up issues in JIRA and post patches to Review Board for further discussion, if they feel like it is warranted. It is, of course, always a good idea to do code reviews, and if a patch seems large and is committed directly without review, someone may ask you that you revert the patch and put it up for review. Finanly, in general, we try to have discussions on the mailing list, as opposed to on the JIRA issues. More people read the development mailing list than the asterisk-bugs mailing list, and it is an appropriate mechanism for discussion of code. Hope that clears things up! By: Eugene (varnav) 2013-11-05 05:45:30.119-0600 Patch for Asterisk 12 based on discussion in asterisk-dev. Emit a warning if auth methoid (or one of auth methods) is set to plaintext. Additionally, warning is emitted every time plaintext auth is sent or accepted. Why? The tricky thing with deprecation is what auth methods we set as default. As far as I can see inside sources, if auth= parameter is omitted, auth methods are set to "md5 first, then plaintext". Another thing is that if we have auth=md5, but remote side has auth=plaintext, and we call them, plaintext will be used. We will see warning too. Issue with this patch is that if type=friend, get_auth_method() is called twice, so warning is displayed twice on chan_iax2 load. Patch adds note to UPGRADE.txt too. Tested by me. By: Eugene (varnav) 2013-11-06 11:55:47.519-0600 Allright, new version of patch. Plaintext is not set as default anymore. I've tested - this breaks nothing, will work even if remote peers have auth=plaintext Warnings are on startup only. By: Eugene (varnav) 2014-11-16 03:39:37.963-0600 This security-related issue has a patch available for more than a year already. By: Eugene (varnav) 2015-04-14 03:25:16.680-0500 This was reported more than 1,5 years ago. Fix is easy and safe. Without fix we violate nine-years-old RFC and we have a security vulnerability in place. Please fix this. By: Eugene (varnav) 2015-08-11 04:18:42.852-0500 Any hope this will be implemented? By: Joshua C. Colp (jcolp) 2015-08-11 05:03:41.745-0500 It is possible for this to be implemented, yes. As I mentioned on your other issue since this is just attached here the burden is on someone else to take it through the code review process. Noone has as of yet done that. If you'd like to then this has a much better chance of going in faster. By: Eugene (varnav) 2015-08-11 05:54:40.997-0500 I would be happy to do that, but is it possible for me to review my own code? By: Joshua C. Colp (jcolp) 2015-08-11 06:02:23.722-0500 You can put it up and respond to feedback as it occurs.