Hi Stuart, Thanks for the quick response on a lovely sunday.
On Sonntag, 20. August 2017 21:07:08 CEST you wrote: > The cantata package in Debian is built with -DENABLE_HTTP_SERVER=OFF because > exposing services has never seemed like a good idea without a lot of > careful consideration and detailed code review. I see your point. > I'm quite happy to be > convinced that this is an appropriate thing to do from a security > standpoint, but I need convincing. I think it in fact is. Cantata does a few things to make this secure. * Requests not from the MPD host are rejected with error 400 https://github.com/CDrummond/cantata/blob/master/http/httpsocket.cpp#L258 * Only files which have previously been added for HTTP streaming will be opened: https://github.com/CDrummond/cantata/blob/master/http/httpsocket.cpp#L282 https://github.com/CDrummond/cantata/blob/master/http/httpsocket.cpp#L416 From my point of view, this is sufficient. Attack vectors which come into mind: * Bandwidth and memory exhaustion: I’d assume this is already possible via the normal MPD connection (pretending to have an insanely large playqueue for example). * MPD reading arbitrary files: cantata protects against that by only serving requests for files which have previously been added as streams. * Anyone else reading streamed files: cantata protects against that by only answering requests coming from the MPD host. - IP spoofing: the stream is using TCP, so IP spoofing is not entirely trivial but still possible in a hostile network. I don’t consider this to be a major concern. * Someone under control of the MPD host reading the streamed files: That is possible. Requires to know the streamed file name (possibly inferable from the played music) and spoofing of the user agent (trivial). I do not consider this a major problem. * Security issue in the HTTP handling itself: There was a potential DoS/ memory-exhaustion vector, but the patches I sent upstream for that have been applied [1][2][3]. It essentially boiled down to an unfortunate default for QTcpSocket::readBufferSize. Otherwise, the HTTP handling looks quite sane. There’s no complex parsing involved and from what I can tell, there’s (now) no dynamic allocation based on peer input or dynamic access to buffers. I’m not sure if that is convincing. As a last point, I’d like to add that this feature makes Cantata pretty amazing, and with the patches applied, I think that the attack surface is reduced to a reasonable amount. What do you think? kind regards, Jonas [1]: https://github.com/CDrummond/cantata/commit/7c9bd03 [2]: https://github.com/CDrummond/cantata/commit/8fd108c [3]: https://github.com/CDrummond/cantata/commit/a9963ad _______________________________________________ pkg-multimedia-maintainers mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers
