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

[Security] Native support for 2FA #28868

Closed
quentinus95 opened this issue Oct 14, 2018 · 29 comments
Closed

[Security] Native support for 2FA #28868

quentinus95 opened this issue Oct 14, 2018 · 29 comments

Comments

@quentinus95
Copy link
Contributor

quentinus95 commented Oct 14, 2018

Description
2FA shoud be natively supported by the security component, to get more interoperability between projects and support from the community (as well as external bundles provinding specific 2FA features, e.g. U2F).

There are bundles providing support, but most of them are either outdated or not providing a simple, low level API (with a seamless integration in the Symfony security component). They also do not provide the same guaranteed support as Symfony does.

Example
The latest version of scheb/two-factor-bundle provides a good proof of concept, but the API is too restrictive, high level and some of the validation / handling code for the 2FA is a duplicate from existing code in the security component.

Some of the main low levels features which would be useful:

  • Providers/Challengers: used to implement a way for validating 2FA
  • Activators: used to enable/disabled 2FA for specific providers based on the authentication context
  • SecondFactorInterface, which may be used to persist challenges (same use as the UserInterface, but for 2FA)

I guess a new role equivalent to IS_AUTHENTICATED_ANONYMOUSLY would be required, like in the process described here: https://github.com/scheb/two-factor-bundle/blob/master/Resources/doc/index.md#the-authentication-process

[Edit] See this comment for a proposal to unify sudo mode, MFA and remember me:

@quentinus95 quentinus95 changed the title Native support for 2FA [Feature Request] Native support for 2FA Oct 14, 2018
@quentinus95 quentinus95 changed the title [Feature Request] Native support for 2FA [Security] Native support for 2FA Oct 14, 2018
@madwizard-thomas
Copy link

I also would like to see native support for this, especially since 2FA is becoming a requirement more and more these days.
One of the difficult issues is the semi-logged in state between the first and the second factor login (e.g. the IS_AUTHENTICATED_2FA_IN_PROGRESS state in scheb's library).
I'm working on a webauthn symfony bundle and trying to write a simple demo, ideally without libraries like scheb's to keep things simple. For 2FA I get kind of stuck, ideally I would use the guard authenticator for the username/password login part and then proceed with the second factor before having the token/user fully authenticated. I'm not sure if this is currently possible with the guard authenticator. Maybe with some special token containing a reference to the user account without actually having an authenticated user (similar to scheb's library) but I'm worried this might break in guard.

@julienfastre
Copy link

Thanks for opening this issue. I also think having a native support for 2FA would be great.

Concerning the graph and the state of "partially authenticated" (with "IS_AUTHENTICATED_2FA_IN_PROGRESS"), I would like to discuss one problem we could face.

For some of our application, we store some authorization logic in database. The role does not suffice to check some right, and we do not store any role into the token. In voter, we check that the token contains some User (and we assume there that the user is fully authenticated) and, then, we perform the verification of authorization.

Some example here.

I just fear that, in a state "partially authenticated", the line $token->getUser() instanceof User will return true.

You could argue that we should also check that the token contains the role IS_AUTHENTICATED_FULLY, and I must admit that this should be correct. When we wrote this code (we begin with symfony 2.5 I think), 2FA was never discussed. Checking that the token contains an User was considered easier. And maybe there are also some developer who made the same error.

If it is not too much complicated, I think it would be reasonable to avoid a state "semi authenticated" which might open security issues in application based upon symfony.

@julienfastre
Copy link

Hum... Diving into the api, I show the method TokenInterface::isAuthenticated. It sounds like we should use this method and rewrite our voter.

If I am alone to see a security issue in the state "partially authenticated", I must admit that we should rewrite all our Voters using this method.

We should just ensure that, in the state "partially authenticated", the method isAuthenticated should return false.

@quentinus95
Copy link
Contributor Author

The issue you're describing is a bit the same as when using the remember me feature (IS_AUTHENTICATED_REMEMBERED vs IS_AUTHENTICATED_FULLY).

I'm not completely sure if 2FA is part of the authentication or authorization process. If part of the authentication (I prefer this), using roles may be confusing to determine if the user is fully authenticated or in progress. If part of the authorization, it may allow to have some highly secured areas where 2FA is required and others where it's not. Usage of roles is then convenient.

Maybe adding a kind of "identity confidence" level in the token can solve this and be used also in the remember me feature?

@stof
Copy link
Member

stof commented Oct 30, 2018

The issue is not exactly the same. IS_AUTHENTICATED_REMEMBERED is considered as authenticated. But then, some parts of the app my require being fully authenticated (not trusting remember-me for sensitive things).
For 2FA, you don't want them to be able to do anything else than entering the 2FA code after doing the first step of authentication. It is not about being more trusted, but about having a flow where the authentication involves multiple steps (and so we should be marked as authenticated only after the last step).

@quentinus95
Copy link
Contributor Author

quentinus95 commented Oct 31, 2018

@stof I don't really agree. In both cases what we want to do is setting a minimum level of trust in the current user identity.

When the user IS_AUTHENTICATED_REMEMBERED, we use IS_AUTHENTICATED_FULLY to force authentication to protect specific parts of the app containing sensitive data. I'm not sure using a role here is relevant because IS_AUTHENTICATED_REMEMBERED is clearly not a "role", it's more a level of trust.

I'm starting to believe that what we need is to store a level of identity trust in the token (for instance a simple integer), and use it to protect some routes (maybe using the access controls and/or setting minimum values for firewalls).

This may be used for both remember me feature as well as 2FA. You can force 2FA globally on your app by requiring a high level of trust which would trigger the 2FA mechanism. You can also use it, for instance, on a payment page on a app where you want to send an text message / ask for a U2F code to confirm the payment. You use it to increase the level of trust in the user identity and then grant access.

@stof
Copy link
Member

stof commented Oct 31, 2018

@quentinus95 IS_AUTHENTICATED_REMEMBEREDis not a role. The input of isGranted is not a role, but a permission attribute. RoleVoter allows using roles as permission attributes, but this does not mean that all permission attributes are roles.

@quentinus95
Copy link
Contributor Author

Thanks for the clarification. Do you think having having a kind of identity confidence level stored inside the token (for instance) could make things simpler regarding the issues we are facing for implementing 2FA properly?

@julienfastre
Copy link

julienfastre commented Oct 31, 2018

@quentinus95 As I understand, you want to have a higher level of confidence in authentication for some action's controller, and allow the user to reach other action.

But my use case is to replace completely the username/password authentication by 2FA, without having to rewrite dozens of Voter to check that "the user is fully authenticated" (at least, some user will use 2FA and other not, which will make things more complicated).

In the way I consider the job of a Authentication Provider (but maybe I am wrong), the authentication provider should authenticate completely, without having a "middle" state where the user "is-authenticated-but-not-fully" which is not obvious to understand. In my opinion, the user is authenticated, or not. But I am happy to discuss this.

@quentinus95
Copy link
Contributor Author

Actually, some action's controller may be the whole application.

You could consider a form login gives you 50 points of confidence, and an additional 2FA form can give you 25 additional points. If you want to enable 2FA on your app, you may add an access control in your security.yml to ask for a minimum of 75 points for the path ^/, which would enable 2FA on the whole app.

This kind of system is interesting because you may have many different chained authentication forms depending on the minimum level of trust required, as well as reducing the level in some situations (for instance if you stay inactive during a long period).

I have the feeling that "fully authenticated" does not mean a lot and is against any evolution in the security level we define in our apps. Being able to define a level (has a trust greater than X) may solve this issue.

@MichaelKubovic
Copy link

Just a side-note, we could take some inspiration from the https://github.com/apereo/cas. They have a very robust implementation built around spring & pac4j. One thing I'd like to bring in is the notion of multi-factor authentication (mfa) rather than 2-factor authentication. We should allow for flexibility in a sense that authentication providers could be possibly chained. It's worth reading through their docs this https://apereo.github.io/cas/6.0.x/mfa/Configuring-Multifactor-Authentication.html

@quentinus95 although the score model could be seen as an improvement to authorization, it's is not a requirement for multi-factor authentication imo. could be also added to an existing guard-protected setup. let's focus on the authentication.

@stof An intermediate permission attribute or an alternative will be required in order to control access to some user's protected resources (phone number, email address, screen personalisation data), while the user is in the process of authentication.

@scheb uses a TwoFactorToken in addition to the intermediate permission attribute. An existence of this type of token in token storage indicates that the authentication is in process.

Another thing that scheb does, is decoration of configured authentication providers to intercept the authentication process. I believe this was necessary because of lack of flexibility in a firewall itself. We could add a MultiFactorListener to the chain similary to AccessListener. It can initiate the 2fa flow.

A few more ideas

  • make use of EntryPoint once firewall listener initiates authentication (the configuration of entrypoint is bound to a multifactor authentication provider)
  • the same mechanism that allows access to /login_check and /login_form should be reused for any mfa provider that requires displaying form/validating credentials (I guess it's the role of MultiFactorFirewall)

I'm happy work on a POC as soon as we have these basic low-level integrations agreed.

@scheb
Copy link
Contributor

scheb commented Mar 27, 2019

Another thing that scheb does, is decoration of configured authentication providers to intercept the authentication process. I believe this was necessary because of lack of flexibility in a firewall itself.

Yes, exactly, I can confirm that. It is doing it to intercept any authentication provider. It is used to warp the authenticated token with the TwoFactorToken and return it instead to the AuthenticationManager, to prevent privileges (roles) from leaking into the system.

If there would be a better way doing it, I'd be glad using it. Manipulating DIC with a decorator feels a bit hacky.

@curry684
Copy link
Contributor

curry684 commented Apr 7, 2019

The work on 2FA should be part of updating Symfony Security as a whole, and is tracked in #30914. I like the discussions here so I'll recommend we keep it open and link it from there.

@fmonts
Copy link

fmonts commented Sep 28, 2019

The "partially authenticated" state can be avoided simply by asking the 2FA in the login form, example:
(credits to BitMex)
image

@quentinus95
Copy link
Contributor Author

@fmonts it would not work if 2FA is not enabled for all users nor if SMS or emails are used as a second factor.

@Nek-
Copy link
Contributor

Nek- commented Sep 28, 2019

Btw, isn't this related to #30914 ?

And also this: #31952

@fmonts
Copy link

fmonts commented Sep 30, 2019

@fmonts it would not work if 2FA is not enabled for all users nor if SMS or emails are used as a second factor.

If not enabled you leave the field blank. True for email, SMS are a very different protocol, I think it's expensive and quite outdated today, also every gateway has its own different API, so I don't think it's worth to build a native support in Symfony for SMS...

@quentinus95
Copy link
Contributor Author

@fmonts I don't think Symfony should support a specific SMS provider, but it should provide a way to register a custom "challenger" to add support for it. Forcing the user to have the 2FA field on the login for is too opinionated. Moreover, for FIDO/WebAuthn there is no 2FA field at all.

@wouterj
Copy link
Member

wouterj commented Sep 30, 2019

@Nek- it is related, but that issues serves as a major list (more a milestone than an issue). Having discussions in more targetted issues is better imho.


I would say as soon as we build support for this in core, which would be very nice btw, we should build an MFA implementation instead of just 2FA. We'll then be more flexible.

I think it would make the most sense if MFA is fully part of the authentication phase. Meaning a not fully authenticated user shouldn't be able to enter the authorization phase (thus voters and such). This means it won't break any existing apps. I was thinking about maybe there is way to have a ChainAuthenticator (or the like, maybe authentication manager?) that loops over a list of guard authenticators and only when they are all finished returns an authenticated token.

@quentinus95
Copy link
Contributor Author

quentinus95 commented Oct 1, 2019

@wouterj actually I believe MFA/2FA could be used with the same mecanisms as for sudo mode or remember me.

It would be interesting to have a way to quantify the level of trust we give to an identity. When the user is authenticated, there is no more concept of "fully authenticated" user. We just increase the level of trust.

The same way we use roles for authorization, we could ask for a minimum level of trust to access resources. It would depend on your current level (determined by the way you authenticated, for instance remember me < username password < username password + 2FA < ...).

This would allow to support sudo mode (i.e. re-asking the user password could increase (temporary) the level of trust), remember me (i.e. the level of trust is lower than when using username & password) and MFA (each new factor increases the level of trust).

It could even allow for MFA authentication only (for instance authentication using a temporary code recieved by email or SMS).

It seems very concenient that all these concepts are grouped together, as they are all part of the same idea: quantify how much we trust the identity of the authenticated used (a little bit like when you ask people to sign your PGP key).

@MichaelKubovic
Copy link

What you described seems to me as "level of assurance", referenced in OIDC spec as Authentication Context Class reference.

acr
OPTIONAL. Authentication Context Class Reference. String specifying an Authentication Context Class Reference value that identifies the Authentication Context Class that the authentication performed satisfied. The value "0" indicates the End-User authentication did not meet the requirements of ISO/IEC 29115 [ISO29115] level 1. Authentication using a long-lived browser cookie, for instance, is one example where the use of "level 0" is appropriate. Authentications with level 0 SHOULD NOT be used to authorize access to any resource of any monetary value. (This corresponds to the OpenID 2.0 PAPE [OpenID.PAPE] nist_auth_level 0.) An absolute URI or an RFC 6711 [RFC6711] registered name SHOULD be used as the acr value; registered names MUST NOT be used with a different meaning than that which is registered. Parties using this claim will need to agree upon the meanings of the values used, which may be context-specific. The acr value is a case sensitive string.

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@Nek-
Copy link
Contributor

Nek- commented Dec 18, 2020

Yes

@remoteclient
Copy link

Yes.

@jspeedz
Copy link

jspeedz commented Feb 5, 2021

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

Yes!

@remoteclient
Copy link

I am working with Symfony and Api-Platform using jwt authentication in split cookies.

My customers want a 2FA functionality. So it would be good if this feature will be added.

@wouterj
Copy link
Member

wouterj commented Feb 5, 2021

@remoteclient using https://github.com/scheb/2fa/ is the recommended solution to add 2FA in Symfony.

@fabpot
Copy link
Member

fabpot commented Feb 5, 2021

Let's close as we do have support via scheb/2fa. @scheb does an awesome job and the bundle is even compatible with the next security system.

@fabpot fabpot closed this as completed Feb 5, 2021
@wouterj
Copy link
Member

wouterj commented Feb 5, 2021

Note that I believe we should find the "hacky" bits that scheb/2fa needs to work with Symfony and see if Symfony can improve. I've talked with @scheb about this in the past year and he contributed most improvements himself. I believe the only left-over improvement is adding "some knowledge of authentication factors (sudo mode)", which is covered in #39308 as well. I'm welcoming any help here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests