Allow authenticating with a username to redis #1488
Conversation
|
Thank you @maddymeows ! Could you please rebase the PR and also explain if you tested with new and old redis. |
maddymeows
force-pushed
the
redis-username
branch
2 times, most recently
from
81e0ab1
to
2408c78
Compare
maddymeows
force-pushed
the
redis-username
branch
from
2408c78
to
6a110d3
Compare
maddymeows
force-pushed
the
redis-username
branch
from
70e9221
to
875358d
Compare
|
Sorry for tagging the regression fix alongside this PR, came up in testing after rebasing.
Done
I've tested on debian 10 which has redis dated before ACLs were introduced, works fine as long as you don't configure a username.
As far as I know, no |
| @@ -163,9 +167,6 @@ static Ryconninfo *RyconninfoParse(const char *userdb, char **errmsg) { | |||
| if (!(co->host)) { | |||
| co->host = strdup("127.0.0.1"); | |||
| } | |||
| if (!(co->password)) { | |||
There was a problem hiding this comment.
Originally during testing I created an extra branch if (!(co->user)) { co->user = strdup(""); }.
However, I found out that it made coturn always authenticate with a username and password, which would break for redis servers that don't have the ACL feature.
That's because the branches using password (which I ended up copying for username branches) checked if the pointer to password was set, which it always was with the statement I just removed, rather than if password contained a useful value. So the password conditionals haven't been working for a while, coturn just discarded the error that redis threw out and continued on (which is fine, but not ideal).
Hence also changes to make those branches co->password && strlen(co->password) rather than just co->password, just in case. I could effectively change them to co->password && co->password[0] but strlen felt nicer.
Error handling of missing auth doesn't seem to be handled fully correctly from what I grasp, but happens moreso as a side effect, the side effect of using the side effect is it never warns about if the password is needed which is why there are no logs complaining about use of AUTH when wasn't needed.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -268,7 +273,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| ret = redisLibeventAttach(base, co->host, co->port, co->password, atoi(co->dbname)); | |||
| ret = redisLibeventAttach(base, co->host, co->port, co->user, co->password, atoi(co->dbname)); | |||
Check warning
Code scanning / PREfast
'co->dbname' could be '0': this does not adhere to the specification for the function 'atoi'. Warning
There was a problem hiding this comment.
co->dbname is never null because a default is given in RyconninfoParse, I don't know how to silence this.
I needed this
I just looked for occurrences of password and added username branches there. Let me know if there's anything wrong, I'm not proficient in C at all.