opnsense
Input Field for 2FA Code During Login
Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
- [X] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
- [X] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue
Is your feature request related to a problem? Please describe.
The current implementation of 2FA in OPNsense requires users to manually prepend or append their 2FA token to their password before submitting the login form. This approach is unintuitive and can be confusing, especially for users unfamiliar with this method.
For the purpose of improving user experience and ensuring a smoother, more intuitive authentication process, I propose a change to how 2FA codes are entered during login process.
Describe the solution you like
The solution involves creating a dedicated input field for the 2FA code on the login page, separate from the password field. The system will handle the logic for concatenating the 2FA code with the password server-side during authentication, based on the authserver configuration.
This proposal supports both:
Prepending the MFA code to the password (default TOTP Authserver). Appending the MFA code to the password (TOTP Reverse token order is checked).
The server-side implementation ensures:
No manual appending/prepending is required by the user. Error messages for missing or invalid MFA codes are displayed inline on the login form for a better user experience.
Describe alternatives you considered
- Retaining the current manual appending method: This method is error-prone and not user-friendly.
- Adding client-side logic: While possible, it is less secure and relies on browser functionality.
Additional context
Add any other context or screenshots about the feature request here or links to relevant forum thread or similar
The code change to accomplish this is trivial...This could certainly be improved, but this DOES work for me.
`130a131,139
function isMFARequired($config) { global $config; $authMode = $config['system']['webgui']['authmode'] ?? 'Local Database'; $authServer = array_filter($config['system']['authserver'] ?? [], function($server) use ($authMode) { return $server['name'] === $authMode && $server['type'] === 'totp'; }); return !empty($authServer); }
148a158,159
$showMFAField = isMFARequired($config);
169a181,202
// Concatenate 2FA token with password if MFA is enabled if ($showMFAField) { if (empty($_POST['mfacode'])) { display_login_form(!empty($_POST['usernamefld']) ? gettext('MFA code is required.') : null); exit; } // Sanitize MFA code input $_POST['mfacode'] = preg_replace('/[^0-9]/', '', $_POST['mfacode']); // Check for passwordFirst logic $passwordFirst = isset($authServer[0]['passwordFirst']) && $authServer[0]['passwordFirst'] === '1'; if ($passwordFirst) { // Concatenate password first, then token $_POST['passwordfld'] = $_POST['passwordfld'] . $_POST['mfacode']; } else { // Default: Concatenate token first, then password $_POST['passwordfld'] = $_POST['mfacode'] . $_POST['passwordfld']; } }
411a445,460
<div class="form-group"> <label for="mfacode"><?= gettext("MFA Code:"); ?></label> <input type="text" class="form-control" name="mfacode" id="mfacode" value="" /> </div> <?php endif; ?>
`
@smitty-nieto: Please do a pull request with your additions in https://github.com/opnsense/core/issues/6310
https://github.com/opnsense/core/pull/8241
I referenced PR : 6310 it was showing closed and couldn't pull.
From: Dr. Uwe Meyer-Gruhl @.> Sent: Thursday, January 23, 2025 11:43 AM To: opnsense/core @.> Cc: Mike Smith @.>; Mention @.> Subject: Re: [opnsense/core] Input Field for 2FA Code During Login (Issue #8239)
@smitty-nietohttps://github.com/smitty-nieto: Please do a pull request with your additions in #6310https://github.com/opnsense/core/issues/6310
— Reply to this email directly, view it on GitHubhttps://github.com/opnsense/core/issues/8239#issuecomment-2610548284, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BB5FOQU6PF5LEICIY5G4OY32MES35AVCNFSM6AAAAABVXWQPPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJQGU2DQMRYGQ. You are receiving this because you were mentioned.Message ID: @.***>
But you can add a comment to #6310, because volunteers were being asked for and now we have one... ;-)
I REALLY hope this gets traction. Password+TOTP in the same field is such an antiquated method of 2FA, and none of the 4 password managers I've tested / use will do Password+TOTP without hacks (that I'm not willing to do).
You've proven it's an easy fix, and password managers should be happy with the format!
Just wanted to say thanks @smitty-nieto for taking the time to find a solution, and I'll knock on wood that the devs see it as a positive addition!
Not likely before the login sequence is rewritten to MVC, one of the concerns with this approach is possible unintended information leakage, it's not a great idea to educate an attacked about the password scheme being used before a login has taken place.
The problem is, if we make this optional with another switch, we will cut off future more logical choices to implement options like this, by our knowledge there are also no other (firewall) vendors offering this feature in a similar manner.
Figuring out what the most logical next step would be takes time which we currently are just not willing to spend due to other, fur us more important, priorities.
I understand the prioritizing reason.
But there is no information leakage when the OTP field is presented regardless if OTP is activated and if OTP is being checked at the same time with the password, not giving away if one or the other has failed.
This is a simple question if both parts (password and OTP) are presented in one or two input fields.
I see one small issue with the code I submitted that would validate your concern on exposing if MFA is required or not.
This if statement could be removed which wouldn't expose if 2FA is used or not.
if (empty($_POST['mfacode'])) {
display_login_form(!empty($_POST['usernamefld']) ? gettext('MFA code is required.') : null);
exit;
}
Seems the other issue you have is exposing the 2FA/MFA field since it says we are requiring MFA and thus exposing that to would be hacker?
The way the code is set now, it looks to see if MFA is required and dynamically shows that field. We could show it ALL the time and if 2FA isn't configured, it would just work and only use user/pass fields. If 2FA is configured, it would concat token + password and validate. On error, it wouldn't tell you if it was 2FA or password was the issue.
Another approach, if I should take this on is the way Unifi and other products (not firewalls) handle 2FA. We leave the login screen the way it is, after successful auth, we then paint a new form asking for 2FA. Is that what you prefer? I could code that in I'm sure (would be more than authgui.inc changes).
Another approach, if I should take this on is the way Unifi and other products (not firewalls) handle 2FA. We leave the login screen the way it is, after successful auth, we then paint a new form asking for 2FA. Is that what you prefer? I could code that in I'm sure (would be more than authgui.inc changes).
I think it is less of a problem to give away if MFA is enabled in the first place (which can be cured by having the field regardless), but if you expose that the password is correct, such that you make guessing the correct combination easier. Thus, this is exactly not how to do it correctly.
I think it is less of a problem to give away if MFA is enabled in the first place (which can be cured by having the field regardless),
I concur. If we showed it all the time and only acted on it on the backend based on if TOTP is enabled or not, this should solve all the issues. And with a generic "Username or Password was incorrect" we would not expose if it was user, password, or 2FA that was incorrect.
Ok two things:
- You're leaking a version number metric with showing it always. For every single install out there.
- People are going to hate the second field when they don't use it.
This PR is a start but this was just the tip of the iceberg here and the main reason why we are not going to get involved at this point. Use the code at your own risk in this current state.
Ok two things:
- You're leaking a version number metric with showing it always. For every single install out there.
- People are going to hate the second field when they don't use it.
This PR is a start but this was just the tip of the iceberg here and the main reason why we are not going to get involved at this point. Use the code at your own risk in this current state.
This only shows 2FA/MFA if you have LOCAL + TOTP selected. Otherwise it only shows user and password field. I could add for LDAP + TOTP as well, but wanted to see if this has any traction. I set back to Local Database to test and does in fact only show user / pass.
I believe some don't like the fact that it shows MFA at all since that exposes it is in use instead of popping it after a successful auth. Anyhow. More discussion is good. If we could get the core devs to chime in, I'd be happy to contribute more extensive code. This was proof of concept and honestly, I think it works great. Keeper auto-populates 2FA code.
We run about 20 Business editions for various clients (MSP) ... So I'd say this is high on our list to get done.
What I do not understand is why the responsibility of inserting a password+otp cannot be delegated to the password manager.
e.g. KeepassXC can do it https://github.com/keepassxreboot/keepassxc-browser/issues/1322#issuecomment-832391890
What I do not understand is why the responsibility of inserting a password+otp cannot be delegated to the password manager.
I think it comes down to supporting 2FA so that ALL password managers can handle it as opposed to having to hack or create custom fields and the likes.
Someone mentioned no other firewalls are doing 2FA. We manage a bunch being an MSP...Sonicwall and Fortinet both do 2FA.
While I really don't like the Sonicwalls, they do handle 2FA cleanly.
The example is exactly how I would expect it (and it makes sense from a security perspective). I just want to stress the point that there is more work and maintenance load involved here (the fact that it's easy to get something done does not mean it's the right thing to do and will not cause complains or bug reports later) and worst of all technical debt that needs to be addressed in the form of the legacy layout and backend login procedures... which is the reason why this wasn't done they way it should be done.
...making it a billion times easier to guess the correct password/OTP combination by tackling password first, then OTP? The rule not to expose which of the two factors was wrong is even in the PSD2 (then again, TOTP is not allowed there, either).
Sure, it does not give away if OTP is in place which gives less of a version metric (which IMHO is pointless for reasons I told Franco behind the scenes).
I totally get that an input field that is unused by most people will confuse them, though.
Theoretically, one could keep the security aspect by always using an OTP popup (if OTP is enabled) after the password has been entered - regardless if the password was correct - and checking both after entry. But probably that will confuse people actually using OTP.
On the other hand, IDK if/which password managers can handle a field that is presented in a second step. BTW: I used KeypassXC with the proposed hack for a long time now, so my blood does not boil over this. I concur that it should be done right.
IMO, the argument would just mean a combined field is the best way to go from a security standpoint. ;)
Isn't the point moot anyway when using a second factor that is stored in the same password manager as the first factor?
Could just use a strong password then too, cause anybody who can take over your password manager will have all of your factors needed to log in anywhere.
Yes, correct. That is the very reason why TOTP is forbidden by PSD2: Both password and TOTP are knowledge factors, whereas PSD2 demands two factors of different realms, i.e. knowledge (something only the user knows), possession (something only the user possesses) or inherence (something the user is). They call that strong customer authentication (SCA).
Obviously, that cannot be done with a password manager - one of the reasons european bank customers hate it.
In a way, you could argue that being able to use a password manager to use OTP undermines the security created by it by reducing the need to steal two secrets to one. But to weaken security and gain useability is the user's decision (which arguably is enabled by such a feature),
We will likely implement the proper workflow in some future release, assigning this to myself in the meantime to make it clear that it is on the list, but without a release scheduled.
Upvote for this ;-)
2FA is important, but optional, and the main login screen shouldn't expose whether 2FA is in use or not - this is why the majority of systems that support 2FA and have it enabled pop a second login page just for the token (Cisco signon, PayPal, my bank) all do it that way.
Please implement as soon as practical. I will send beer.
