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

Authkey / cookie login security. #5266

Closed
SamMousa opened this issue Sep 30, 2014 · 14 comments
Closed

Authkey / cookie login security. #5266

SamMousa opened this issue Sep 30, 2014 · 14 comments
Labels
type:docs Documentation

Comments

@SamMousa
Copy link
Contributor

According to the documentation the authkey adds extra security to the remember me cookie.

I am confused as to how it does this.

  1. As far as I can tell not using authkey will basically disable all security since the cookie data is not even signed.
  2. Adding an authkey in the way proposed in the docs (adding a column to the user table with a random string) is essentially the same as saving the users' password in a cookie.

Why not instead sign the cookie!?!?!

  1. Add the user ID.
  2. Add the creation or expiration date (creation date allows you to change validity on the application side).
  3. Add a signature using the applications' secret key.
  4. Add a signature using the users' password hash.
  5. Set it to HTTP ONLY.

Basically the documentation should say "the auth key adds very little to the otherwise totally insecure remember me cookie".

By implementing the above suggestions:

  • A user invalidates all remember me cookies by changing his password.
  • The application can enforce expiration dates (a cookie expiration date cannot be enforced).
  • The application can provide feedback to the user in case app signature passes but users' does not (ie: Your cookie has been invalidated becasue you changed your password. Please login again.)
@qiangxue
Copy link
Member

Cookies are already signed and use httpOnly. See yii\web\Request::enableCookieValidation.

@qiangxue
Copy link
Member

Do you have any other concerns?

@SamMousa
Copy link
Contributor Author

Ah, i found it.
As far as I can tell HttpOnly is not set to true however. The signing is done by the Response object but HttpOnly is not set anywhere and it defaults to false.

Still the other concerns remain valid:

  1. One cannot invalidate a login cookie, ever.
  2. One cannot enforce validity periods.
  3. EnableAutologin should be forced to false if cookievalidation is disabled. (Developers who do not go deep into the source code could assume a remember me cookie uses a random token in which case signing it is not relevant since it is supposed to be unguessable).

(Not sure why adding the duration to the cookie, since afaik you cannot reliably tell when the cookie was created).

@SamMousa
Copy link
Contributor Author

  1. Could be solved by implementing getAuthKey to return a value derived from the users password hash. That way the cookies are invalidated when a user changes his or her password.

@qiangxue
Copy link
Member

One cannot invalidate a login cookie, ever.

That's why we have authKey which can be regenerated in this case to invalidate the login cookie.

One cannot enforce validity periods.

Again, this can be implemented with the help of authKey and validateAuthKey().

EnableAutologin should be forced to false if cookievalidation is disabled.

cookieValidation prevents cookie tempering. Even if you turn off cookieValition (which you should have very good reason to do), the presence of authKey will still help to make sure you cannot make up a cookie.

@SamMousa
Copy link
Contributor Author

I agree but the docs suggest authkey "improves" security, which implies there is security without it.

I mean all these things are not immediately clear from the authkey / validateAuthKey documentation.

Validity should be part of the cookie validation; since changing the expiration date is tampering with the cookie.
Also, why include the duration if you are not even using it?

@samdark samdark added the type:docs Documentation label Sep 30, 2014
@qiangxue
Copy link
Member

I agree but the docs suggest authkey "improves" security, which implies there is security without it.

Isn't this true according to my description above? Without the authKey and if you turn off cookieValidation, you can fake a cookie and act as an arbitrary user.

I mean all these things are not immediately clear from the authkey / validateAuthKey documentation.

The requirement such as expiration of authKey is application specific. I don't think we should explain it in the doc. It may be suitable as a cookbook tutorial.

Also, why include the duration if you are not even using it?

We ARE using duration to set cookie expiration time.

@SamMousa
Copy link
Contributor Author

Okay.. Note however that duration is not used in the sense that a cookie expiration time is not enforced (i can edit it in my browser).
So if you are signing cookies it makes sense to include the date it was signed in the data as well; that way it will become invalid after the timeout the developer sets.

@qiangxue
Copy link
Member

This is by design - duration is used to set cookie expiration time, which as you said, can be modified as you want on the client side. If you want to enforce strict auth status duration that doesn't allow client modification, you have to implement it yourself (you can do so by manipulating authKey to combine some timestamp information).

@SamMousa
Copy link
Contributor Author

I understand how it works, but this is clearly not correctly documented:

Cookie Validation ¶
When you are reading and sending cookies through the request and response components like shown in the last two subsections, you enjoy the added security of cookie validation which protects cookies from being modified on the client side. This is achieved by signing each cookie with a hash string, which allows the application to tell if a cookie is modified on the client side or not. If so, the cookie will NOT be accessible through the cookie collection of the request component.

How is changing the expiry date of a cookie not a modification!?
I understand design decisions need to be made as your team sees fit, but at least document them correctly. These are security issues waiting to happen.

@qiangxue
Copy link
Member

Cookie validation only protects the cookie value from being tampered.
For cookie expiration time, it doesn't make sense to be protected because regardless what cookie expiration time is, you can always send a cookie in the request. And in the request you can't get cookie expiration time.

@SamMousa
Copy link
Contributor Author

Regardless of what you choose to do, your reasoning is wrong:

  1. If you don't include the expiration date in validation than you shuold mention that in the DOCs.
  2. If you want to you can protect it by simply adding it in the data as well and verifying it on the server.

You can never "send" a cookie in a request if you did not receive that cookie from the application; it wouldn't pass validation.
Basically the docs say "unvalidated or altered cookies won't make it into the request object" which is a false statement.

@qiangxue
Copy link
Member

Modified the doc to make it clearer.

@raxmatov
Copy link

raxmatov commented Aug 5, 2016

check your authKey afterLogin if it is null or empty try regenerate it and save

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

No branches or pull requests

5 participants