ServerContainers / samba

samba - (ghcr.io/servercontainers/samba) (+ optional zeroconf, wsdd2 & time machine) on alpine [x86 + arm]
411 stars 54 forks source link

excessive permissions on timemachine parent folder #55

Closed dtiefnig closed 2 years ago

dtiefnig commented 2 years ago

Hi!

In June last year you committed change befc254 to scripts/samba_create_timemachine_user_dir.sh (now: samba_create_user_dir.sh) adding two lines which force timemachine root folder to be owned by nobody:nogroup and have file mode bits set to 777. While I can't think of any reasonable scenario that could require this, there sure will be a reason why this has been implemented.

Now while this may not seem to matter much in the closed container environment, it may easily become a security issue on the docker host system. Making the directory world readable, will allow every user on the host system to change the directory contents. E.g. rename subdirectories and create new ones with arbitrary (malicious) content. I know nothing about TimeMachine, but as this applies to all multi-user shares (having a path ending with '%U') it may also affect user home directories which are shared that way.

As in my case, I defined a docker volume '/home:/shares/home/' and a samba volume with 'path=/shares/home/%U' this would leave my /home on the host system with 777 permissions. Any local user (attacker) could now rename the home directory of another user (victim) and place a malicious .bashrc in it which will be executed upon login of victim with victim's UID/GID. If done cleverly, victim would not even notice and attacker could at least gain access to all of victim's files and spy on its shell input/output.

Suggested remedy: Fully revert change befc254. Is it really needed? There should be nothing writing to parent directories of samba shares (except samba_create_user_dir.sh), in my very humble opinion. As long as the directory is readable and executable we should change neither ownership nor permissions.

If this still brakes TimeMachine shares, maybe permissions could be restricted to 775? Please note this might still create a security issue on the host system (or any other system which has access to the filesystem if it is shared) under some circumstances, as GID's are not synced between containers and their host. The GID 65533 of nogroup in your container may be used for anything on the host system and every user could be member of it.

Please also note that group "nogroup" in your container does not have any users assigned to it. The primary group of user "nobody" is "nobody". Maybe you wanted to use group "nobody" in the first place. But again, this may just be some TimeMachine / Mac OS weirdness.

Maybe the behaviour should also at least be limited to TimeMachine shares, if it is really necessary and can not be mitigated otherwise. Your entrypoint.sh could specify a parameter to samba_create_user_dir.sh which is passed by samba preexec and enables/disables the chown and chmod.

In any case, if permissions/ownership need to be changed, place a prominent warning in the documentation of SAMBA_VOLUME_CONFIG_myconfigname, making users aware of possible implications.

Using "servercontainers/samba:latest" with "Created": "2022-01-20T09:28:41.6175721Z".

Kind regards Daniel

MarvAmBass commented 2 years ago

Hi Daniel,

thanks for the hint - it seems to no longer make sense here. I'll remove the chmod 777 and chown nobody:nogroup part entirely

it's from a time where this container didn't support users, groups, user ids etc.

kinds regards

Marvin

dtiefnig commented 2 years ago

Hi,

good to hear it's easy to solve. Thanks for the great work btw., really like the image. Seems to be the only one which has it all (incl. avahi and WSD) and it's really easy to deploy.

Cheers, Daniel

MarvAmBass commented 2 years ago

Just finished my tests, the missing chmod 777 doens't seem to have bad side effects. I'll release it and once it's pushed close this issue.

thanks! that's really nice to hear!

And it's really great to see, that the commuity looks over it and tries to improve it, fix bugs etc.

Thanks and have a great day!

Marvin