-
Notifications
You must be signed in to change notification settings - Fork 653
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,170 @@ | ||
use rand::{distributions::Uniform, rngs::OsRng, Rng}; | ||
use sha2::{Digest, Sha256}; | ||
|
||
const TOKEN_LENGTH: usize = 32; | ||
|
||
pub(crate) struct SecureToken { | ||
sha256: Vec<u8>, | ||
} | ||
|
||
impl SecureToken { | ||
pub(crate) fn generate(kind: SecureTokenKind) -> NewSecureToken { | ||
let plaintext = format!( | ||
"{}{}", | ||
kind.prefix(), | ||
generate_secure_alphanumeric_string(TOKEN_LENGTH) | ||
); | ||
let sha256 = Sha256::digest(plaintext.as_bytes()).as_slice().to_vec(); | ||
|
||
NewSecureToken { | ||
plaintext, | ||
inner: Self { sha256 }, | ||
} | ||
} | ||
|
||
pub(crate) fn parse(kind: SecureTokenKind, plaintext: &str) -> Option<Self> { | ||
// This will both reject tokens without a prefix and tokens of the wrong kind. | ||
if SecureTokenKind::from_token(plaintext) != Some(kind) { | ||
return None; | ||
} | ||
|
||
let sha256 = Sha256::digest(plaintext.as_bytes()).as_slice().to_vec(); | ||
Some(Self { sha256 }) | ||
} | ||
|
||
pub(crate) fn sha256(&self) -> &[u8] { | ||
&self.sha256 | ||
} | ||
} | ||
|
||
pub(crate) struct NewSecureToken { | ||
plaintext: String, | ||
inner: SecureToken, | ||
} | ||
|
||
impl NewSecureToken { | ||
pub(crate) fn plaintext(&self) -> &str { | ||
&self.plaintext | ||
} | ||
} | ||
|
||
impl std::ops::Deref for NewSecureToken { | ||
type Target = SecureToken; | ||
|
||
fn deref(&self) -> &Self::Target { | ||
&self.inner | ||
} | ||
} | ||
|
||
fn generate_secure_alphanumeric_string(len: usize) -> String { | ||
const CHARS: &[u8] = b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; | ||
|
||
OsRng | ||
.sample_iter(Uniform::from(0..CHARS.len())) | ||
.map(|idx| CHARS[idx] as char) | ||
.take(len) | ||
.collect() | ||
} | ||
|
||
macro_rules! secure_token_kind { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 :) |
||
($(#[$attr:meta])* $vis:vis enum $name:ident { $($key:ident => $repr:expr,)* }) => { | ||
$(#[$attr])* | ||
$vis enum $name { | ||
$($key,)* | ||
} | ||
|
||
impl $name { | ||
const VARIANTS: &'static [Self] = &[$(Self::$key,)*]; | ||
|
||
fn prefix(&self) -> &'static str { | ||
match self { | ||
$(Self::$key => $repr,)* | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
secure_token_kind! { | ||
/// Represents every kind of secure token generated by crates.io. When you need to generate a | ||
/// new kind of token you should also add its own kind with its own unique prefix. | ||
/// | ||
/// NEVER CHANGE THE PREFIX OF EXISTING TOKEN TYPES!!! Doing so will implicitly revoke all the | ||
/// tokens of that kind, distrupting production users. | ||
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] | ||
pub(crate) enum SecureTokenKind { | ||
API => "cio", // Crates.IO | ||
} | ||
} | ||
|
||
impl SecureTokenKind { | ||
fn from_token(token: &str) -> Option<Self> { | ||
Self::VARIANTS | ||
.iter() | ||
.find(|v| token.starts_with(v.prefix())) | ||
.copied() | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use std::collections::HashSet; | ||
|
||
#[test] | ||
fn test_generated_and_parse() { | ||
const KIND: SecureTokenKind = SecureTokenKind::API; | ||
pietroalbini marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let token = SecureToken::generate(KIND); | ||
assert!(token.plaintext().starts_with(KIND.prefix())); | ||
assert_eq!( | ||
token.sha256(), | ||
pietroalbini marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Sha256::digest(token.plaintext().as_bytes()).as_slice() | ||
); | ||
|
||
let parsed = | ||
SecureToken::parse(KIND, &token.plaintext()).expect("failed to parse back the token"); | ||
assert_eq!(parsed.sha256(), token.sha256()); | ||
} | ||
|
||
#[test] | ||
fn test_parse_no_kind() { | ||
assert!(SecureToken::parse(SecureTokenKind::API, "nokind").is_none()); | ||
} | ||
|
||
#[test] | ||
fn test_persistent_prefixes() { | ||
// Changing prefixes will implicitly revoke all the tokens of that kind, disrupting users. | ||
// This test serves as a reminder for future maintainers not to change the prefixes, and | ||
// to ensure all the variants are tested by this test. | ||
let mut remaining: HashSet<_> = SecureTokenKind::VARIANTS.iter().copied().collect(); | ||
let mut ensure = |kind: SecureTokenKind, prefix| { | ||
assert_eq!(kind.prefix(), prefix); | ||
remaining.remove(&kind); | ||
}; | ||
|
||
ensure(SecureTokenKind::API, "cio"); | ||
|
||
assert!( | ||
remaining.is_empty(), | ||
"not all variants have a test to check the prefix" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_conflicting_prefixes() { | ||
// This sanity check prevents multiple tokens from starting with the same prefix, which | ||
// would mess up the token kind detection. If this test fails after adding another variant | ||
// do not change the test, choose another prefix instead. | ||
for variant in SecureTokenKind::VARIANTS { | ||
for other in SecureTokenKind::VARIANTS { | ||
if variant == other { | ||
continue; | ||
} | ||
if variant.prefix().starts_with(other.prefix()) { | ||
panic!("variants {:?} and {:?} share a prefix", variant, other); | ||
} | ||
} | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.