Skip to content

Don't change owners in /var/mail recursively #2256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MohammedNoureldin wants to merge 8 commits into from
Closed

Don't change owners in /var/mail recursively #2256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9b9cf92
Don't change owners in /var/mail recursively
MohammedNoureldin Oct 19, 2021
2c33b64
lint fix
polarathene Oct 20, 2021
7cb3fda
Change owners in */var/mail* when LDAP is disabled
MohammedNoureldin Oct 20, 2021
361af09
fix: Only change ownership to `/var/mail` subdirs when required
polarathene Oct 23, 2021
f41a4d5
chore: Restore applying only when necessary
polarathene Oct 23, 2021
9f9f47e
chore: Only log when relevant
polarathene Oct 23, 2021
f58cac2
Use LDAP UID as user identifier in LDAP tests
MohammedNoureldin Oct 26, 2021
0a9696a
lint fix
polarathene Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions target/scripts/check-for-changes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ do
# ${LOGIN}:${PASS}:5000:5000::/var/mail/${DOMAIN}/${USER}::userdb_mail=maildir:/var/mail/${DOMAIN}/${USER}
echo "${LOGIN}:${PASS}:5000:5000::/var/mail/${DOMAIN}/${USER}::${USER_ATTRIBUTES}" >>/etc/dovecot/userdb
mkdir -p "/var/mail/${DOMAIN}/${USER}"
_fix_varmail_user_ownership "/var/mail/${DOMAIN}/${USER}"

if [[ -e /tmp/docker-mailserver/${LOGIN}.dovecot.sieve ]]
then
Expand Down Expand Up @@ -214,11 +215,6 @@ s/$/ regexp:\/etc\/postfix\/regexp/
rm /tmp/vhost.tmp
fi

if find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
then
chown -R 5000:5000 /var/mail
fi

supervisorctl restart postfix

# prevent restart of dovecot when smtp_only=1
Expand Down
14 changes: 14 additions & 0 deletions target/scripts/helper-functions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,20 @@ function _obtain_hostname_and_domainname
fi
}

# Assign `docker:docker` UID & GID ownership to accomodate Dovecot
function _fix_varmail_user_ownership
{
local VARMAIL_DIR=${1}

# Fix permissions for Dovecot by ensuring UID/GID is 5000:
# Skips if 1 level deep the UID or GID is already 5000.
if find "${VARMAIL_DIR}" -maxdepth 1 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
then
_notify 'inf' "Fixing ${VARMAIL_DIR} permissions"
chown -R 5000:5000 "${VARMAIL_DIR}" || _shutdown "Failed to fix ${VARMAIL_DIR} permissions"
fi
}

# Call this method when you want to panic (emit a 'FATAL' log level error, and exit uncleanly).
# `dms_panic` methods should be preferred if your failure type is supported.
function _shutdown
Expand Down
1 change: 0 additions & 1 deletion target/scripts/start-mailserver.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ function register_functions

# ? >> Fixes

_register_fix_function '_fix_var_mail_permissions'
[[ ${ENABLE_AMAVIS} -eq 1 ]] && _register_fix_function '_fix_var_amavis_permissions'

[[ ${ENABLE_CLAMAV} -eq 0 ]] && _register_fix_function '_fix_cleanup_clamav'
Expand Down
14 changes: 0 additions & 14 deletions target/scripts/startup/fixes-stack.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,6 @@ function fix
touch /dev/shm/supervisor.sock
}

function _fix_var_mail_permissions
{
_notify 'task' 'Checking /var/mail permissions'

# fix permissions, but skip this if 3 levels deep the user id is already set
if find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
then
Comment on lines -20 to -22
Copy link
Copy Markdown
Member

@polarathene polarathene Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to have been added to reduce startup overhead of redundantly applying the permissions to contents of multiple GB in size.

Related, this code snippet has been copied into check-for-changes.sh, which will also apply chown -R 5000:5000, as identified here.

You will want to apply your changes to this too:

docker-mailserver/target/scripts/check-for-changes.sh

Lines 217 to 220 in 34ba3c2

if find /var/mail -maxdepth 3 -a \( \! -user 5000 -o \! -group 5000 \) | read -r
then
chown -R 5000:5000 /var/mail
fi

_notify 'inf' 'Fixing /var/mail permissions'
chown -R 5000:5000 /var/mail || _shutdown 'Failed to fix /var/mail permissions'
else
_notify 'inf' 'Permissions in /var/mail look OK'
fi
}

function _fix_var_amavis_permissions
{
local AMAVIS_STATE_DIR='/var/mail-state/lib-amavis'
Expand Down
1 change: 1 addition & 0 deletions target/scripts/startup/setup-stack.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ function _setup_dovecot_local_user
# ${LOGIN}:${PASS}:5000:5000::/var/mail/${DOMAIN}/${USER}::userdb_mail=maildir:/var/mail/${DOMAIN}/${USER}
echo "${LOGIN}:${PASS}:5000:5000::/var/mail/${DOMAIN}/${USER}::${USER_ATTRIBUTES}" >> /etc/dovecot/userdb
mkdir -p "/var/mail/${DOMAIN}/${USER}"
_fix_varmail_user_ownership "/var/mail/${DOMAIN}/${USER}"

# Copy user provided sieve file, if present
if [[ -e "/tmp/docker-mailserver/${LOGIN}.dovecot.sieve" ]]
Expand Down
2 changes: 2 additions & 0 deletions test/mail_with_ldap.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ function setup_file() {
-e DOVECOT_TLS=no \
-e DOVECOT_PASS_FILTER="(&(objectClass=PostfixBookMailAccount)(uniqueIdentifier=%n))" \
-e DOVECOT_USER_FILTER="(&(objectClass=PostfixBookMailAccount)(uniqueIdentifier=%n))" \
-e DOVECOT_USER_ATTRS="=uid=%{ldap:UID},=gid=5000,=home=/var/mail/%Ln,=mail=maildir:~/Maildir" \
Copy link
Copy Markdown

@Murazaki Murazaki Nov 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that by adding Maildir here, the path to the mail folder changed to /var/mail/<domain>/<user>/Maildir which led to the test suite error. (I'm using this scheme too but the test suite might not have been doing it)

Edit: /var/mail/%Ln also does not consider the domain name actually, so it should give /var/mail/<user>/Maildir here

Copy link
Copy Markdown
Contributor Author

@MohammedNoureldin MohammedNoureldin Nov 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing in this, @Murazaki! Something still missing apparently, could you check it and commit the required changes in your opinion? I have been working on another important issue in the last few days, maybe we can close this permission issue also if we work on this together.

Copy link
Copy Markdown

@Murazaki Murazaki Dec 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Mohammed. I tried here but couldn't finish either. There might be a file permission issue:
https://github.com/Murazaki/docker-mailserver/tree/patch-1

-e DOVECOT_PASS_ATTRS="UID=user,userPassword=password" \
-e REPORT_RECIPIENT=1 \
-e ENABLE_SASLAUTHD=1 \
-e SASLAUTHD_MECHANISMS=ldap \
Expand Down