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
refactor(turborepo): Signature Authentication (2nd try) #4980
refactor(turborepo): Signature Authentication (2nd try) #4980
Conversation
…lgorithm (basically replace json serialization with a simple concatenation)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
|
@NicholasLYang is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some minor comments. I think we should announce this in the turbo channel / turboverse before merging.
} | ||
} | ||
|
||
fn secret_key(&self) -> Result<Vec<u8>, SignatureError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that HMAC_SHA256 doesn't have key length requirements but a comment that verifies that would be handy.
artifact_body: &[u8], | ||
expected_tag: &str, | ||
) -> Result<bool, SignatureError> { | ||
let secret_key = hmac::Key::new(HMAC_SHA256, &self.secret_key()?); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we could pull HMAC_SHA256 into a constant, since it's used in more than one place. Something like TURBO_HMAC_ALGO
Base64EncodingError(#[from] base64::DecodeError), | ||
} | ||
|
||
static TURBO_HMAC_ALGORITHM: Algorithm = HMAC_SHA256; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is static because apparently you can't have a const that refers to another const
Description
Refactors the signature authentication to use a language-independent algorithm, as the previous implementation depended on Go's JSON serialization.
NOTE: This is a global hash bump.
Testing Instructions
Testing is using fuzzing across the Go-Rust boundary to check that signatures are correctly generated on both sides.