Add MFA/TOTP support

[svenseeberg]

It took some time, but now (mostly) @drunken-sod and I have finished a first draft of a MFA implementation with TOTP ( #184 ). I'd expect that this PR is far from being ready to merge, but provides a base for discussing all details. We tried to stick to existing logic. However, especially the handling of the 2nd step of the login process seems a bit "hacky". To improve the it, a refactoring of the session handling would be required.

To provide an overview of what this PR does, I'll copy the addition from the security documentation:

Overview of added features

1. Multi-factor authentication with TOTP for admin and mailbox users.
2. Synchronization of the TOTP secret with a Mail front end, for example Roundcubemail. This enables TRUSTED mail user clients (MUAs) to implement MFA internally.
3. Enable MUAs with allowed IP addresses to log in with username and password. Use this feature with care. It basically deactivates MFA for specified IPs. This feature is intended for mail user clients that implement MFA themselves, for example Roundcubemail. However, this can also be used to deactivate MFA when a VPN is used or other use cases.
4. Allow SMTP, IMAP and POP login with app passwords when a TOTP secret is set. The app passwords cannot be used to log in at Postfixadmin itself. That means only the normal user password plus the TOTP factor allow adding, changing or removing app passwords.

Configuration steps

These features are DEACTIVATED by default because they need to be supported by your MTA/MDA configuration to become effective. Please read carefully through the documentation before activating these features.

To activate those features, run through the following procedure:

1. Change your MDA (and if required MTA) password query. You can take a look at the example query listed in the Postfix-Dovecot-Postgresql-Example.md file. The example should work for Dovecot out of the box.
2. Set up synchronization of TOTP secrets with a mail user client application. This is important. Otherwise MFA will not be used to protect access to mails. Use the mailbox_post_TOTP_change_secret_script setting in the config.inc.php. The mailbox username and domain will be passed as parameters, the shared secret via stdin. For Roundcubemail you can have a look at the scripts/sync-roundcubemail-totp.php example.
3. Activate TOTP and app passwords in the config.inc.php by setting
   $CONF['totp'] = 'YES';
   $CONF['app_passwords'] = 'YES';

And sorry, this is a bit of a monster PR.

Oh, and one large caveat: the sync with Roundcubemail is not yet tested. I'd expect that we first need to update https://github.com/alexandregz/twofactor_gauthenticator to support the same TOTP mechanisms.

[DavidGoodwin]

Thank you for taking the time to submit this! I've left a few comments on the PR for things which stood out immediately. I've not yet tried running / using the code.

[drunken-sod]

Thank's for the comments!
Fixed those issues and a bunch of php notices that were clogging up the logs.
Tested with Postgres, fixed stuff. (The PG workaround for REPLACE, "ON CONFLICT" doesn't work on indices, only fields...).
SQLite not tested I'm afraid as I have odd problems connecting here (unrelated, happens in master also...).

Sqlite is next on the list but I wanted to have this available for testing first.
Cheers

[DavidGoodwin]

try running 'composer psalm' on the code - it's currently highlighting about 26/27 issues ...

[drunken-sod]

Later than I expected (vacation), I finally got the stuff psalm complained about fixed.
One script is moved to scripts/examples/ which is now excluded from psalm as it needs external includes, therefore generating a bunch of undefined... errors otherwise.

[DavidGoodwin]

Thank you - I'll try and use the code soon ... I'm happy to merge it, but would like other people to express support/check it if possible.

[svenseeberg]

I just tested the compatibility of the TOTP secret with https://github.com/alexandregz/twofactor_gauthenticator and it seems that it does not work. I will start on a PR there as well. To get started, I already created an issue: alexandregz/twofactor_gauthenticator#183

[Neustradamus]

To follow

[drunken-sod]

@Neustradamus if you have a chance to test it I would love some bug reports or interface complaints ;-)
The implementation in Postfixadmin is done, what's missing is that we would like to use the same secret in some other systems, e.g. roundcube.

[Jieiku]

What is left before this could be merged in? I am happy to help test.

Thank you - I'll try and use the code soon ... I'm happy to merge it, but would like other people to express support/check it if possible.

I will test it as soon as time permits (hopefully very soon). This would be a very useful feature to have.

[DavidGoodwin]

• I can't see how to turn off TOTP for an admin or normal user once enabled ? (presumably it should be possible to set admin.totp_secret or mailbox.totp_secret to null?)
• The application password thing probably needs a better label on the button than 'Add Exception' ?

Otherwise, from a PHP point of view, it looks good to me....

[drunken-sod]

I can't see how to turn off TOTP for an admin or normal user once enabled ? (presumably it should be possible to set admin.totp_secret or mailbox.totp_secret to null?)

This is exactly the kind of glaring omission I was hoping for ;-)

The application password thing probably needs a better label on the button than 'Add Exception' ?

Hm, yes, it is kind of cryptic isn't it? "Add application-specific password" maybe.
"Password" for the generated password is also a tad ambiguous.

Amends forthcoming soon.

[drunken-sod]

Added "TOTP code (empty to disable)" possibility.
Label for app-passwords fixed.

[DavidGoodwin]

Thank you!

[Neustradamus]

@drunken-sod: Good job!

Merged commits:

• 4d17aa6
• ae47269
• 56b21b7

[Neustradamus]

@drunken-sod and others: Maybe you can look this draft-ietf-kitten-scram-2fa?

• https://datatracker.ietf.org/doc/html/draft-ietf-kitten-scram-2fa

SCRAM is very important for security.

[svenseeberg]

Thanks @DavidGoodwin 🎉

SCRAM is very important for security.

I think that mostly depends on the implementation within the used libraries. But at some point we should have a look. Can you create a new issue for that?

[DavidGoodwin]

@svenseeberg there are a few changes I need to make to the code in master - as far as I can see ....

• admins cannot see/list/modify exemptions their users have added
• exceptions needs to be renamed exemptions (minor)
• i'm not sure why admins would want to create their own app passwords (but the system allows it)
• totp stuff - admins can't remove/revoke totp for their users

Ideally the mailbox edit screen would show something listing totp status / app passwords / exemptions etc?

[DavidGoodwin]

i'm also not convinced the logic is correct around who can add/remove totp codes/exemptions ... i suspect anyone can delete anyone else's atm.

[svenseeberg]

i'm also not convinced the logic is correct around who can add/remove totp codes/exemptions ... i suspect anyone can delete anyone else's atm.

Yes, that would be problematic.

[drunken-sod]

• admins cannot see/list/modify exemptions their users have added

Yes, this would be good

• exceptions needs to be renamed exemptions (minor)

Yes they do, don't they? I'm blushing.

• i'm not sure why admins would want to create their own app passwords (but the system allows it)

If they receive mail it might be especially important for admins for separate their passwords? Saw no reason not to include it as it was no extra effort.

• totp stuff - admins can't remove/revoke totp for their users

Yes, nice glaring omission, thanks ;-)

Ideally the mailbox edit screen would show something listing totp status / app passwords / exemptions etc?

I'll come up with something.

i'm also not convinced the logic is correct around who can add/remove totp codes/exemptions ... i suspect anyone can delete anyone else's atm.

That would be embarrassing.

I'll get to it reasonably soon but probably not this week.

[DavidGoodwin]

Hey @drunken-sod - See also https://github.com/postfixadmin/postfixadmin/tree/topt-app-passwords-fixes - I have sort of started on trying to resolve some of the above issues (but it's not complete yet).

[Neustradamus]

@svenseeberg: Can you reopen the ticket?

• SCRAM-SHA-***(-PLUS) supports #313

[vaelu]

When will this get released?

[DavidGoodwin]

#805 - the application passwords need some work still.

From memory - the dovecot SQL query which checks for app passwords isn't valid (See #802) and there isn't any way for admins to add them on a user's behalf?

https://github.com/postfixadmin/postfixadmin/blob/8451a91fb899f04be67591319f417e96c2ed1ae4/TODO