Signature validation utility
Monadic-Cat opened this issue · 8 comments
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:
- You can provide a function which consumes the two headers used and the request body, like
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
. - You can provide the same, but by consuming a web server library's request type directly, like
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 bytwilight-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 totwilight-util
.
At the very least, I would suggest support for Axum and Actix Web. - As a compromise between 1 and 2, we could define a wrapper request type which can be made by passing in
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.
- Providing a function which would make it easy to check the signature would be something im not opposed to.
- I would be against providing any further utilities like our own implementations for webservers.
There are many different crates out there and eventually we will have a bunch of middlewares we need to maintain.
Further more it shouldn't be that hard to get the headers of a request. To make it a little bit easier, we could additionally export the header names as constants. - I don't think we should overcomplicate that.
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 check_signature
signature needs an additional parameter for the signature.
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.
Actually, I just realized that we could accept an iterator of (&str, &[u8])
, like how http-ratelimiting does it.
Actually, I just realized that we could accept an iterator of (&str, &[u8]), like how http-ratelimiting does it.
I'm unsure of whether this would be a reliable option.
Eg:
- actix-web provides an iterator of all headers but does not use strings but
HeaderValue
which does not implementToString
. - rocket provides an iterator but over
Header<'_>
where we need to call.value()
in order to get the data. - tide handles that different too.
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);
I'd personally prefer an API that returns a result, similar to twilight-validate
, so that we can simply propagate with check_signature(timestamp, body, signature, key)?;
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);I'd personally prefer an API that returns a result, similar to
twilight-validate
, so that we can simply propagate withcheck_signature(timestamp, body, signature, key)?;
Should it return the Interaction
in the Ok
variant, or just ()
so the check can be done with ?
and users handle doing the deserialization their selves?
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 extract_discord
, but it could be perfectly reasonable for someone to want to do the deserialization on their own, using their own model or their own parser other than serde_json
.)
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 check_signature
and extract_interaction
, which does the checking itself.
Accordingly with the comment posted here: #2205 (comment),
I am closing this issue as "not wanted".