Icecast-Server issueshttps://gitlab.xiph.org/xiph/icecast-server/-/issues2018-03-06T12:49:47Zhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2124With <role> every client needs to go thru the stack of auth_ts. This will del...2018-03-06T12:49:47ZPhilipp SchafftWith <role> every client needs to go thru the stack of auth_ts. This will delay client connectionsEvery client is now forced to auth. As currently each <role> is implemented with a independent thread and queue this may take extra time. also some modules such as the static type can tell with like no delay if a user matches and does no...Every client is now forced to auth. As currently each <role> is implemented with a independent thread and queue this may take extra time. also some modules such as the static type can tell with like no delay if a user matches and does not need the asynchronous handling of a thread.
I suggest to build an interface so the type cann tell if they need the asynchronous interface or if a synchronous interface will do.
This would reduce the number of threads by at least one per process plus one per password (non-htpasswd) auth. If htpasswd auth can be converted is to be checked.
I suspect a massive performance improvement by this.Icecast 2.5.0Philipp SchafftPhilipp Schaffthttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2123Support management of multiple <role>s2018-03-06T12:49:47ZPhilipp SchafftSupport management of multiple <role>sAs with <role> support there can be multiple per <mount> and global <role>s management support in the admin/ interface needs update.
there are 3 most major parts of this:
1. defining format and outputting information about those <role>s...As with <role> support there can be multiple per <mount> and global <role>s management support in the admin/ interface needs update.
there are 3 most major parts of this:
1. defining format and outputting information about those <role>s on a given (mount or global) object.
1. update the admin/*.xsl files to reflect the new interface.
1. update command_manageauth() to reflect the changes.
Please define a format for the output and reassign to me after that.
Examples:
```
<authentication>
<role id="xxx" name="yyy" />
<role id="xxx" name="yyy" />
<role id="xxx" name="yyy" />
</authentication>
```
```
<authentication>
<role>
<id>yyy</id>
<name>xxx</name>
</role>
<role>
<id>yyy</id>
<name>xxx</name>
</role>
<role>
<id>yyy</id>
<name>xxx</name>
</role>
</authentication>
```
The first example mimics the config file format. The second is more to how stuff in admin/* has been.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2122README.md needs minor fixes2018-03-06T12:49:47ZThomas B. RückerREADME.md needs minor fixese.g. webm and opus missing
Also this ticket will serve for testing git to trac hooks.e.g. webm and opus missing
Also this ticket will serve for testing git to trac hooks.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2121doc subdirectories don't work if packaging overrides "docdir"2018-03-06T12:49:47ZThomas B. Rückerdoc subdirectories don't work if packaging overrides "docdir"Case in point: debian
Patch is in the works as I need it also for my deb/RPM packaging on OBS.Case in point: debian
Patch is in the works as I need it also for my deb/RPM packaging on OBS.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2116Write FAQ on how to set <hostname> correctly2022-03-21T09:35:20ZPhilipp SchafftWrite FAQ on how to set <hostname> correctlyAs we introduced checks for <hostname> and may going to be more strict with this over time documentation needs to be updated.
- Check if documentation tells about setting this to an IP address anywhere. If so correct this.
- Write a sma...As we introduced checks for <hostname> and may going to be more strict with this over time documentation needs to be updated.
- Check if documentation tells about setting this to an IP address anywhere. If so correct this.
- Write a small FAQ on setting <hostname> correctly. This should be mostly for those who aren't familiar with the concept of hostnames.
Some ideas for the FAQ:
- Tell what a hostname, a FQDN is and how it is related and more importantly not related to IP addresses.
- Include a hint to use same domain as webpage.
- Include hint for falling back to whatever server hoster use a default hostname.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2112Locks on avl client_trees needed?2020-10-11T11:18:30ZMarvin ScholzLocks on avl client_trees needed?>do we need to use locks on the avl client_trees in source.c and fserv.c?
Found in TODO, still relevant?>do we need to use locks on the avl client_trees in source.c and fserv.c?
Found in TODO, still relevant?Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2111Race condition in fserv.c?2018-10-04T10:53:27ZMarvin ScholzRace condition in fserv.c?>race condition between avl_tree_unlock(pending_tree) and
>thread_cond_wait(&fserv_cond) in fserv.c, it's a pain to fix but should be.
Found in the TODO, still relevant?>race condition between avl_tree_unlock(pending_tree) and
>thread_cond_wait(&fserv_cond) in fserv.c, it's a pain to fix but should be.
Found in the TODO, still relevant?Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2108Registrable URL handlers in connection.c instead of hardcoded list2018-07-16T09:09:26ZMarvin ScholzRegistrable URL handlers in connection.c instead of hardcoded listSuggested in TODO
>general registerable url-handlers in connection.c rather than hard-coded list
>(already getting unmaintainable)Suggested in TODO
>general registerable url-handlers in connection.c rather than hard-coded list
>(already getting unmaintainable)Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2106Pull vorbis comments (metadata) and send to stats2018-03-06T12:49:47ZMarvin ScholzPull vorbis comments (metadata) and send to stats> Pull out vorbis comments and send to stats. This seems to be being done, but it isn't working right.
Icecast should pull [vorbis comments](http://de.wikipedia.org/wiki/Vorbis_comment) and make them available in stats.> Pull out vorbis comments and send to stats. This seems to be being done, but it isn't working right.
Icecast should pull [vorbis comments](http://de.wikipedia.org/wiki/Vorbis_comment) and make them available in stats.Philipp SchafftPhilipp Schaffthttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2105Make config option (-c) optional?2018-06-16T22:40:36ZMarvin ScholzMake config option (-c) optional?> Should icecast automatically (i.e. without needing -c) look for the config file in /etc/icecast.xml or something?> Should icecast automatically (i.e. without needing -c) look for the config file in /etc/icecast.xml or something?Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2104Check: Bytes sent and time listening might be broken?2018-10-04T10:54:41ZMarvin ScholzCheck: Bytes sent and time listening might be broken?> logging - bytes send and time listening may both be broken?
As mentioned in the TODO file, we should check this.
> logging - bytes send and time listening may both be broken?
As mentioned in the TODO file, we should check this.
Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2102Make strcmp in main.c#_start_logging explicit2018-03-06T12:49:47ZMicheil SmithMake strcmp in main.c#_start_logging explicitCurrently the don't pipe to standard output options are in the else branch, because of usage of `strcmp` such as `if ( strcmp(a, b) ) { `.
This gist https://gist.github.com/miksago/cfb3f41784bff197facc includes changes to make the comp...Currently the don't pipe to standard output options are in the else branch, because of usage of `strcmp` such as `if ( strcmp(a, b) ) { `.
This gist https://gist.github.com/miksago/cfb3f41784bff197facc includes changes to make the comparison of strings explicit as well as changes the logic to be:
```
if config->access_log == '-'; then
handle standard output
else
handle logging
```
Philipp SchafftPhilipp Schaffthttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2099Review how Icecast binds to sockets (IPv6/IPv4)2022-03-07T10:07:56ZThomas B. RückerReview how Icecast binds to sockets (IPv6/IPv4)The current behaviour, as introduced after considerable research and discussion neeeds to be reviewed.
Either we should bind to *both* if no IP nor protocol is specified or we MUST document this better and change the default config to e...The current behaviour, as introduced after considerable research and discussion neeeds to be reviewed.
Either we should bind to *both* if no IP nor protocol is specified or we MUST document this better and change the default config to explicitly bind to both.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2098TAGs for comments in all icecast projects2018-03-06T12:49:47ZPhilipp SchafftTAGs for comments in all icecast projectsI suggest to define the following tags in comments for more easy finding stuff that needs to be reviewed or handled in future release. Those tags should be added as part of the coding style guide to define some standard for the Icecast p...I suggest to define the following tags in comments for more easy finding stuff that needs to be reviewed or handled in future release. Those tags should be added as part of the coding style guide to define some standard for the Icecast project and be used by all components handed by the project including but not limited to the Icecast server, Ices2 and the library subprojects. I'm unsure if/how it will match the stream directory subproject.
The following tags should be defined:
Actions:
- REVIEW: ask for a review of this.
- REWRITE: ask for a rewrite of this section.
- TODO: ask for implementation of a feature.
- FIXME: ask correcting an implementation.
- REMOVE: remove a block or feature.
- DOCUMENT: documentation for this block or feature is missing, incomplete or wrong and needs update.
Conditions:
- [BEFORE|AFTER|IN] RELEASE $version: This is relevant to release $version. $version can also be NEXT.
- [BEFORE|AFTER|IN] YEAR $yyyy: This is relevant to (4-digit) year $yyyy
Extra Tags:
- IMPORTANT: This is an important problem.
- SECURITY: This is security critical.
- LEAK: This leaks some resource (memory, file descriptors, ...).
Syntax:
```
/* [CONDITION[ CONDITION[...]]] [EXTRA TAGS] ACTION [#TICKET]: DESCRIPTION
* [LONG DESCRIPTION]
*/
```
Examples:
```
/* BEFORE RELEASE 2.5.3 REVIEW #1234: Should we convert A to B?
* A is according to standard REF0. This standard was superseded by
* standard REF1 which could be implemented with option B.
* This may break early clients of standard REF0 not being aware of SOMETHING.
*/
/* IN YEAR 2022 REWRITE: Change copyright statement as license expires. */
/* LEAK FIXME #1234: Fix case object can not be added to queue. */
/* BEFORE RELEASE NEXT IMPORTANT SECURITY FIXME #1234: Do not expose passwords on authentication failure of backend server */
/* AFTER RELEASE 2.5.3 REMOVE: Remove support for icecast 1.x style SOURCE requests */
```Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2097In <listener> in some tags are camelcase that should be converted to lowercase.2018-03-06T12:49:47ZPhilipp SchafftIn <listener> in some tags are camelcase that should be converted to lowercase.in <listener> the following tags are camelcase: <ID>, <IP>, <UserAgent> and <Connected>. Those should be converted to lower case.
XML tells that tag names are case sensitive.
This needs to be documented before we can change it and be cha...in <listener> the following tags are camelcase: <ID>, <IP>, <UserAgent> and <Connected>. Those should be converted to lower case.
XML tells that tag names are case sensitive.
This needs to be documented before we can change it and be changed with a major release. I suggest to document it as 'Parsers MUST be case insensitive for ALL tags in ANY admin/-command output.'
```
09:51 <+tbr> as a broad statement, I'd not expect this change to happen before 2.6
09:52 < ph3-der-loewe> I haven't thought about a timeline for that already.
09:52 < ph3-der-loewe> I'm fine with <= 3.0.0 :)
09:53 < ph3-der-loewe> with 3.0.0 the question that arise is if the interface still exist ;)
```
Please review this before 2.5 to check if we are on-track on this.Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2096use setresuid()/setresgid() instead of setuid()/setgid()2018-03-06T12:49:47Zd26264b9use setresuid()/setresgid() instead of setuid()/setgid()We should be dropping privileges with setresuid()/setresgid() when requested as recommended by "Setuid Demystified" (http://www.cs.berkeley.edu/~daw/papers/setuid-usenix02.pdf).
Also, chdir("/") after chroot() and actually check for pro...We should be dropping privileges with setresuid()/setresgid() when requested as recommended by "Setuid Demystified" (http://www.cs.berkeley.edu/~daw/papers/setuid-usenix02.pdf).
Also, chdir("/") after chroot() and actually check for proper return values on both. This was modelled after OpenSSH's chroot() logic.
Tested on OpenBSD. Someone should try compiling/testing on Linux to verify. As far as I can tell, the proper syscalls are implemented on Linux as well.
```
--- src/main.c Mon May 5 18:29:06 2014
+++ src/main.c Thu Nov 27 18:55:34 2014
@@ -377,7 +377,7 @@
fprintf(stderr, "WARNING: Cannot change server root unless running as root.\n");
return;
}
- if(chroot(conf->base_dir))
+ if(chroot(conf->base_dir) == -1 || chdir("/") == -1)
{
fprintf(stderr,"WARNING: Couldn't change server root: %s\n", strerror(errno));
return;
@@ -398,7 +398,7 @@
}
if(uid != (uid_t)-1 && gid != (gid_t)-1) {
- if(!setgid(gid))
+ if(!setresgid(gid, gid, gid))
fprintf(stdout, "Changed groupid to %i.\n", (int)gid);
else
fprintf(stdout, "Error changing groupid: %s.\n", strerror(errno));
@@ -406,7 +406,7 @@
fprintf(stdout, "Changed supplementary groups based on user: %s.\n", conf->user);
else
fprintf(stdout, "Error changing supplementary groups: %s.\n", strerror(errno));
- if(!setuid(uid))
+ if(!setresuid(uid, uid, uid))
fprintf(stdout, "Changed userid to %i.\n", (int)uid);
else
fprintf(stdout, "Error changing userid: %s.\n", strerror(errno));
```
Philipp SchafftPhilipp Schaffthttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2089[duplicate] icecast sends output of <on-connect> script to source client2018-03-06T12:49:47ZSven Herzberg[duplicate] icecast sends output of <on-connect> script to source clientPlease look at #2087 instead.
----
Using the on-connect script from #2087, a client which does not close the connection immediately after receiving the 200 response, has a chance of reading “stdout” after stopping to send any data.
If...Please look at #2087 instead.
----
Using the on-connect script from #2087, a client which does not close the connection immediately after receiving the 200 response, has a chance of reading “stdout” after stopping to send any data.
If this is unintentional, this data should end up in e.g. the `<errorlog>` target.
If this is intentional, the length of the data should be indicated by a `Content-Length` header, or should be properly encoded as `Transfer-Encoding: chunked` (which would then be required as a header in the response).Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2088accept data with “Transfer-Encoding: chunked”2018-03-06T12:49:47ZSven Herzbergaccept data with “Transfer-Encoding: chunked”Many HTTP frameworks will automatically encode data streams using the aforementioned method. I propose this behavior for Icecast:
* check if the protocol is not `HTTP/1.1` or there is no `Transfer-Encoding` header, continue as in the pa...Many HTTP frameworks will automatically encode data streams using the aforementioned method. I propose this behavior for Icecast:
* check if the protocol is not `HTTP/1.1` or there is no `Transfer-Encoding` header, continue as in the past (i.e. assume `identity` encoding)
* if the `Transfer-Encoding` header is present and it is `identity`, continue as in the past
* if the `Transfer-Encoding` header is present and it is `chunked`, accept the data and strip the encoding information both from the output stream and from the dumpfile
* if the `Transfer-Encoding` header is present and has a different value, terminate the request with 501 (Unimplemented) and provide an HTTP response body listing the supported encodings (in case developers need to debug this).
That behavior in compliant with RFC2616 (Section 3.6) and RFC7230 (Section 3.3.1).Icecast 2.5.0Thomas B. RückerThomas B. Rückerhttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2087icecast does not only write the stream data into the dumpfile2018-03-06T12:49:47ZSven Herzbergicecast does not only write the stream data into the dumpfileHow to reproduce:
Write a script (e.g. called “test-script.sh”) with this content and set the executable bit:
```
set -e
set -x
echo "stdout"
echo "stderr" >&2
```
Then specify that script in an mount point of the icecast configurat...How to reproduce:
Write a script (e.g. called “test-script.sh”) with this content and set the executable bit:
```
set -e
set -x
echo "stdout"
echo "stderr" >&2
```
Then specify that script in an mount point of the icecast configuration like this (I have this in my mount point defaults):
```
<on-connect>/path/to/test-script.sh</on-connect>
```
The beginning of the dump-file will look like this:
```
# hexdump -C /srv/icecast/test-stream/backup.mp3 | head
00000000 2b 20 65 63 68 6f 20 73 74 64 6f 75 74 0a 2b 20 |+ echo stdout.+ |
00000010 65 63 68 6f 20 73 74 64 65 72 72 0a 73 74 64 65 |echo stderr.stde|
00000020 72 72 0a 2b 20 65 78 69 74 20 30 0a […] |rr.+ exit 0.[…]|
```
This makes the dumpfiles difficult to use as backups of the stream data.
I think the `<errorlog>` target would be more a appropriate target for this output.Icecast 2.5.0Philipp SchafftPhilipp Schaffthttps://gitlab.xiph.org/xiph/icecast-server/-/issues/2086[PATCH] Send final status code only after the source data was received2018-03-06T12:49:47ZMarvin Scholz[PATCH] Send final status code only after the source data was receivedIcecasts new PUT support should comply with the HTTP protocol, currently this isn't the case since it sends the status line (`200 OK`) right after the source clients connects, but should only send it at the end of the request. Error code...Icecasts new PUT support should comply with the HTTP protocol, currently this isn't the case since it sends the status line (`200 OK`) right after the source clients connects, but should only send it at the end of the request. Error codes can be sent earlier, since they indicate that transmission of data is finished.
> An HTTP/1.1 (or later) client sending a message-body SHOULD monitor the network connection for an error status while it is transmitting the request. If the client sees an error status, it SHOULD immediately cease transmitting the body.
With the success code 200 (and others like 201) this is not the case, since it would indicate Success until all data is received which makes no sense.
If a source client needs a indicator when to start sending data, it should set the `Expect: 100-continue` header and wait for the `100 Continue` reply from the server.
Here is an example what Icecast currently sends:
```
> PUT /testsendung.mp3 HTTP/1.1
> Authorization: Basic REDACTED=
> Host: example.com:8001
> Accept: */*
> Content-Type: audio/ogg
> Ice-Public: 1
> Ice-Name: Teststream
> Ice-Description: This is just a simple test stream
> Ice-URL: http://example.org
> Ice-Genre: Rock
> Expect: 100-continue
>
< HTTP/1.1 100 Continue
> [ Some stream data sent by client ]
< HTTP/1.0 200 OK
> [ More stream data sent by client ]
```
Instead Icecast should send:
```
> PUT /testsendung.mp3 HTTP/1.1
> Authorization: Basic REDACTED=
> Host: example.com:8001
> Accept: */*
> Content-Type: audio/ogg
> Ice-Public: 1
> Ice-Name: Teststream
> Ice-Description: This is just a simple test stream
> Ice-URL: http://example.org
> Ice-Genre: Rock
> Expect: 100-continue
>
< HTTP/1.1 100 Continue
> [ Stream data sent by client ]
< HTTP/1.1 200 OK
< Content-Length: 0
< Connection: close
<
* Closing connection 0
```
(Additionally note that Icecast mixes HTTP Protocol)
This patch fixes the behavior so that it matches the second one. I am not completely sure if I fixed it the right way, since the file server internals are not 100% clear to me.
I marked it was critical since I think we should address this asap, so that (new) source clients do not start to rely on this (wrong) behavior.Philipp SchafftPhilipp Schafft