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

[feature req] Run optional script on change of mailbox password... #623

Open
ean365 opened this issue May 22, 2022 · 6 comments
Open

[feature req] Run optional script on change of mailbox password... #623

ean365 opened this issue May 22, 2022 · 6 comments

Comments

@ean365
Copy link

ean365 commented May 22, 2022

Add feature to specify a script to run on change of user's mailbox password. Pass username, old password, and new password to the script, and receive back success/fail status and a status message. Run the script prior to updating the database, and only update the database if the script succeeds, and display/log success message passed back. If it fails, do not update password, display/log error message passed back.

This is useful and necessary in cases, like mine and many others, when using Dovecot with mail_crypt, Must know when the user changes their password, because the user's encrypted mailbox needs to be rekeyed with the new password. Must have username, old password, and new password in order to do this via doveadm mailbox cyptokey password.

@DavidGoodwin
Copy link
Member

we do have $CONF['mailbox_postpassword_script'] .... (I don't use it, but I think that's being run after the database has been updated, which isn't what you're asking for?)

$CONF['mailbox_postpassword_script'] = '';
and

#408

and

#421

@ean365
Copy link
Author

ean365 commented May 22, 2022

Thanks for patiently and kindly knocking some of my brain cells loose, David! Ugh. I even forgot that I had commented on this a year ago when I was working on it last (#441). Catching up... Glad to see it implemented, and looks like @BotoX worked in my suggestion to pipe in the passwords instead of placing on the command line.

Yes, "postpassword" is great, but actually changing it to "prepassword" (*before* password update in the database) would be better, in my estimation, working in the way I explained above. That way if the script failed for some reason, it would cancel the password change in the database. (Encrypted mailboxes are nice and private and secure, but hazardous when dealing with password changes and rekeying -- need to make sure everything is in sync/agreement.)

@DavidGoodwin
Copy link
Member

We should really use a database transaction -

  1. begin transaction
  2. change database
  3. run script, if OK, commit transaction, else roll back?

@ean365
Copy link
Author

ean365 commented May 24, 2022

That'll work, too.

@DavidGoodwin
Copy link
Member

Hm, well, in this case I think we could wrap the calls into a transaction, but I'm not sure it's a good idea - see
https://github.com/postfixadmin/postfixadmin/compare/feature-save-in-transaction

Problems :

  • Some postSave() calls might have an external script running, which queries the database; they won't see the updated state of the database until commit calls, which is after they run.
  • Code is in the PFAHandler, I suspect it should really be in the MailboxHandler to limit the impact the change might have

@ean365
Copy link
Author

ean365 commented Jun 11, 2022

Sorry, this turned into a long response. Please excuse any didact or pedant...

(I haven't looked at PFA code in a while.) My first thought is that I tend to agree that it would be best to keep all actions in MailboxHandler that work on or relate directly to a user's Mailbox. At very least... maybe changing a password should be a fairly atomic action in the database... most especially and critically when running Postfix with Dovecot and mail_crypt. Talking it though, it's not entirely clear to me the best way to implement it...

No external script should ever be doing, nor should be able to do, much of anything other than look at metadata, such as size/quota, with a user's mail when the mailboxes are all encrypted. So, I'm not sure what trouble we could get into if an external script misses a password change until it runs again, might not be an issue, same as any other change that an external script would miss if run between a change and commit. Then again, somebody will think of something that could break it, and maybe the external scripts themselves would need to coordinate notification of exceptional changes that might impact them, hold off running, or handle appropriately otherwise, etc. Then again, PFA is a great tool, and maybe it can help make such things easier to organize and administer. Your addition of the various pre- and post- scripts are a definite step in that direction.

Specifically with regard to systems implementing Postfix with Dovecot and mail_crypt on a per-mailbox basis:

  • The user's password is not just used for authentication, but also to encrypt a user's Mailbox storage. More specifically, the user's password encrypts (and decrypts) the private key to their Mailbox.

  • When a user makes a change to their authentication password, a corresponding change must be made to their Mailbox encryption password by making an external call to doveadm mailbox cryptokey password ....

  • When running Dovecot with mail_crypt on a per-mailbox basis, not all Mailboxes need to be encrypted and Dovecot works with it either way, but if a user's Mailbox is encrypted, then they can not, and should not be able to, reset a forgotten password. A forgotten password/crypto-key basically makes all stored encrypted mail inaccessible forever. So will an improperly handled password change!

Assuming that a user knows their current password and wishes to change it, the password change sequence should be:

  • User provides PFA dialog with old-password and new-password and confirmed-new-password (plaintext).

  • PFA confirms that the new-password and confirm-new-password match (plaintext).

  • PFA calls pre-password-change script and (selectively/optionally) either provides username and old-password, new-passowrd, confirmed-new-password on the script command line, or through a pipe, or a combination of both methods of parameter passing (all plaintext). (In the case of changing a password, the password info will need to be piped to the script so that it does not otherwise accidentally appear to any user on a well-timed ps listing while the script is running. In the case of Mailboxes running mail-crypt, this will be a script or direct call to doveadm mailbox cryptokey password ....)

  • Pre-password-change script does its thing and returns a success or failure code, as well as a human readable message.

  • PFA checks return status of the pre-password-change script and...

    • Upon success, both logs and updates the database accordingly and commits all changes.
    • Upon failure, makes no changes to the password in the database, and both logs and displays the returned error code and message to the user.
    • Might also want to do something smart if it never returns, times-out?
  • Similar to above, a different specific call to doveadm must (or used to have to) be made when creating a new user's encrypted mailbox. May be implemented within pre- and post- user/mailbox creation scripts? Also, pre- and post- user/mailbox deletion scripts?

I hope all above helps in some way.

Note: The rest of the following is not necessary, as it can probably be handled nonetheless within the various appropriate pre- and post- scripts that you have been implementing. Though, implemented simply with no additional features, it would potentially mean that these external scripts will need to add and check additional custom fields within the database, which could require database permissions and passwords for the external scripts that we might otherwise not want or need to provide for security purposes. So, it might be useful and nice to add "options" features to PFA and keep within one database...

Aside from the pre-password-change script that we are discussing, it would be useful if PFA had the following helpful features (I believe another PFA contributor may have broached this as well)...

Add specific support for Dovecot mail_crypt. Add a check-box option to PFA that indicates whether the domain mail system and/or an individual user's mailbox should be implementing per-mailbox encryption, and store this in the domain's and each user's mailbox database entry. Then make the appropriate calls to doveadm, thus having support for mail_crypt built in to PFA.

Short of that, perhaps this could even be accomplished more generically for all pre- and post- scripts (i.e., not specific to mail_crypt or password change), simply by adding a "mailbox-options" field to each user's mailbox database entry that allows a list of enumerated (free-form? arbitrary?) options to be stored, and a way of entering, viewing, and modifying these only on the admin page for each user's mailbox. Then, when an external pre- or post- script runs, it can at least access the database and check the "mailbox-options" field for any relevant options. Default options could be stored for each domain mail system entry.

If scripts are not permitted to access the database, for good security purposes, then another way to accomplish this would be to pass the relevant options directly to the script. The mapping of options relevant to each script could be accomplished by PFA using a simple matrix with specified (free-form? arbitrary?) options down the left side and a check-boxes, one for each pre- and/or post- script, across each row, indicating whether to pass that option to the script each time it is called for that user. Again, default options could be stored for each domain mail system entry.

Sorry, lots here to consider. One step at a time. I would and will be happy to help further define, code, and implement this. However, don't have the bandwidth, at least at the moment. Very much appreciate your time, dedication and involvement!

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

2 participants