Bugzilla – Bug 792
security: CLI should require authentication if HTTP does
Last modified: 2008-12-15 13:06:07 UTC
By default, SlimServer binds to ports for both HTTP and CLI interfaces. Currently, the Server "Security" settings allows users to password-protect the HTTP interface, but not the CLI interface. The CLI code should be modified to define an authentictaion command, and to insist on that command's use if HTTP authentication has been enabled. (Months ago I wrote code to do this, but I've lost that code/patch.) I think 6.0 would be a good time to introduce this change, as there's been a good deal of developer activity on the CLI front.
fred: is this something you might be able to consider for the enhanced CLI?
Yes, sounds like a good idea.
Created attachment 271 [details] patch to add functionality Here's a patch to implement this. Last time I wrote such a patch, I also wrote changes to strings.txt; this time, the only reference to the tie-in to the "authorize" pref is in the cli-api.html documentation. I think editing strings.txt would be a good idea, but wanted to put the basic patch out there for now...
My patch adds a "login" command with usage like login username:pass%20phrase The ":" separator was suggested by somebody when I first looked at this some months ago, but I wonder if it would be better (more like the other commands) to separate username/password with whitespace, e.g. login username pass%20phrase ?
Thanks Martin. Yes I think "login user password" is more coherent with the rest of the API, so I'd vote for that. The real issue with this login thing is how to indicate failure while being coherent. You propose "login successful" but it is the first time in the entire API where the server reply is different than the command. Success is essentially assumed by the API model. One possible way is to have the login command be "login user pass ?" and the server would reply replacing ? by 1 or 0 depending on success or failure... Do we want to implement IP range blocking as well as for the www interface?
Hi, Fred. I don't like the idea of echoing the cleartext password back in the reply: that seems like bad security practice. How about responding to "login user pass ?" with "login user ******** 1" or "login user ******** 0", where the password is always replaced with either a fixed number of "*" characters (which would be preferable) or as many "*" characters as there were decoded password characters (e.g. "a%20b" would decode to "a b" and be represented by "***")? You should also note that this patch also changes CLI behavior when a client attempts a command on a server that requires authentication without first authenticating. For instance, if a client connects to a login-protected server and asks "player count ?", the server will respond "login required". This continues until the "login" command is successful. This seems to me the best approach -- the server cannot simply echo back what the client sent, as in the case of most non-"?"-bearing commands, echoing the client's command indicates success. I think an IP range restriction is a good idea, but should it be the same as for the web IP restrictions? -Peter
Good point for the password, although it was sent as clear text anyway... Right now you can send "blah blah blah" to the CLI and it will happily return you "blah blah blah" without a care in the world that this command did nothing. So the CLI assumes the client knows what it does, it is not designed as an interactive thing for humans, the client is a computer. I therefore see no problem in returning the same command without doing anything if not correctly login. I don't think that's the best, but if we introduce something for login, why not for "blah blah blah" above? That's my point/problem. IP restrictions as for WWW for now, what do you think?
Password: yes, I think there ought to be an option for requiring SSL/TLS on both the HTTP and CLI interfaces, but from a quick read of the SlimProto docs it's apparent that I need to think about this more before submitting a request on that issue. One step at a time. And the first step is trying to secure the CLI as well as the HTTP interface. You have a good point with "blah blah blah". I think the CLI reponse probably ought to include a status code, ala HTTP. So you'd have transactions like login user pass 200 login user ******** player count ? 200 player count 1 blah blah blah 404 blah blah blah Ideally, all the status codes would be registered in a hash in CLI.pm or something. Codes 200-299 would be success, and everything else would denote failure of some sort. So a client attempting commands against a server that required authentication would see something like player count ? 401 player count ? displaynow ? ? 401 displaynow ? ? There are definite advantages to including a return code. A home automation system that tried 00:aa:bb:cc:dd:ee power 1 and saw the response 500 00:aa:bb:cc:dd:ee power 1 (500 = general error) or 404 00:aa:bb:cc:dd:ee power 1 (404 = no such player or command) or some such would know that something was wrong, and not to assume that the player was successfully powered on. That's much cleaner than having to ask the CLI for the display contents & parse the result string to determine if the player's up. But that's likely too much change in an "alpha" release -- not to mention that, at least from what I see in CLI.pm, it appears that Slim::Control::Stdio::executeCmd() doesn't return any clear status code that CLI.pm could translate into a clear CLI status code. How do you think the CLI should respond to "player count ?" and similar queries if authentication were required but the CLI client had not authenticated? Do you think it'd be best just to echo it (and similar "?" commands) verbatim? Then "player count ?" or somesuch could be used as a test by CLI clients -- if the server included a "?" in the response, then authentication is required. BTW, I do need to do more work on my patch, regardless: 1) I need to undef the %authenticated entry when a CLI connection closes (either 'exit' command or network close) (security/memory leak). 2) The login command ought to be recognized even if the client's authenticated (not passed to executeCmd()), and should never result in client being kicked out if authorization is not enabled. 3) HTTP.pm checkAuthorization should be patched so CLI.pm can tell it not to try the special SlimProto "player" passwords, but only the HTTP username/password. Not big issues, but things I should do as well as add the IP restriction test. For simplicity's sake (both SlimServer's and CLI clients'), I'd like the CLI to 1) only test the IP requirement when the CLI connection is initiated. That is, if IP restrictions are added or modified after a CLI client is connected, that CLI client connection will be allowed to continue regardless of its IP address. 2) do not require authentication from a CLI client if authorization was not required when the CLI client sent its first command. That is, if the username/password requirement is enabled after a CLI client is connected and begins to use the CLI, that CLI client connection will be allowed to continue even if it did not use the 'login' command. 3) do not require re-authentication from a CLI client if the username/password for authorization changes after the client successfully uses the 'login' command. That is, if the username or password changes after a CLI client has authenticated with a previous username+password, that CLI client connection will be allowed to continue. None of these are the most "secure" alternative, but without per-CLI-command status codes, I think these behaviors make sense. If we had clear per-command status codes, then I think CLI connections should be re-evaluated if the IP or authorization settings change, to better help users identify when they've made a configuration change that would otherwise shoot them in the foot hours/days/weeks later when their home automation system had to reconnect to the CLI interface.
Pushing this off to post-6.0 unless we've got a solid fix now.
I posted a patch to the list (never applied) that does the following. I believe this could be called a "solid fix". (a) Options in the Security tab of the Server Preferences apply to CLI and HTTP. There is no separate settings for each. (b) CLI security is enforced when the connection is established. If the Server Prefs are changed, existing connections are unaffected. (c) A new "login user password" command is added to the CLI. If successful, it returns "login user **", if not, the connection is dropped by the server. Any other command issues if login is required has the same effect, the connection is dropped. The reason for the disconnection is that the CLI has no status code. There is no indication in the returned CLI data that the command was successful or not. Introducing this would be a good idea for the future but for the meantime, disconnection has the desired effect on any client: something is wrong. This keeps compatibility to a maximum while enforcing the security.
This bug appears to have been fixed in the latest release! If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look. Make sure to include the version number of the software you are seeing the error with.