Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Cookie expiration time is set too high (90 days) #4198

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

Closed
Shnatsel opened this issue Nov 21, 2021 · 2 comments
Closed

Cookie expiration time is set too high (90 days) #4198

Shnatsel opened this issue Nov 21, 2021 · 2 comments

Comments

@Shnatsel
Copy link
Member

Shnatsel commented Nov 21, 2021

Describe the bug

Right now the crates.io session cookie is set to be cleared by the browser in 90 days, unless refreshed.

Such a long cookie lifetime makes sense for certain other websites (e.g. Google, Github) that assume constant interaction from authenticated users. However, crates.io uses the cookie only for highly sensitive administrative tasks such as generating access tokens for Cargo or adding new publishers.

On crates.io the most common flow is "log in, perform a sensitive action, never use anything requiring authentication for weeks". There is no frequent action requiring authentication, such as commenting on issues. As such, there is no reason to keep the cookie around for that long.

To Reproduce
Steps to reproduce the behavior:

  1. Go to crates.io in the browser
  2. Log in with Github
  3. Open Developer Console in the browser and inspect the cookies
  4. Observe the cookie expiring 90 days in the future

Expected behavior
The expiration date set to 1 day or even 1 hour, to reflect the sensitivity of the cookie as well as how rarely it it is actually used.

Desktop (please complete the following information):

  • OS: Any (tested on Linux)
  • Browser: Firefox
  • Version: 94

Additional context

Cookie theft is a common target for malware. In fact, the malware distributed as part of the recent NPM compromise was stealing cookies. It is important not to allow a single compromised package to steal credentials for other crates.io accounts.

See also: #2630

@Shnatsel Shnatsel added the C-bug 🐞 Category: unintended, undesired behavior label Nov 21, 2021
@bjorn3
Copy link
Member

bjorn3 commented Nov 21, 2021

The cookie is also used to eg show you the list of crates you are watching (and the latest updates for these crates), which crates you published. Maybe the cookie could allow read/write access the first so much time and only allow read-only access (and maybe adding new crates to follow) after that until it expires? Kind of like github's "sudo mode" for security sensitive operations.

@Shnatsel
Copy link
Member Author

Ah, I did not realize that because I have never used those features. Yes, having a separate cookie for write operations sound good. The only downside is that it imposes greater implementation complexity, but I suppose that can't be helped.

@Turbo87 Turbo87 removed the C-bug 🐞 Category: unintended, undesired behavior label Nov 21, 2021
@rust-lang rust-lang locked and limited conversation to collaborators Nov 21, 2021
@Turbo87 Turbo87 closed this as completed Nov 21, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants