Skip to content
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.

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

Changing password via roundcube does not function #2185

Open
kiekerjan opened this issue Oct 27, 2022 · 15 comments
Open

Changing password via roundcube does not function #2185

kiekerjan opened this issue Oct 27, 2022 · 15 comments

Comments

@kiekerjan
Copy link
Contributor

Changing the password via the Roundcube settings does not work. See report here.

The error reported via /var/log/roundcubemail/errors.log:
DB Error: [1] near "%": syntax error (SQL Query: UPDATE users SET password=%D WHERE email='a@b.c') in /usr/local/lib/roundcubemail/program/lib/Roundcube/rcube_db.php on line 564 (POST /mail/?_task=settings&_action=plugin.password-save)
The sql update query contains %D which was removed from the 1.6.0 version of roundcube (see 1.6.0 versus 1.5.3)

Several observations:

  • $config['password_algorithm'] = 'clear'; should probably be set to dovecot
  • There is a MIAB password driver in the Roundcube password plugin. No idea if it works. Also needs you to configure the admin password so does not feel nice to use.
@kiekerjan
Copy link
Contributor Author

Created a quick fix to the configuration, see this pull request. However, it does not work:
PHP Error: Password plugin: Failed to execute command: /usr/bin/doveadm pw -s 'SHA512-CRYPT'. Error: doveconf: Fatal: Error in configuration file /etc/dovecot/conf.d/10-ssl.conf line 16: ssl_key: Can't open file /home/user-data/ssl/ssl_private_key.pem: Permission denied in /usr/local/lib/roundcubemail/plugins/password/password.php on line 747 (POST /mail/?_task=settings&_action=plugin.password-save)

If I do a chgrp dovecot ssl_private_key.pem and a chmod g+r ssl_private_key.pem it does work, but I cannot oversee the security consequences of that.

@jvolkenant
Copy link
Contributor

It looks like %D would become %P https://github.com/roundcube/roundcubemail/blob/993b888afe29c383bf45c84f17090f4db96367ba/plugins/password/config.inc.php.dist#L120

If the www-data user can't run doveadm, what about setting %P and $config['password_algorithm'] = 'sha512-crypt';

@kiekerjan
Copy link
Contributor Author

Yeah, I already changed %D into %P. I'll try the sha512-crypt algorithm, that's a good idea.

@kiekerjan
Copy link
Contributor Author

I tested with:

	config['password_query']='UPDATE users SET password=%P WHERE email=%u';
	$config['password_algorithm']='sha512-crypt';
	$config['password_algorithm_prefix']='{SHA512-CRYPT}';

which seems to produce a password that is accepted by Roundcube when logging in.
However, as already noted by ddavness and also seen on the forum, there is an associated issue where editing the sqlite database by roundcube makes it not possible for postfix to access it.

@jvolkenant
Copy link
Contributor

I'll have to retry this on 22.04; using the above config and restarting postfix the password is accepted. I wonder what changed with doveadm in 22.04 too

@kiekerjan
Copy link
Contributor Author

You might want to check /var/log/mail.log on postfix errors. As seen in the issue mentioned by ddavness, Roundcube 1.6.0 introduces an sqlite configuration that's incompatible with postfix. Or simply check if you can send mail on that box.

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Nov 5, 2022

The above pull request fixes the configuration of the password plugin.
There remains the issue where (trying to) change the password causes postfix to stop functioning correctly. This is caused by a PRAGMA set on the sqlite database by the Roundcube password plugin. Journal_mode is set to WAL by the plugin. Postfix is not able to open the sqlite database with WAL enabled.
To recover, give the following command:
sudo sqlite3 users.sqlite 'PRAGMA journal_mode=delete;'

Note: both issues were introduced with the 1.6.0 release of Roundcube.

I can think of several fixes:

  • Patch the Roundcube password plugin
    There is a single line in the Roundcube code that sets the PRAGMA. By commenting it out the issues is fixed, but might have impact on the sqlite db of Roundcube itself. This could also be discussed with upstream.
  • Patch postfix
    I have no idea where to start. This could be discussed with upstream
  • Add a monitor
    Add some sort of timer (e.g. Cron job) to regularly check the sqlite database on the PRAGMA
  • Disable the password plugin
  • [update] use the MiaB driver of the password plugin
    Current implementation only supports admin user to change passwords (it uses the MiaB API), but ddavness has done some work to allow normal users to change their own passwords.

I'm sure there are more variants to think of ;) , so please add your thoughts on this here. My current feeling is that patching sqlite database handling of the password plugin is simplest, and at the same time keeping the functionality.

@downtownallday
Copy link
Contributor

The journal changing pragma was almost certainly intended for roundcube's database only, so this is probably a roundcube bug. That's probably the right avenue to explore, IMO. I don't think commenting out the PRAGMA code entirely is viable as it was added to address another issue (PR is mentioned in the roundcube code).

Another possible fix for your list of several fixes: that there may be a way to juggle the permissions so that roundcube and postfix both have r/w access to the -wal and -shm files created by sqlite. When these files are created, they retain the same mode as users.sqlite, but with the owner and group set to that of the calling process. Setting a common group on the parent directory plus the group sticky bit, would cause the files to inherit the parent's group.

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Nov 5, 2022

The journal changing pragma was almost certainly intended for roundcube's database only, so this is probably a roundcube bug. That's probably the right avenue to explore, IMO. I don't think commenting out the PRAGMA code entirely is viable as it was added to address another issue (PR is mentioned in the roundcube code).

For sure you are right. And because Roundcube uses the same code to open other sqlite databases, it also applies to MiaB's database. However, reading through the original issue I think MiaB is better off without the PRAGMA than with.

Another possible fix for your list of several fixes: that there may be a way to juggle the permissions so that roundcube and postfix both have r/w access to the -wal and -shm files created by sqlite. When these files are created, they retain the same mode as users.sqlite, but with the owner and group set to that of the calling process. Setting a common group on the parent directory plus the group sticky bit, would cause the files to inherit the parent's group.

How did you test this? I tried the following:

  • Added postfix user to group www-data
  • Rebooted to be sure group privileges applied

This did not work for me. The permissions were set as follows (I did not touch these)

-rw-rw-r--  1 root          www-data      36864 Nov  5 17:14 users.sqlite
-rw-rw-r--  1 root          www-data      32768 Nov  5 17:15 users.sqlite-shm
-rw-rw-r--  1 root          www-data          0 Nov  5 17:15 users.sqlite-wal

@downtownallday
Copy link
Contributor

You're right, it doesn't work. I was only looking at this theoretically and didn't test it. After loading a test system, it looks like the permissions are okay as-is. Not exactly sure why postfix would gain access to the -wal and -shm files when not being in the www-data group.

What fixed the issue for me was to remove the chroot flag for the trivial-rewrite and cleanup postfix services (there may be others that need changing as well, I didn't test thoroughly). That's done by setting the chroot field to 'n' for those two services in /etc/postfix/master.cf.

I'm using a modified miab that uses ldap for the user database and doesn't have this issue, so I'm going to take myself out of this. I probably shouldn't have posted in the first place, but it looked interesting. I hope what I provided helps.

@e-fu
Copy link

e-fu commented Nov 6, 2022

sqlite3 users.sqlite ".dump" | sqlite3 users.sqlite_new and renaming the file fixed the corrutped sqlite db for me

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Nov 6, 2022

Found this thread which pretty much describes the postfix issue we're seeing here. The takeaway seems to be that's it's either a permissions issue or not supported (at least not advised) by postfix.

@kiekerjan kiekerjan reopened this Nov 6, 2022
@kiekerjan
Copy link
Contributor Author

I'm sorry, don't know what happened there. Thick fingers probably 😞

@kiekerjan
Copy link
Contributor Author

I like the solution of ddavness. Just to have a choice, I have 2 pull requests which are a little more reviewable ;)

@Macr0Nerd
Copy link

You're right, it doesn't work. I was only looking at this theoretically and didn't test it. After loading a test system, it looks like the permissions are okay as-is. Not exactly sure why postfix would gain access to the -wal and -shm files when not being in the www-data group.

What fixed the issue for me was to remove the chroot flag for the trivial-rewrite and cleanup postfix services (there may be others that need changing as well, I didn't test thoroughly). That's done by setting the chroot field to 'n' for those two services in /etc/postfix/master.cf.

I'm using a modified miab that uses ldap for the user database and doesn't have this issue, so I'm going to take myself out of this. I probably shouldn't have posted in the first place, but it looked interesting. I hope what I provided helps.

In the issue linked above, this fixed my issue as well. For the time being, until a solution is merged, this might be a decent recommended fix.

JoshData pushed a commit to kiekerjan/mailinabox that referenced this issue May 13, 2023
downtownallday pushed a commit to downtownallday/mailinabox-ldap that referenced this issue May 13, 2023
downtownallday pushed a commit to downtownallday/mailinabox-ldap that referenced this issue May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants