mpd: Support blacklisting MPD commands in the server.

Default blacklist set to listall and listallinfo.

This change has been done to avoid clients being able to call "bad" MPD
commands which are often misused to try and keep a client db. Note that
this change will break some MPD clients, but the blacklist can be controlled
via config to allow opting out for now.
This commit is contained in:
Thomas Adamcik 2015-02-18 00:13:24 +01:00
parent 4a38d1722c
commit e4ba4b3e5f
8 changed files with 40 additions and 6 deletions

View File

@ -84,6 +84,11 @@ v0.20.0 (UNRELEASED)
- Share a single mapping between names and URIs across all MPD sessions. (Fixes:
:issue:`934`, PR: :issue:`968`)
- Add support for blacklisting MPD commands. This is used to prevent clients
from using `listall` and `listallinfo` which recursively lookup the entire
"database". If you insist on using a client that needs these commands change
:confval:`mpd/command_blacklist`.
**Audio**
- Deprecated :meth:`mopidy.audio.Audio.emit_end_of_stream`. Pass a

View File

@ -99,3 +99,10 @@ See :ref:`config` for general help on configuring Mopidy.
``$hostname`` and ``$port`` can be used in the name.
Set to an empty string to disable Zeroconf for MPD.
.. confval:: mpd/command_blacklist
List of MPD commands which are disabled by the server. By default this
setting blacklists `listall` and `listallinfo`. These commands don't fit
well with many of Mopidy's backends and are better left disabled unless
you know what you are doing.

View File

@ -24,6 +24,7 @@ class Extension(ext.Extension):
schema['max_connections'] = config.Integer(minimum=1)
schema['connection_timeout'] = config.Integer(minimum=1)
schema['zeroconf'] = config.String(optional=True)
schema['command_blacklist'] = config.List(optional=True)
return schema
def validate_environment(self):

View File

@ -163,6 +163,11 @@ class MpdDispatcher(object):
def _call_handler(self, request):
tokens = tokenize.split(request)
# TODO: check that blacklist items are valid commands?
blacklist = self.config['mpd'].get('command_blacklist', [])
if tokens and tokens[0] in blacklist:
logger.warning('Client sent us blacklisted command: %s', tokens[0])
raise exceptions.MpdDisabled(command=tokens[0])
try:
return protocol.commands.call(tokens, context=self.context)
except exceptions.MpdAckError as exc:

View File

@ -87,3 +87,12 @@ class MpdNotImplemented(MpdAckError):
def __init__(self, *args, **kwargs):
super(MpdNotImplemented, self).__init__(*args, **kwargs)
self.message = 'Not implemented'
class MpdDisabled(MpdAckError):
error_code = 0
def __init__(self, *args, **kwargs):
super(MpdDisabled, self).__init__(*args, **kwargs)
assert self.command is not None, 'command must be given explicitly'
self.message = '"%s" has been disabled in the server' % self.command

View File

@ -6,3 +6,4 @@ password =
max_connections = 20
connection_timeout = 60
zeroconf = Mopidy MPD server on $hostname
command_blacklist = listall,listallinfo

View File

@ -5,6 +5,7 @@ import warnings
def _is_pykka_proxy_creation():
return False
stack = inspect.stack()
try:
calling_frame = stack[3]

View File

@ -16,6 +16,7 @@ class MpdDispatcherTest(unittest.TestCase):
config = {
'mpd': {
'password': None,
'command_blacklist': ['disabled'],
}
}
self.backend = dummy_backend.create_proxy()
@ -26,14 +27,18 @@ class MpdDispatcherTest(unittest.TestCase):
pykka.ActorRegistry.stop_all()
def test_call_handler_for_unknown_command_raises_exception(self):
try:
with self.assertRaises(MpdAckError) as cm:
self.dispatcher._call_handler('an_unknown_command with args')
self.fail('Should raise exception')
except MpdAckError as e:
self.assertEqual(
e.get_mpd_ack(),
'ACK [5@0] {} unknown command "an_unknown_command"')
self.assertEqual(
cm.exception.get_mpd_ack(),
'ACK [5@0] {} unknown command "an_unknown_command"')
def test_handling_unknown_request_yields_error(self):
result = self.dispatcher.handle_request('an unhandled request')
self.assertEqual(result[0], 'ACK [5@0] {} unknown command "an"')
def test_handling_blacklisted_command(self):
result = self.dispatcher.handle_request('disabled')
self.assertEqual(result[0], 'ACK [0@0] {disabled} "disabled" has been '
'disabled in the server')