Authentication broken

[MeindertKempe]

Bug report

Describe the bug

In mpd version 0.23 authentication seems entirely broken, trying to connect with a password gives no permissions and connecting from localhost only default_permissions are given.

Relevant config:

# Permissions #################################################################
#
# If this setting is set, MPD will require password authorization. The password
# setting can be specified multiple times for different password profiles.
#
password                       "password@read,add,player,control,admin"
#
# This setting specifies the permissions a user has who has not yet logged in. 
#
default_permissions       "read"
#
local_permissions          "read,add,player,control,admin"
###############################################################################

Expected Behavior

running mpc --host=password@localhost status returns the status

running mpc --host=localhost status returns status

running mpc --host=localhost play starts playing

Actual Behavior

running mpc --host=password@localhost status returns MPD error: you don't have permission for "status"

running mpc --host=localhost status returns status

running mpc --host=localhost play returns MPD error: you don't have permission for "play"

Version

Music Player Daemon 0.23 (0.23)
Copyright 2003-2007 Warren Dukes <warren.dukes@gmail.com>
Copyright 2008-2021 Max Kellermann <max.kellermann@gmail.com>
This is free software; see the source for copying conditions.  There is NO
warranty; not even MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Database plugins:
 simple proxy upnp

Storage plugins:
 local udisks nfs curl

Neighbor plugins:
 upnp udisks

Decoders plugins:
 [mad] mp3 mp2
 [mpg123] mp3
 [vorbis] ogg oga
 [oggflac] ogg oga
 [flac] flac
 [opus] opus ogg oga
 [sndfile] wav aiff aif au snd paf iff svx sf voc w64 pvf xi htk caf sd2
 [audiofile] wav au aiff aif
 [dsdiff] dff
 [dsf] dsf
 [hybrid_dsd] m4a
 [faad] aac
 [mpcdec] mpc
 [wavpack] wv
 [openmpt] mptm mod s3m xm it 669 amf ams c67 dbm digi dmf dsm dtm far imf ice j2b m15 mdl med mms mt2 mtm nst okt plm psm pt36 ptm sfx sfx2 st26 stk stm stp ult wow gdm mo3 oxm umx xpk ppm mmcmp
 [modplug] 669 amf ams dbm dfm dsm far it med mdl mod mtm mt2 okt s3m stm ult umx xm
 [mikmod] amf dsm far gdm imf it med mod mtm s3m stm stx ult uni xm
 [sidplay] sid mus str prg P00
 [wildmidi] mid
 [fluidsynth] mid
 [ffmpeg] 16sv 3g2 3gp 4xm 8svx aa3 aac ac3 adx afc aif aifc aiff al alaw amr anim apc ape asf atrac au aud avi avm2 avs bap bfi c93 cak cin cmv cpk daud dct divx dts dv dvd dxa eac3 film flac flc fli fll flx flv g726 gsm gxf iss m1v m2v m2t m2ts m4a m4b m4v mad mj2 mjpeg mjpg mka mkv mlp mm mmf mov mp+ mp1 mp2 mp3 mp4 mpc mpeg mpg mpga mpp mpu mve mvi mxf nc nsv nut nuv oga ogm ogv ogx oma ogg omg opus psp pva qcp qt r3d ra ram rl2 rm rmvb roq rpl rvc shn smk snd sol son spx str swf tak tgi tgq tgv thp ts tsp tta xa xvid uv uv2 vb vid vob voc vp6 vmd wav webm wma wmv wsaud wsvga wv wve rtp:// rtsp:// rtsps://
 [gme] ay gbs gym hes kss nsf nsfe rsn sap spc vgm vgz
 [pcm]

Filters:
 libsamplerate soxr

Tag plugins:
 id3tag

Output plugins:
 shout null fifo pipe alsa ao oss openal solaris pipewire pulse jack httpd snapcast recorder

Encoder plugins:
 null vorbis opus lame twolame wave flac

Archive plugins:
 [bz2] bz2
 [zzip] zip
 [iso] iso

Input plugins:
 file io_uring archive alsa qobuz curl ffmpeg nfs mms cdio_paranoia

Playlist plugins:
 extm3u m3u pls xspf asx rss soundcloud flac cue embcue

Protocols:
 file:// alsa:// cdda:// ftp:// ftps:// gopher:// hls+http:// hls+https:// http:// https:// mms:// mmsh:// mmst:// mmsu:// nfs:// qobuz:// rtmp:// rtmpe:// rtmps:// rtmpt:// rtmpte:// rtmpts:// rtp:// rtsp:// rtsps:// scp:// sftp:// smb:// srtp://

Other features:
 avahi dbus udisks epoll icu inotify ipv6 systemd tcp un

Log

Oct 16 03:27:56 mkdesktop mpd[119530]: client: [1] closed
Oct 16 03:27:56 mkdesktop mpd[119530]: client: [1] process command list returned 0
Oct 16 03:27:56 mkdesktop mpd[119530]: client: command returned 0
Oct 16 03:27:56 mkdesktop mpd[119530]: client: process command "currentsong"
Oct 16 03:27:56 mkdesktop mpd[119530]: client: command returned 0
Oct 16 03:27:56 mkdesktop mpd[119530]: client: process command "status"
Oct 16 03:27:56 mkdesktop mpd[119530]: client: [1] process command list
Oct 16 03:27:56 mkdesktop mpd[119530]: client: [1] opened from [::1]:49964
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] closed
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] process command list returned 3
Oct 16 03:27:52 mkdesktop mpd[119530]: client: command returned 3
Oct 16 03:27:52 mkdesktop mpd[119530]: client: process command "status"
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] process command list
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] command returned 0
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] process command "password "password""
Oct 16 03:27:52 mkdesktop mpd[119530]: client: [0] opened from [::1]:49962
Oct 16 03:27:36 mkdesktop mpd[119530]: output: converting in=44100:24:2 -> f=44100:24:2 -> out=44100:16:2
Oct 16 03:27:36 mkdesktop mpd[119530]: output: opened "my_fifo" (fifo) audio_format=44100:16:2
Oct 16 03:27:36 mkdesktop mpd[119530]: output: OutputThread could not get realtime scheduling, continuing anyway: %s
Oct 16 03:27:36 mkdesktop mpd[119530]: output: opened "pulse audio" (pulse) audio_format=44100:24:2
Oct 16 03:27:36 mkdesktop mpd[119530]: decoder: audio_format=44100:24:2, seekable=true
Oct 16 03:27:36 mkdesktop systemd[587]: Started Music Player Daemon.
Oct 16 03:27:36 mkdesktop mpd[119530]: decoder_thread: probing plugin mad
Oct 16 03:27:36 mkdesktop mpd[119530]: output: OutputThread could not get realtime scheduling, continuing anyway: %s
Oct 16 03:27:36 mkdesktop mpd[119530]: playlist: queue song 0:"The Irish Rovers/01 - Albums/2020 - Saints And Sinners/15 - The Irish Reggae Band.mp3"
Oct 16 03:27:36 mkdesktop mpd[119530]: event: RTIOThread could not get realtime scheduling, continuing anyway: %s
Oct 16 03:27:36 mkdesktop mpd[119530]: state_file: Loading state file /home/mk/.config/mpd/state
Oct 16 03:27:36 mkdesktop mpd[119530]: curl: with OpenSSL/1.1.1l
Oct 16 03:27:36 mkdesktop mpd[119530]: curl: version 7.79.1
Oct 16 03:27:36 mkdesktop mpd[119530]: input: Input plugin 'qobuz' is not configured: %s
Oct 16 03:27:36 mkdesktop mpd[119530]: simple_db: reading DB
Oct 16 03:27:36 mkdesktop mpd[119530]: decoder: Decoder plugin 'wildmidi' is unavailable: configuration file does not exist: /etc/timidity/timidity.cfg
Oct 16 03:27:36 mkdesktop mpd[119530]: hybrid_dsd: The Hybrid DSD decoder is disabled because it was not explicitly enabled
Oct 16 03:27:36 mkdesktop mpd[119530]: sndfile: libsndfile-1.0.31
Oct 16 03:27:36 mkdesktop mpd[119530]: opus: libopus 1.3.1
Oct 16 03:27:36 mkdesktop mpd[119530]: vorbis: Xiph.Org libVorbis 1.3.7
Oct 16 03:27:36 mkdesktop mpd[119530]: libsamplerate: libsamplerate converter 'Fastest Sinc Interpolator'
Oct 16 03:27:36 mkdesktop mpd[119530]: server_socket: bind to '0.0.0.0:6600' failed (continuing anyway, because binding to '[::]:6600' succeeded): Failed to bind socket: Address already in use
Oct 16 03:27:36 mkdesktop systemd[587]: Starting Music Player Daemon...

[morganmay]

I am having the same problem after upgrading from 0.22.11 to 0.23.

[MaxKellermann]

First of all, the log you posted does not reflect your description. It does not show a failed status. Please post a new log.

connecting from localhost only default_permissions are given.

Of course. Why would localhost be special? Are you mixing up "regular TCP connection from loopback device" (what you're doing) with "clients connecting on a local socket" (what local_permissions is defined for)?

(A "local socket" is sometimes called "UNIX domain socket", and is a special kind of connection without TCP and without a network device.)

I doubt that this ever worked for you.

Expected Behavior

running mpc --host=password@localhost status returns the status

I tried with your configuration, and this actually returns the status.

running mpc --host=localhost status returns status

And this worked for me, too.

running mpc --host=localhost play starts playing

This does not work, by design, as I explained above.

The only thing that's really broken for you is the "No permission for status" with password. Works for me. What you can do is do a git bisect, that's the only thing that appears useful to me.

And I'm really curious about the "localhost" problem. Please bisect that as well - what was the most recent commit that still worked? As I said, I doubt this has ever worked, and I think this bug report is wrong, but maybe you can prove me wrong.

[justin-sleep]

Authentication appears to have also broken for me on the 0.23 release. With authentication set up:

password "mympdpassword@read,add,control,admin"

Upon running mpc --host=mympdpassword@localhost status, I get MPD error: you don't have permission for "status", and this is output to my logs:

Oct 17 01:21 : client: [0] opened from 127.0.0.1:50324
Oct 17 01:21 : client: [0] process command "password "mympdpassword""
Oct 17 01:21 : client: [0] command returned 0
Oct 17 01:21 : client: [0] process command list
Oct 17 01:21 : client: process command "status"
Oct 17 01:21 : client: command returned 3
Oct 17 01:21 : client: [0] process command list returned 3
Oct 17 01:21 : client: [0] closed

Looks like on my machine, ncmpcpp, mpc, and mpd-mpris cannot authenticate with the latest release.

[MaxKellermann]

@justin-sleep, please bisect (works for me, sigh)

[justin-sleep]

After investigating further, I think this might just be a problem with Arch's package for 0.23. I can't reproduce this with the source code, even when using the same files and build configuration as Arch's package.

[carnager]

After investigating further, I think this might just be a problem with Arch's package for 0.23. I can't reproduce this with the source code, even when using the same files and build configuration as Arch's package.

That's what i experienced too... Reported a bug here: https://bugs.archlinux.org/task/72454

[MaxKellermann]

Hm, I wonder how a distribution can break their MPD package - what's causing this bug? In case anybody wants to report another bug: the Arch package has left debugging enabled - release builds should configure with -Db_ndebug=true. I'm closing this bug - we can reopen this if it turns out that this Arch bug is really caused by a MPD bug.

[tryone144]

I did some testing with different build configurations on my Arch system regarding the authentication issue. It seems something inside handle_password() breaks when enabling compiler optimizations.

compiler / flags	status
gcc v11.1.0 without optimizations	works
gcc v11.1.0 with -O1 or larger	broken
clang v12.0.1 with -O1 or less	works
clang v12.0.1 with -O2 or larger	broken

The assembly output in gdb suggests that the permission variable in handle_password() is optimized away and assumed to be always 0. This results in a "hardcoded" client.SetPermission(0) effectively not granting any permissions:

0x000000000005826a <+138>:  movl   $0x0,0x20f0(%rbx)

I did not have time to do further checks with older versions of gcc/clang or enabling only specific optimizations. However, I am not sure whether this is to be considered a bug of MPD or of the compiler.

MPD v0.22.11 built with identical options, compiler versions and flags works as expected however.

---

Assembly output from gdb (`gcc -O2 -g`):

``` Dump of assembler code for function _Z15handle_passwordR6Client7RequestR8Response: 0x00000000000581e0 <+0>: push %r14 0x00000000000581e2 <+2>: mov $0xffffffffffffffff,%rdx 0x00000000000581e9 <+9>: push %r13 0x00000000000581eb <+11>: push %r12 0x00000000000581ed <+13>: mov %rcx,%r12 0x00000000000581f0 <+16>: push %rbp 0x00000000000581f1 <+17>: push %rbx 0x00000000000581f2 <+18>: mov %rdi,%rbx 0x00000000000581f5 <+21>: sub $0x30,%rsp 0x00000000000581f9 <+25>: mov (%rsi),%rbp 0x00000000000581fc <+28>: mov %fs:0x28,%rax 0x0000000000058205 <+37>: mov %rax,0x28(%rsp) 0x000000000005820a <+42>: xor %eax,%eax 0x000000000005820c <+44>: lea 0x10(%rsp),%r13 0x0000000000058211 <+49>: mov %r13,(%rsp) 0x0000000000058215 <+53>: test %rbp,%rbp 0x0000000000058218 <+56>: je 0x58227 <_Z15handle_passwordR6Client7RequestR8Response+71> 0x000000000005821a <+58>: mov %rbp,%rdi 0x000000000005821d <+61>: call 0x2eb50 0x0000000000058222 <+66>: lea 0x0(%rbp,%rax,1),%rdx 0x0000000000058227 <+71>: mov %rsp,%r14 0x000000000005822a <+74>: mov %rbp,%rsi 0x000000000005822d <+77>: mov %r14,%rdi 0x0000000000058230 <+80>: call 0x119090 <_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12_M_constructIPKcEEvT_S8_St20forward_iterator_tag.constprop.0> 0x0000000000058235 <+85>: lea 0x121524(%rip),%rdi # 0x179760 <_ZL20permission_passwords.lto_priv.0> 0x000000000005823c <+92>: mov %r14,%rsi 0x000000000005823f <+95>: call 0x71520 <_ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_jESt10_Select1stIS8_ESt4lessIS5_ESaIS8_EE4findERS7_> 0x0000000000058244 <+100>: mov (%rsp),%rdi 0x0000000000058248 <+104>: mov %rax,%rbp 0x000000000005824b <+107>: cmp %r13,%rdi 0x000000000005824e <+110>: je 0x5825e <_Z15handle_passwordR6Client7RequestR8Response+126> 0x0000000000058250 <+112>: mov 0x10(%rsp),%rax 0x0000000000058255 <+117>: lea 0x1(%rax),%rsi 0x0000000000058259 <+121>: call 0x2e300 <_ZdlPvm@plt> 0x000000000005825e <+126>: lea 0x121503(%rip),%rax # 0x179768 <_ZL20permission_passwords.lto_priv.0+8> 0x0000000000058265 <+133>: cmp %rax,%rbp 0x0000000000058268 <+136>: je 0x58293 <_Z15handle_passwordR6Client7RequestR8Response+179> 0x000000000005826a <+138>: movl $0x0,0x20f0(%rbx) 0x0000000000058274 <+148>: xor %eax,%eax 0x0000000000058276 <+150>: mov 0x28(%rsp),%rdx 0x000000000005827b <+155>: sub %fs:0x28,%rdx 0x0000000000058284 <+164>: jne 0x582c7 <_Z15handle_passwordR6Client7RequestR8Response+231> 0x0000000000058286 <+166>: add $0x30,%rsp 0x000000000005828a <+170>: pop %rbx 0x000000000005828b <+171>: pop %rbp 0x000000000005828c <+172>: pop %r12 0x000000000005828e <+174>: pop %r13 0x0000000000058290 <+176>: pop %r14 0x0000000000058292 <+178>: ret 0x0000000000058293 <+179>: mov %r14,%r9 0x0000000000058296 <+182>: mov $0x2,%ecx 0x000000000005829b <+187>: mov $0x3,%esi 0x00000000000582a0 <+192>: mov %r12,%rdi 0x00000000000582a3 <+195>: lea 0xcceba(%rip),%rax # 0x125164 0x00000000000582aa <+202>: mov $0xc,%r8d 0x00000000000582b0 <+208>: lea 0xda56a(%rip),%rdx # 0x132821 0x00000000000582b7 <+215>: mov %rax,(%rsp) 0x00000000000582bb <+219>: call 0x117610 <_ZN8Response9VFmtErrorE3ackN3fmt2v817basic_string_viewIcEENS2_17basic_format_argsINS2_20basic_format_contextINS2_8appenderEcEEEE.constprop.0> 0x00000000000582c0 <+224>: mov $0x3,%eax 0x00000000000582c5 <+229>: jmp 0x58276 <_Z15handle_passwordR6Client7RequestR8Response+150> 0x00000000000582c7 <+231>: call 0x2e940 <__stack_chk_fail@plt> End of assembler dump. ```

[MaxKellermann]

Thanks @tryone144, that was the right hint - there's a misplaced pure attribute: https://github.com/MusicPlayerDaemon/MPD/blob/771c46032f4dd60097d46524ef6873a49b051a01/src/Permission.hxx#L35-L37 This breaks because the function has an output parameter, which is a side effect. Of course, output parameters are a bad design, and should be replaced with a std::optional<unsigned> return type. Anybody up to writing a PR for this?