Skip to content

Add SecureToken to enforce good token hygiene #3320

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

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

pietroalbini
Copy link
Member

This PR adds the SecureToken struct, which relies on the type system to enforce good practices for generating and verifying tokens:

  • SecureToken forces adding an unique prefix in front of every kind of generated token. This allows token scanning services to properly identify tokens generated by crates.io, and allows crates.io to detect which kind of token it's about to revoke.
  • SecureToken only stores and exposes the SHA256 of already generated tokens, to prevent accidental plaintext leakages. The plaintext token value is only exposes for newly generated tokens.

r? @Turbo87

.collect()
}

macro_rules! secure_token_kind {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this a bit hard to read. Did you consider using something like strum instead of this custom macro? If we'll still go with the custom macro I think it'd be good to add a few more doc comments on it at least :)

Copy link
Member Author

@pietroalbini pietroalbini Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's worth bringing another dependency in just for this. I'll try to add some comments to the macro.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is also fine to retrofit. I'm approving the PR in the meantime :)

@Turbo87 Turbo87 added A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear labels Feb 22, 2021
This commit adds the `SecureToken` struct, which relies on the type
system to enforce good practices for generating and verifying tokens:

* `SecureToken` forces adding an unique prefix in front of every kind of
  generated token. This allows token scanning services to properly
  identify tokens generated by crates.io, and allows crates.io to detect
  which kind of token it's about to revoke.

* `SecureToken` only stores and exposes the SHA256 of already generated
  tokens, to prevent accidental plaintext leakages. The plaintext token
  value is only exposes for newly generated tokens.
@pietroalbini pietroalbini force-pushed the refactor-token-generation branch from 7211160 to 7ce54f4 Compare February 23, 2021 12:42
@pietroalbini pietroalbini force-pushed the refactor-token-generation branch from 7ce54f4 to 46840ca Compare February 23, 2021 13:57
@Turbo87
Copy link
Member

Turbo87 commented Feb 25, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 25, 2021

📌 Commit 46840ca has been approved by Turbo87

bors added a commit that referenced this pull request Feb 25, 2021
Add SecureToken to enforce good token hygiene

This PR adds the `SecureToken` struct, which relies on the type system to enforce good practices for generating and verifying tokens:

* `SecureToken` forces adding an unique prefix in front of every kind of generated token. This allows token scanning services to properly identify tokens generated by crates.io, and allows crates.io to detect which kind of token it's about to revoke.
* `SecureToken` only stores and exposes the SHA256 of already generated tokens, to prevent accidental plaintext leakages. The plaintext token value is only exposes for newly generated tokens.

r? `@Turbo87`
@bors
Copy link
Contributor

bors commented Feb 25, 2021

⌛ Testing commit 46840ca with merge febf9f8...

@bors
Copy link
Contributor

bors commented Feb 25, 2021

💔 Test failed - checks-actions

@Turbo87
Copy link
Member

Turbo87 commented Feb 25, 2021

@bors retry

@bors
Copy link
Contributor

bors commented Feb 25, 2021

⌛ Testing commit 46840ca with merge eb093ee...

@bors
Copy link
Contributor

bors commented Feb 25, 2021

☀️ Test successful - checks-actions
Approved by: Turbo87
Pushing eb093ee to master...

@bors bors merged commit eb093ee into rust-lang:master Feb 25, 2021
@pietroalbini pietroalbini deleted the refactor-token-generation branch February 25, 2021 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants