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] Add "sudo mode" #33955

Open
javiereguiluz opened this issue Oct 11, 2019 · 19 comments
Open

[Security] Add "sudo mode" #33955

javiereguiluz opened this issue Oct 11, 2019 · 19 comments
Labels
Feature Keep open RFC RFC = Request For Comments (proposals about features that you want to be discussed) Security

Comments

@javiereguiluz
Copy link
Member

Description
"Sudo mode" is a security feature which allows web applications to ask users to reenter their passwords before performing some critical task (unless they have reentered it "recently").

I asked around in the Symfony Slack and some people said that it'd be great to add this to Symfony core ... but others disagree arguing that it's trivial to implement it yourself. So, let's discuss about this feature. Thanks!

Example
GitHub for example uses it:

image

@javiereguiluz javiereguiluz added this to the next milestone Oct 11, 2019
@yaffol
Copy link

yaffol commented Oct 11, 2019

The reason I would like this is that it would make it more likely that it would be used 'by default' within apps, and it would raise the overall level of security likely to be implemented within apps based on Symfony. I would certainly use it, and while I have considered implementing it myself, I am nervous of doing things related to security without doing proper due-diligence on them, so I've never yet found the time.

Having a peer-reviewed implementation of this would make me much more likely to use it.

The way I imagine it working could be having another AUTHENTICATED_ status (like AUTHENTICATED_RECENTLY) which could be checked in the same way that you might check AUTHENTICATED_REMEMBERED or similar at the moment.

Thanks for recommending this 👍

@jaikdean
Copy link

@patrickvale has summed up my opinions as well as I could.

@Pierstoval
Copy link
Contributor

To me, a "sudo mode" is mostly an enhancement of the current "remember me" authentication system. This means that we could create a token with an expiration date, such as ExpirableTokenInterface, then, we could use $authorizationChecker->isGranted() with a special AUTHENTICATED_RECENTLY security attribute (like @patrickvale's idea) would check the token type. If token implements the ExpirableTokenInterface, access is granted if the expiration date is not reached, and denied if date is reached.

@javiereguiluz javiereguiluz added Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Oct 11, 2019
@stof
Copy link
Member

stof commented Oct 11, 2019

The way I do something like that is by having some pages requiring IS_AUTHENTICATED_FULLY while most of my app allows IS_AUTHENTICATED_REMEMBERED.
I don't think we need a separate AUTHENTICATED_RECENTLY attribute unless your PHP session lifetime is so long that you could be fully authenticated for too long to be considered recent.

@Pierstoval
Copy link
Contributor

Pierstoval commented Oct 11, 2019

@stof There's also something: if somebody logs in using the password remembered by the web browser, they may not know the actual password, so I think a IS_AUTHENTICATED_FULLY authentication is not enough: we need a 2nd "stronger" authentication to make sure the password is truly known. We could even imagine such 2nd authentication to be different: using captcha, use 2factor auth...

@yaffol
Copy link

yaffol commented Oct 11, 2019

@Pierstoval that's a good point - the ability to 'easily' add 2FA (eg SMS) would be very interesting for some use cases.

@wouterj
Copy link
Member

wouterj commented Oct 11, 2019

@quentinus95 mentioned in the 2FA issue an idea of "identity trust level" that can replace remember me, 2FA and sudo mode: #28868 (comment)

I think that any modern version of the Security component (which we should achieve in Symfony 5) has to contain sudo mode and 2FA. Remember me in the current component is very complex and stopping most changes in the component. So I would be very happy if we can continue thinking about this "identity trust level" and transfer it into a workable issue to see if it works (and if it has any downsides).

@stof
Copy link
Member

stof commented Oct 11, 2019

@wouterj as we already reached the feature freeze schedule, a rework of the whole security component won't make it in the 5.0 release.

@stof
Copy link
Member

stof commented Oct 11, 2019

@Pierstoval sudo mode is not about having a second factor.
If you use a password manager for your github password, that still work for their sudo mode even though you don't know your password (only the password manager does). sudo mode is only about asking to authenticate at higher trust than a remember-me cookie.

@wouterj
Copy link
Member

wouterj commented Oct 12, 2019

@wouterj as we already reached the feature freeze schedule, a rework of the whole security component won't make it in the 5.0 release.

I know. When I talk about Symfony 5, I mean the 5.x lifecycle.

@jkufner
Copy link

jkufner commented Oct 13, 2019

Also, some operations are so sensitive that the sudo mode should not stand for "recently", but rather for that one time event only.

For example, when sending money from my banking app, I have to use 2FA to confirm the payment. Then, when I submit another payment, I have to use 2FA again for the second payment no matter how long ago was the first payment.

This feature (both "one-time" and "recently") should be available in core because it is so easy to implement it wrong and potential damage is too great.

@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?

@yaffol
Copy link

yaffol commented Dec 18, 2020

I re-read this only the other day and I think, yes, it would be very useful. It's a very standard thing to verify that the person requesting a specific action is the real key holder, irrespective of how recently they logged in.

@wouterj
Copy link
Member

wouterj commented Dec 18, 2020

Oh yes, this is still relevant (there are concrete plans to integrate this in 5.3). I've added the Keep Open label, to make sure it will not get auto closed.

@bigfoot90
Copy link

Are there any news on this?

1 similar comment
@Xusifob
Copy link

Xusifob commented Aug 21, 2022

Are there any news on this?

@wouterj
Copy link
Member

wouterj commented Aug 21, 2022

I don't know, do you have any news?

@fabpot
Copy link
Member

fabpot commented Aug 22, 2022

I think there are no questions about "do we want it?". It's more like, "who wants to work on it?" As you know, features do not happen magically, and we need more volunteers to help. The core team is here to help if you have any questions about the implementation or anything else.

@wouterj
Copy link
Member

wouterj commented Aug 22, 2022

In the past, I've outlined some broad implementation ideas in #39308 (under "At some knowledge of authentication factors (sudo mode)")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Keep open RFC RFC = Request For Comments (proposals about features that you want to be discussed) Security
Projects
None yet
Development

No branches or pull requests