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

Adding hostNET Password Driver #5786

Closed
wants to merge 3 commits into from
Closed

Conversation

jtkoerting
Copy link
Contributor

New driver file to support hostNET Managed-Root Servers.

New driver file to support hostNET Managed-Root Servers.
@jtkoerting jtkoerting changed the title Create hostnet.php Adding hostNET Password Driver Jun 3, 2017
This was referenced Jun 4, 2017
@alecpl
Copy link
Member

alecpl commented Jun 4, 2017

What software is hostNET using? Is this available for other hosting providers or hostNET only? I wouldn't want to merge provider-specific code.

@jtkoerting
Copy link
Contributor Author

I guess every server using the dovecot 2 mailuser file format can use this. This is why I added the password_hostnet_mailuserfile option, but I can't guarantee that it's working for everyone.

We have roundcube preinstalled for our customers since years and they are using in on >500.000 Mailboxes here, so the patch would be great for many people as they do there roundcube updates on their own on our managed-root systems.

As you have other provider-specific code, I thought that is no general problem. We are happy to do some donation if this is of help, as this would help us and our customers.

@alecpl
Copy link
Member

alecpl commented Jun 5, 2017

We have indeed one (domainFactory), I don't know how that happened. We'll consider this. You can make a donation anyway ;).

@jtkoerting
Copy link
Contributor Author

I did. And many thanks for considering that. I'm sure it will help many roundcube users!

@thomascube
Copy link
Member

Donation thankfully received :-)

If this is generic for dovecot it should probably be named something like "dovecot_mailuser" or at least be described in the README file.

@alecpl
Copy link
Member

alecpl commented Jun 6, 2017

I think the plugin name should be dovecot_passwdfile (to match with dovecot's naming, https://wiki.dovecot.org/BasicConfiguration). Also, instead of using doveadm directly you could use password::hash_password($password) method. So, it could be configured with password scheme. Then configuration would look like this:

$config['password_driver'] = 'dovecot_passwdfile';
$config['password_algorithm'] = 'sha512-crypt';
$config['password_algorithm_prefix'] = '{SHA512-CRYPT}'; // optional, I suppose
// or if you prefer dovecot way:
$config['password_algorithm'] = 'dovecot';
$config['password_dovecotpw'] = '/usr/local/sbin/doveadm pw';
$config['password_dovecotpw_method'] = 'SHA512-CRYPT';
$config['password_dovecotpw_with_method'] = false; // or true

renamed the driver, removed hostnet specific config option and changed the code to be easily adopted for vanilla dovecot 2 passwd-file environments.
@jtkoerting
Copy link
Contributor Author

jtkoerting commented Jun 8, 2017

I rewrote and renamed the plugin, respecting your concerns.

One remark might be allowed for me: If you have even ONE provider specific module - thus supporting non opensource - you should allow this to everyone else. Otherwise, you should remove all provider specific code. An excuse like 'This module was always here...' is not fair. :)

By now, I have the code that way, that it could be used in any vanilla dovecot 2 environment using the dovecot passwd-file format, files and methods given in dovecot/conf.d/auth-passwdfile.conf.ext

On the other hand - and please keep at least this for us - I put in a little piece of code which is checking, if this roundcube instance is running on our own BSD derivate (hostBSD) to set some other defaults.

That way it can be used nearly out of the box for our customers and tweaked for everyone else using the dovecot 2 passwd-file driver for user and passwort authentication.

One remark: the password::hash_password($password) (crypt() function) is not working/is not compatible with (at least) the SHA512-CRYPT used from doveadm pw - so, it shouldn't be used here.

Only these (existing) plugins/password/config.inc.php options are needed:

$config['password_driver'] = 'dovecot_passwdfile'
$config['password_dovecot_passwdfile_path']: The path of your dovecot passwd-file '/path/to/filename' as set in dovecot/conf.d/auth-passwdfile.conf.ext
$config['password_dovecotpw']: Full path and 'pw' command of doveadm binary - like '/usr/local/bin/doveadm pw'
$config['password_dovecotpw_method']: Dovecot hashing algo (http://wiki2.dovecot.org/Authentication/PasswordSchemes)
$config['password_dovecotpw_with_method']: T_rue if you want the hashing algo as prefix in your passwd-file_

Please let me know if this is fine for you this way.

@jtkoerting
Copy link
Contributor Author

ouch - my mistake. This one is new:

$config['password_dovecot_passwdfile_path']: The path of your dovecot passwd-file '/path/to/filename' as set in dovecot/conf.d/auth-passwdfile.conf.ext

@thomascube
Copy link
Member

Fine, thanks a lot!

Now if you could squash your 3 commits into one, it'd be perfect for merging into master.

@thomascube
Copy link
Member

One thing to note though: with file_get_contents() > (alter content) > file_put_contents() there's the chance that another process has changed the file in the meantime and those changes are lost due to a complete overwrite. Consider to lock the file while updating or use something like sed -i to replace the one entry inline.

@alecpl
Copy link
Member

alecpl commented Jun 27, 2017

Also, are you sure about escapeshellcmd() use?

@jtkoerting
Copy link
Contributor Author

sorry guys - completely filled up with other jobs at my company atm. I will come back asap.

Copy link
Member

@alecpl alecpl left a comment

Choose a reason for hiding this comment

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

Waiting for the fixes to the last couple of comments.

@alecpl
Copy link
Member

alecpl commented Apr 25, 2021

dovecot_passwdfile already exists.

@alecpl alecpl closed this Apr 25, 2021
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

Successfully merging this pull request may close these issues.

3 participants