search

hestiacp / hestiacp

Hestia Control Panel | A lightweight and powerful control panel for the modern web.
https://hestiacp.com
GNU General Public License v3.0
3.31k stars 675 forks source link

[Bug] Unnecessary maintenance of both passdb and userdb files? #2408

Open kpapad904 opened 2 years ago

kpapad904 commented 2 years ago

Describe the bug

Not a "bug" per se, more an opportunity to optimize some v-* scripts.

Currently HestiaCP maintains the info about its mail users in 3 different files:

  1. /usr/local/hesta/data/users/$USER/mail/$DOMAIN.conf
  2. /etc/exim4/domains/$DOMAIN/passwd
  3. /etc/exim4/domains/$DOMAIN/accounts

It seems to me that the accounts file may be unnecessary, since its data is also contained in the passwd file.

Dovecot's passdb and userdb can be separate files, or can be a combined file in /etc/passwd-style layout; but in this case all userdb extra fields must be prefixed with userdb_, otherwise they’re treated as passdb extra fields

HestiaCP is already using the combined passdb + userdb format (and not a separate userdb), because its /etc/exim4/domains/$DOMAIN/passwd files include a quota field prefixed with userdb_ and Dovecot's auth-passwdfile.conf.ext only references passwd:

# more /etc/dovecot/conf.d/auth-passwdfile.conf.ext
passdb {
  driver = passwd-file
  args = scheme=MD5-CRYPT username_format=%n /etc/exim4/domains/%d/passwd
}

userdb {
  driver = passwd-file
  args = username_format=%n /etc/exim4/domains/%d/passwd
}

I could not find any references to the accounts files outside of HestiaCP own code (3 files in /usr/local/hestia/bin/ .

PS: Some background about Dovecot’s passdb vs userdb https://doc.dovecot.org/configuration_manual/authentication/password_databases_passdb/ https://doc.dovecot.org/configuration_manual/authentication/user_databases_userdb/

Tell us how to replicate the bug

Check the code of

/usr/local/hestia/bin/v-add-mail-domain
/usr/local/hestia/bin/v-add-mail-account:
/usr/local/hestia/bin/v-delete-mail-account:

Which components are affected by this bug?

Control Panel Backend

Hestia Control Panel Version

1.5.7

Operating system

Debian 10

Log capture

No response

kpapad904 commented 2 years ago

Please read https://doc.dovecot.org/configuration_manual/authentication/user_database_extra_fields/

This subject is relevant to the idea of storing data in the combined passdb+userdb file (e.g. Exim4 per user send limit proposed in https://github.com/hestiacp/hestiacp/pull/2225#issuecomment-1030569159 )

jaapmarcus commented 2 years ago

I would assume it is for:

https://github.com/hestiacp/hestiacp/blob/590d8e77a686ce9b378ea29bb25e3f110c040a42/bin/v-add-mail-account#L69-L72

We had an huge issues with Exim 4.94.4 getting auto reply / forwarding working properly due to changes with exim and verifying...

kpapad904 commented 2 years ago

I'm saying that accounts doesn't seem to be used by neither Dovecot nor Exim4 (which uses Dovecot for auth).

jaapmarcus commented 2 years ago

It is currently used in

https://github.com/hestiacp/hestiacp/blob/590d8e77a686ce9b378ea29bb25e3f110c040a42/install/deb/exim/exim4.conf.4.94.template#L414-L420

It is indeed not used exim 4.94.3 and lower but only in 4.94.4. They have made a lot of changes in exim it self causing a lot more work and validations to done as it was missing the email user account In the string.

Before we could use local_part as variable. But that has been removed in 4.94.4 and now you re forced to use the some value in the config... 

root@dev:/etc/exim4/domains/test.nl# cat accounts 
info:**info**:jaap:mail:/home/jaap

root@dev:/etc/exim4/domains/test.nl# cat passwd 
info:{MD5}$1$1zii2rsP$password:jaap:mail::/home/jaap:0:userdb_quota_rule=*:storage=0M
jaapmarcus commented 2 years ago

For reference:

https://github.com/hestiacp/hestiacp/blob/590d8e77a686ce9b378ea29bb25e3f110c040a42/install/deb/exim/exim4.conf.template#L414-L420

kpapad904 commented 2 years ago

Ah, I see. I'm testing this on Debian 10 which has Exim 4.92 ...

jaapmarcus commented 2 years ago

That would explain... In Exim 4.94.3 and below it is not used at all. But adding the exception would make it longer...

It would be nice to consolidate it in one line instead of 2... But might need to spend some time on it ...

This is also one of main reason why upgrading Debain 10 to 11 needs the user the current exim4 config with this file. (And manually apply their changes) to it as writing a "export" script would take a *** ton of time...

kpapad904 commented 2 years ago

I understand.

But passwd (passdb) already contains all the fields present in accounts (userdb), doesn't it?

str="$account:$md5:$user:mail::$HOMEDIR/$user:${quota}:userdb_quota_rule=*:storage=${quota}M" userstr="$account:$account:$user:mail:$HOMEDIR/$user"

so it seems we'd have to rewrite the Exim lookups L416-417 to use passwd instead of accounts.

jaapmarcus commented 2 years ago

Yes and include the username in passwd db....

jaapmarcus commented 2 years ago

$account can't be access in the passwd file ....

It can't be on place 0 ...

kpapad904 commented 2 years ago

I'm reading this (I assume you already have tried it)

https://git.exim.org/exim.git/blob/HEAD:/src/README.UPDATING

and this

Tainted file path

The primary changes made to the configuration templates between the old and new version is to address errors related to "tainted" file paths. Exim became more strict about refusing to use files using certain variable name. The "taintedness" refers to certain variables used within a file path in the configuration which could contain unsanitized values.

The new template versions have all been corrected, however the update is not able to check other custom configuration files which might be pulled in as an include option, so it is possible that you may notice some delivery issues and/or these errors in your logs. If this is the case, you need to correct whatever path is "tainted" by replacing these variables by equivalents which are sanitized:

$domain -> $domain_data $local_part -> $local_part_data

Not every variable has a *_data equivalent and most other variables are already sanitized, so please do not start modifying any which don't produce error output. You can find the Exim variable index here:

https://www.exim.org/exim-html-current/doc/html/spec_html/ch-variable_index.html

jaapmarcus commented 2 years ago

I know we have tried a lot of things but can't remember exactly...

kpapad904 commented 2 years ago

Just checked the Exim list archives and apparently others hit the same problem e.g.

I am migrating from exim 4.92 to 4.94, so the tainted data "problem" comes into the focus. I have a transport:

uservacation: driver = autoreply file = /etc/exim4/autoreply/${domain}/${local_part}.msg [...]

After fiddling around with dsearch I ended up with two nested dsearch'es. One for the directory with the domainname and finally one for the file ${local_part}.msg within that directory. So I have a line as follows:

file = ${lookup {${local_part}.msg} dsearch,ret=full {${lookup{$domain} dsearch,ret=full,filter=directory {/etc/exim4/autoreply}}/}}

This works as expected. But is there a more elegant solution ?

and Jeremy Harris replied:

If the router calling that transport, or some previous router in the chain preceding, happened to have done successful domain= and/or local_parts= tests, then the corresponding _data variables will be usable.

https://lists.exim.org/lurker/message/20211214.125846.82d90fc7.en.html

jaapmarcus commented 2 years ago

My Exim knowledge it self is quite limited. It is also not really my hobby to tinker with it. We received the changes that where necessary to get it working from @myvesta. And it seems to be working after a few changes on our side.

We are open for changes regarding Exim4 config. And I don't mind improving it. How ever I am not able to start and play with it. Exim4 config is not really in my "I love" to do it ...

jaapmarcus commented 2 years ago

Just changed:

https://github.com/hestiacp/hestiacp/blob/590d8e77a686ce9b378ea29bb25e3f110c040a42/install/deb/exim/exim4.conf.4.94.template#L416

To

file = /etc/exim4/domains/$domain_data/autoreply.$local_part_data.msg

2022-02-07 06:40:13 1nGwl3-0054TL-4H => info info@xxxxx R=localuser T=local_delivery 2022-02-07 06:40:13 1nGwl3-0054TL-4H == info@xxxxx R=autoreplay T=userautoreply defer (2): No such file or directory: Failed to open file /etc/exim4/domains//autoreply..msg when sending message from userautoreply transport: No such file or directory

https://mox.sh/sysadmin/tainted-filename-errors-in-exim-4.94/

Again Exim4 config if not my hobby...

myvesta commented 2 years ago

yes, exim 4.94+ can't access passwd file...

kpapad904 commented 2 years ago

Just changed:

file = /etc/exim4/domains/$domain_data/autoreply.$local_part_data.msg

Again Exim4 config if not my hobby...

I fully understand you ... Before putting this issue on hold, could you try once more with the syntax I sent above? (I don't have a HestiaCP system with Debian 11 + Exim 4.94 to try it myself)

file = ${lookup {${local_part}.msg} dsearch,ret=full {${lookup{$domain} dsearch,ret=full,filter=directory {/etc/exim4/autoreply}}/}}

Source: https://lists.exim.org/lurker/message/20211214.125846.82d90fc7.en.html

kpapad904 commented 2 years ago

yes, exim 4.94+ can't access passwd file...

So it will refuse to open any file named passwd regardless of its location?

myvesta commented 2 years ago

@kpapad904 permissions of passwd file are problem, I think Exim refuse to open it even Exim is added to mail group... for some reason...

jaapmarcus commented 2 years ago

@kpapad904 Send me a DM via forum.hestiacp.com with your private key. And I willl give you access to a test server of mine... It will allow you to play with it and saves me costantly editing and getting distracted 😄