redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.33k stars 993 forks source link

[RFC]: `jwtVerifier` (used by Netlify) does not perform the most important verification #6675

Open joconor opened 2 years ago

joconor commented 2 years ago

Summary

The jwtVerifier webhook verifier, used by Netlify webhooks, should validate the sha256 claim in the JWS token.

Although this webhook verifier is labeled jwtVerifier, its behavior is very Netlify specific. For instance, it can only use the HS256 algorithm for validating the token's signature. This is fine, but it highlights that this is very much a Netlify specific webhook verifier.

But as a Netlify webhook verifier, it appears to skip the key verification for Netlify webhooks: Verifying the sha256 claim.

As documented at https://docs.netlify.com/site-deploys/notifications/#payload-signature, the JWS token contains both iss and sha256 claims. The existing RWJS verifier does (optionally) validate the iss claim.

The example Ruby code given in the Netlify docs show the sha256 claim being verified against a computed SHA256 Digest of the request body. RedwoodJS should be performing this same check.

Recommendation: The jwtVerifier should be renamed to netlifyVerifier and the verification of the sha256 claim should be added. A more capable/generic JWT verifier, possibly based on the Auth0 decoder, can be investigated separately. If renaming the verifier is too disruptive, minimally the sha256 claim needs to be verified to be an effective Netlify webhook verification.

Motivation

Primary motivation is to have incoming Netlify webhooks verified using the Netlify documented technique.

Detailed proposal

Netlify example Ruby code:

require "digest"
require "jwt"
require "sinatra"

def signed(request, body)
  signature = request["X-Webhook-Signature"]
  return unless signature

  options = {iss: "netlify", verify_iss: true, algorithm: "HS256"}
  decoded = JWT.decode(signature, "your signature secret", true, options)

  ## decoded :
  ## [
  ##   { sha256: "..." }, # this is the data in the token
  ##   { alg: "..." } # this is the header in the token
  ## ]
  decoded.first[:sha256] == Digest::SHA256.hexdigest(body)
rescue JWT::DecodeError
  false
end

post "/netlify-hook" do
  body = request.body.read
  halt 403 unless signed(request, body)

  json = JSON.parse(body)
  # do something with the notification payload here
end

Note the verification of the sha256 claim against the computed SHA256 digest of the request body

Are you interested in working on this?

dac09 commented 2 years ago

@dthyresson assigning to you and bumping the priority on this - feels fairly important, but will let you do a proper analysis!

dthyresson commented 2 years ago

I'd argue this would be a hosting provider-specific variation on the JwtVerifier as Netlify adds in a check, issuer, and algorithm -- and those are not standard on every JWTVerifier.

Just as I did not create a "GitHhubVerifier" or a "StripeVerifier" or a "ClerkVerifier" each of those provider specific verifiers can be made from the underlying parts (JWT, Timestamp, and SHA256) because the provider can elect to change their web hook rules.

Therefore, we can either

  1. implement provider specific verifiers and be responsible for maintaining them -- or
  2. allow the developer to verify the claims and headers as per their needs -- or
  3. perhaps when plugins are available implement a suite of provider specific web hooks
  4. add documentation (which again needs to be maintained) to explain the Netlify JWT rules

In short, I do not think the JWT needs to be changed to become a Netlify verifier.

Instead, you use the JwtVerifier (which verifies the signature) and then also add in rules to verify the other parts.

Perhaps the best solution is to update the docs to say that the JwtVerifier when used with Netlify can/could/should also check the other claims.

dthyresson commented 2 years ago

In addition, the https://docs.netlify.com/site-deploys/notifications/#payload-signature docs here are only about Netlify outgoing deploy webhooks and state:

You can use any JWT client library to verify this signature in the service receiving the notification.

The sha256 is not part of the signature but rather a custom claim added by Netlify on the JWT:

decoded.first[:sha256] == Digest::SHA256.hexdigest(body)
rescue JWT::DecodeError
  false
end

They ask you to check the sha256 claim on the decoded signature. Again - this is a provider-specifc rule and not part of every Jwt verification.