Summary:	ASTERISK-24517: TLS support for Solaris, Ming and non-glibc Linux systems
Reporter:	Timo Teräs (fabled)	Labels:
Date Opened:	2014-11-12 05:17:17.000-0600	Date Closed:	2016-11-17 11:07:37.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/Portability
Versions:	12.7.0 13.0.0	Frequency of Occurrence
Related Issues:	is related to ASTERISK-24515 Unconditional use of fopencookie() / funopen() is non-portable is related to ASTERISK-26629 tests/manager: 4 test failures as a result of iostream change
Environment:		Attachments:	( 0) 0001-convert-to-ast_iostream.patch
Description:	TLS is not working on systems without funopen(3) or fopencookie(3). This is a major portability issue as both functions are non-standard extensions. Per comment in ASTERISK-15490 I'm creating this improvement issue, as I don't see this bug being created earlier. ASTERISK-24515 is also related, but it is about *regression* of now defunct regular (non-TLS) connections. This bug is about adding the TLS-support for platforms that have never worked earlier.
Comments:	By: Matt Jordan (mjordan) 2014-11-12 11:51:27.592-0600 Are you considering providing a patch? Improvements/new features without patches are closed unless there is a developer actively working them. By: Rusty Newton (rnewton) 2014-11-13 16:06:40.379-0600 [New Feature Guidelines|https://wiki.asterisk.org/wiki/display/AST/New+Feature+Guidelines] By: Timo Teräs (fabled) 2014-11-14 08:39:31.130-0600 I'll consider working on this. The obvious fix is to remove FILE, and replace it with internal abstraction. This would be possibly just extending ast_filestream. It is mechanical change; but it will be intrusive. Fortunately it seems that most calls are already wrapped by some existing internal API. I'll look into this a bit more next week. By: Rusty Newton (rnewton) 2014-12-03 09:05:26.591-0600 Timo, are you currently working this? If so I can open it up and assign it to you. Otherwise we'll close this out until you or someone else can come up with a patch. By: Rusty Newton (rnewton) 2014-12-17 09:13:39.472-0600 Suspending since we don't have any activity. Please talk to a bug marshal in #asterisk-bugs or #asterisk-dev on irc.freenode.net to request the issue be reopened after you have a patch or begin working on one. Thanks! By: Timo Teräs (fabled) 2015-02-09 07:27:44.840-0600 I'm willing to start working on this now. I looked at how widely the funfile() return FILE objects are used, and it looks like it is limited to http.c, manager.c, tcptls.c and res_http_websocket.c. I'm wondering which of my options I should pursue: 1. remove "FILE f" and "int fd" on the related structs, and implement a custom "ast_file" or similar file io wrapping layer (unfortunately ast_filestream is too tied to audio files, channels and frames) 2. consider using socketpair() + thread + fdopen() to implement funopen like functionality via additional thread Though, I would prefer probably options 1, as it's less resource intensive. Additionally the way you use both the file descriptor and FILE object interleaved is non-portable and not guaranteed to work as expected, especially since fd seems to be in non-blocking mode. To my understanding stdio is not required to work in non-blocking mode at all. Doing #1 would also clean up the code, as there is a lot of mixed of use of the fd (for polling) and FILE for reading/writing. It also seems to be rather useless to use FILE in non-buffered mode - since the whole idea of stdio FILE is to provide buffering. Now it is used only to provide the abstraction in non-conforming ways. So this would justify removing FILE altogether and implementing the required abstraction internally. This would allow also removing the helper functions like ast_careful_fwrite() as the semantics could be written properly in the abstraction layer. By: Matt Jordan (mjordan) 2015-02-09 10:43:17.797-0600 Re-opened and assigned to [~fabled]. I'd recommend option #1 as well. Generally, I'd take whatever approach is least intrusive, as the thread mechanics involved with the TCP/TLS layer are easy to miss or get wrong - and would certainly have a ripple effect through other modules. By: Timo Teräs (fabled) 2015-04-17 09:05:30.059-0500 I've done some additional analysis now. I think I will just make ast_tcptls_session_instance more opaque. Remove FILE *f, and int fd from it. And write additional helper functions for any functionality earlier used via FILE. This simplifies and unifies the code in many places. And allows to remove lot of the glue wrappers. I suppose the original reason for wrapping it in a FILE was due to fgets() and fprintf() usage in http.c. But I think they are relatively easily replaced. I hope to get something for testing within a week or two. By: Timo Teräs (fabled) 2015-04-20 06:27:50.245-0500 seems it is not possible easily after all. E.g. app_externalivr shares the same code path for forked process along with tls/tcp code. So my suggestions is to make a new abstraction for iostreams. See attached patch. It's intended as draft of what I'm working, and is not fully tested or commented. But at least it compiles, and AMI-over-http login worked. By: Timo Teräs (fabled) 2015-04-20 06:28:17.661-0500 first draft By: Friendly Automation (friendly-automation) 2016-11-01 09:05:47.207-0500 Change 2932 had a related patch set uploaded by Timo Teräs: Implement internal abstraction for iostreams [https://gerrit.asterisk.org/2932|https://gerrit.asterisk.org/2932] By: Friendly Automation (friendly-automation) 2016-11-17 11:07:38.366-0600 Change 2932 merged by Joshua Colp: Implement internal abstraction for iostreams [https://gerrit.asterisk.org/2932|https://gerrit.asterisk.org/2932]