Verifying requests using HTTP signature verification (we need a better way)

Verifying requests in accordance with the IETF HTTP Signatures specification Draft specification.

Lots of places have signature verification for their webhooks (for example GitHub, Stripe, Dropbox, Samsung SmartThings, and many many more), so it seems like it would be something Phoenix (or Plug) should be concerned with being able to do so without overriding core modules and functionality.

The problem is we need to verify the payload (the request body), and that is read only once in Plug.Parsers and discarded.

This solution could work but it’s weird if I want to receive and verify webhooks from different services as well as other issues (like not being able to use controllers).

Is there no way to get a better solution for this, maybe as a hex package (I’d write one if I knew a good way to solve this) or in Phoenix or Plug core?

Examples

Example 1: Securing GitHub webhooks

Example 2: Dropbox Webhooks

Example 3: Stripe Webhook verification

Example 4: Samsung SmartThings (IoT Cloud) HTTP signature verification:

3 Likes

Possible Solutions?

I’m not sure what would be the best way to go about solving this, but maybe a way to hook Plugs into the Plug.Parsers module (for certain routes, or route pipelines, or …?) where we can use the body before it is parsed?

Or maybe we could have another plug before Plug.Parsers and Plug.Parsers could check %Conn{} for (and use) a temporary “already fetched body attribute” in the struct, if the body has been read already?

I don’t know if those ideas are possible or feasible, so I’m going to do some digging around, I guess.

Any other thoughts, ideas, possible solutions, concerns?

Does anybody agree that this is a problem that should be solved in core libraries, or at least without having to override core modules or functionality?

1 Like

I have not encountered systems requiring this kind of signature verification yet myself, which gives me the (subjective) opinion that it might nog be that general of a problem (right now). But there could definitely be a Plug that will do this.

Do you often integrate with other services, and use webhooks?

Although, it would be applicable to any use case where you’d want to verify a request body with a signature, the most common situation where you’d encounter this is in verifying 3rd-party webhook requests. (like the ones from GitHub, Stripe, etc that I mentioned earlier)

2 Likes

I have implemented this - and it was my first (and only!) “bad” experience in elixir world.

Something you assume is gonna be fast to do - but gets you into quite the rabbit hole - and you end up with code you are not really happy about.

dev happiness rock bottom - and worst case is if people implement these webhooks without verifying, because it’s quite steep to understand what’s going on.

My brain have suppressed the experience/specifics - so can’t talk of potential solutions - but if there is no easy solution(which I think is the case) a PR with documentation for plug and even (especially?) for Phoenix and a best/recommended way of doing it is probably the fastest way forward…
eg titled “Verifying webhooks”, and then it tells the story of why it’s troublesome, and gives a suggested implementation…

well I think a good pattern is /webhooks/provider - so you add multiple Plugs (which of course is a small performance hit) - and you can decode the json and pass it to the controller… How to read request body multiple times during request handling? - but pretty it ain’t.

so totally share your frustration…

4 Likes

Yeah, I’m looking through right now and Plug.Conn.read_body/2 is not used until inside each specific parser’s parse/5 function, so I think that makes a solution more difficult.

I wonder if we used Plug.Conn.read_body/2 in Plug.Parsers and passed the body into parser.parse/5 instead, if that would be okay. :sweat_smile:

Too much breaking change, probably :frowning:

How about a Plug that replaces the Conn.adapter with one that verifies the body when read? read_body can return an error tuple already, so it shouldn’t break Plug.Parsers.

1 Like

We had this problem, and after talking about it at ElixirDaze Four other people approached me with the same. I think it’s common enough that it should be handled by something that is mean to be as general purpose as plug.

@ryanwinchester we copied and pasted the plug parses into our codebase and added a line the put the raw body in conn.assigns[:raw_body]. Here is the appropriate slide in that ElixirDaze talk about it http://crowdhailer.me/2018-03-02/raxx-refined-web-development/slides.html#27

However we don’t use this solution any more. The punchline is we don’t use plug on any service that validates signatures. We have one service on the front end still using phoenix because it serves assets and html and has to sign requests but not validate.

2 Likes

There is also this issue raised by someone else i know is doing signing
https://github.com/elixir-plug/plug/issues/691

1 Like

Isn’t reading the body done with an adapter?

Maybe a toplevel plug could:

  1. Read the body and then put_private(conn, :raw_body, raw_body)
  2. Put a custom adapter that makes read_body use:raw_body instead.

This way when the parsers parse, they will use the adapter’s read_req_body instead of the one that can only be called once.

The custom adapter could otherwise use the original adapter’s implementation for anything else.

Where can I “plug” in my own Plug.Conn.adapter in the framework?

@ryanwinchester

This is only an idea and it’s pseudocode - I haven’t even compiled it yet:

defmodule FakeAdapter.Cowboy2.Conn do
  @moduledoc false

  @behaviour Plug.Conn.Adapter

  alias Plug.Adapters.Cowboy2

  defdelegate send_resp(payload, status, headers, body),
    to: Cowboy2.Conn

  defdelegate send_file(payload, status, headers, file, offset, length),
    to: Cowboy2.Conn

  defdelegate send_chunked(payload, status, headers),
    to: Cowboy2.Conn

  defdelegate chunk(payload, status),
    to: Cowboy2.Conn

  defdelegate push(payload, path, headers),
    to: Cowboy2.Conn

  def read_req_body(payload, _options) do
    case Map.fetch(payload, :raw_body) do
      {:ok, data} -> {:ok, data, payload}
      :error -> {:error, :no_data}
    end
  end
end
defmodule InterceptAdapter do
  import Plug.Conn

  def init(opts), do: opts

  def call(%Conn{adapter: {adapter, state}} = conn, _opts) do
    case read_all_body(conn) do
      {:ok, body, conn} ->
        state = Map.put(state, :raw_body, body)
        %{conn | adapter: {fake_adapter_for(adapter), state}}

      {:error, reason} ->
        halt(conn)
    end
  end

  def read_all_body(conn, acc \\ "") do
    case read_body(conn) do
      {:ok, body, conn} ->
        {:ok, acc <> body, conn}

      {:more, partial_body, conn} ->
        read_all_body(conn, acc <> partial_body)

      {:error, reason} ->
        {:error, reason}
    end
  end

  defp fake_adapter_for(Elixir.Plug.Adapters.Cowboy2),
    do: FakeAdapter.Cowboy2.Conn
end

So the idea is to plug InterceptAdapter before the plug parsers and theoretically you should get the ability to read more than once

It looks like @josevalim might be willing to allow us a better solution soon:

https://github.com/elixir-plug/plug/issues/691#issuecomment-382842180

2 Likes

I opened a PR for discussion

https://github.com/elixir-plug/plug/pull/698

3 Likes

IT WAS MERGED!

4 Likes

:clap: :clap: :clap: :clap: :clap::green_heart::hearts::purple_heart::orange_heart::blue_heart::tada::tada::tada::tada::tada:

thank you @ryanwinchester and @josevalim

1 Like

I don’t know if this helps, but about a year ago I Implemented something similar to validate an Alexa app backend using Elixir. You can see the Github here. Still needs work (and I am not at all an Elixir expert) but it got the job done. Feel free to grab whatever you want from it.

https://github.com/grahac/alexa_request_verifier

It was a pain to do but I think ryanwinchester’s change should make it easier.