-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
Signature validation utility #2169
Comments
let is_valid = check_signature(timestamp, body, signature, key);
if !is_valid {
return Err("Body signature does not match.")
}
let interaction: Interaction = serde_json::from_str(body); Sth. like this should be straight forward enough. |
I agree that extracting the headers is outside the scope of Twilight. There are multiple header implementations, for example, https://docs.rs/worker/latest/worker/struct.Headers.html and https://docs.rs/http/latest/http/header/struct.HeaderMap.html, and publically depending upon any of them mean tying ourselves to their major versions. BTW, the example |
Actually, I just realized that we could accept an iterator of |
I'm unsure of whether this would be a reliable option.
|
I'd personally prefer an API that returns a result, similar to |
Should it return the let interaction = check_signature(timestamp, body, signature, key)?; vs check_signature(timestamp, body, signature, key)?;
let interaction: Interaction = serde_json::from_str(body); (The former is why I named my initial suggestion |
We had a similar discussion about whether we want to inline validation, I think the best solution is to provide both a separate method and a method that chains validation and actual work. So in this case, both |
Accordingly with the comment posted here: #2205 (comment), |
What?
Signature verification utility for webhook style bots. I was advised to design an API for adding this to
twilight-util
.There are a few approaches to this that come to mind:
fn extract_discord(sig: &[u8], timestamp: &[u8], body: Vec<u8>) -> Result<Interaction, Unauthorized> { ... }
In this scenario, we should definitely provide constants representing the names of the headers,
x-signature-ed25519
andx-signature-timestamp
.fn extract_discord(req: Request) -> Result<Interaction, Bad Request | Unauthorized> { ... }
Which has the advantage of not requiring the user to manually extract headers before jumping into working with
the data they actually care about from Discord. It has the disadvantage of potentially requiring multiple shims,
one for each web server library supported by
twilight-util
.If you do go this way, it may make sense to go a bit further and provide middlewares for those web server libraries
which support adding the signature verification behavior transparently.
This approach has the downside of potentially needing to add many Cargo features to
twilight-util
.At the very least, I would suggest support for Axum and Actix Web.
closures which handle the tasks "get the header by this name" and "get the request body" together with an arbitrary type.
Or use a trait for the same thing, but since users usually aren't able to add impls to their web server dependencies, provide
a convenience macro for defining a wrapper type which can be trivially constructed from their request type and which implements the trait.
Use Case
All bots written in the (relatively) newly introduced style, where you make a web server and give a URL to POST to to Discord,
are required to verify a signature written in the POST request header against another header and the request body.
Other
This crate handles this task for bots which are hosted on Cloudflare Workers: https://github.com/cipherizer/twilight-cloudflare-workers
I wrote this crate which handles this task as a Tower layer, for use in things like Axum web servers: https://git.catmonad.xyz/Monadic-Cat/twilight-signature-validation
Who's Going To Write It
I am willing to implement this feature myself. This includes going through web server libraries and writing middlewares for each one, if necessary. This also, of course, includes taking feedback and changing it to match better with the priorities of the Twilight project.
The text was updated successfully, but these errors were encountered: