ueberauth / guardian

Elixir Authentication
MIT License
3.4k stars 381 forks source link

if a "kid" is included in the jwk, pass it to the jws as a hint #654

Closed mpinkston closed 4 years ago

mpinkston commented 4 years ago

after implementing my own secret fetcher, I realized that the key id passed back in the JWK was not getting added to the signature. As the secret frequently rotates, and the new key id is generated in the secret fetcher, there isn't a convenient place to pass the key id along in the :headers option.

This PR is to detect if a "kid" has been provided in the JWK and pass it along to the signature for easier lookups when there might be a list of more than one possible public key against which to verify.

Please let me know if this looks like a reasonable approach.

mpinkston commented 4 years ago

sure, I'll add an example implementation in the morning (working JST)

mpinkston commented 4 years ago

@Hanspagh I wasn't sure how you wanted this organized in the docs, but this might provide something of a start.

Key Rotation

Guardian provides a Guardian.Token.Jwt.SecretFetcher behaviour that allows custom keys to be used for signing and verifying requests. This makes it possible to rotate private keys while maintaining a list of valid public keys that can be used both for validating signatures as well as serving public keys to external services.

Below is a simple example of how this can be implemented using a GenServer.

defmodule MyApp.Guardian.KeyServer do
  @moduledoc ~S"""
  A simple GenServer implementation of a custom `Guardian.Token.Jwt.SecretFetcher`
  This is appropriate for development but should not be used in production
  due to questionable private key storage, lack of multi-node support,
  node restart durability, and public key garbage collection.
  """

  use GenServer

  @behaviour Guardian.Token.Jwt.SecretFetcher

  @impl Guardian.Token.Jwt.SecretFetcher
  # This will always return a valid key as a new one will be generated
  # if it does not already exist.
  def fetch_signing_secret(_mod, _opts),
    do: {:ok, GenServer.call(__MODULE__, :fetch_private_key)}

  @impl Guardian.Token.Jwt.SecretFetcher
  # This assumes that the adapter properly assigned a key id (kid)
  # to the signing key. Make sure it's there! with something like
  # JOSE.JWK.merge(jwk, %{"kid" => JOSE.JWK.thumbprint(jwk)})
  # see https://tools.ietf.org/html/rfc7515#section-4.1.4
  # for details
  def fetch_verifying_secret(_mod, %{"kid" => kid}, _opts) do
    case GenServer.call(__MODULE__, {:fetch_public_key, kid}) do
      {:ok, public_key} -> {:ok, public_key}
      :error -> {:error, :secret_not_found}
    end
  end

  def fetch_verifying_secret(_, _, _), do: {:error, :secret_not_found}

  # This is not a defined callback for the SecretFetcher, but could be useful
  # for providing an endpoint that external services could use to verify tokens
  # for themselves.
  def fetch_verifying_secrets,
    do: GenServer.call(__MODULE__, :fetch_public_keys)

  # Expire the private key so that a new one will be generated on the next
  # signing request. The public key associated with the old private key should
  # be stored at the very least as long as the largest possible "exp"
  # (https://tools.ietf.org/html/rfc7519#section-4.1.4) value for any token
  # signed by the old private key before this method was called.
  def expire_private_key,
    do: GenServer.cast(__MODULE__, :expire_private_key)

  # Generate a new keypair along with the key ID (kid)
  @spec generate_keypair() :: {:ok, JOSE.JWK.t(), JOSE.JWK.t(), String.t()}
  def generate_keypair() do
    # Choose an appropriate signing algorithm for your security needs.
    private_key = JOSE.JWK.generate_key({:okp, :Ed25519})

    # Generate a kid by using the key's thumbprint
    # https://tools.ietf.org/html/draft-ietf-jose-jwk-thumbprint-08#section-1
    kid = JOSE.JWK.thumbprint(private_key)

    # Update the private key to contain the "kid"
    private_key = JOSE.JWK.merge(private_key, %{"kid" => kid})

    # Create a public key based on the private key. It will carry the same "kid"
    public_key = JOSE.JWK.to_public(private_key)

    {:ok, private_key, public_key, kid}
  end

  def start_link(_opts) do
    GenServer.start_link(__MODULE__, :ok, name: __MODULE__)
  end

  def init(_opts) do
    {:ok, %{private_key: nil, public_keys: %{}}}
  end

  # Callbacks

  def handle_cast(:expire_private_key, state),
    do: {:noreply, %{state | private_key: nil}}

  # Generate a new signing key if one does not already exist
  def handle_call(:fetch_private_key, _from, %{private_key: nil, public_keys: key_list}) do
    {:ok, private_key, public_key, kid} = generate_keypair()

    {:reply, private_key,
     %{
       private_key: private_key,
       public_keys: Map.put(key_list, kid, public_key)
     }}
  end

  def handle_call(:fetch_private_key, _from, %{private_key: private_key} = state),
    do: {:reply, private_key, state}

  def handle_call({:fetch_public_key, kid}, _from, %{public_keys: public_keys} = state),
    do: {:reply, Map.fetch(public_keys, kid), state}

  def handle_call(:fetch_public_keys, _from, %{public_keys: public_keys} = state),
    do: {:reply, Map.values(public_keys), state}
end

Update Guardian's configuration to use the custom KeyServer.

## config/config.exs 

config :my_app, MyApp.Guardian,
  issuer: "myapp",
  allowed_algos: ["Ed25519"],
  secret_fetcher: MyApp.Guardian.KeyServer

Start the KeyServer in the supervision tree so it can serve requests.

## lib/my_app/application.ex

def start(_type, _args) do
  # List all child processes to be supervised
  children =
  [
    MyAppWeb.Endpoint,
    MyApp.Guardian.KeyServer
  ]

  # See https://hexdocs.pm/elixir/Supervisor.html
  # for other strategies and supported options
  opts = [strategy: :one_for_one, name: MyApp.Supervisor]
  Supervisor.start_link(children, opts)
end
Hanspagh commented 4 years ago

Really nice :) What I have done before is to make an example repo, and then add a snippet to the readme, to not make everything too bloated. What we could also do is add it to the guides section. Do you have a preference?

mpinkston commented 4 years ago

That's a very good question. You're right that it shouldn't get too bloated. I think good documentation describes a feature without prescribing an implementation. In this case, it's a pretty simple feature, but it can open a rabbit-hole on how exactly it should be used. How about some middle-ground? I've updated the PR with a snippet, but rather than a whole repo, I've added a link to a gist (which, of course, can be moved to a more appropriate link if needed).

Hanspagh commented 4 years ago

Perfect 👌. I will add the key rotation to the advanced section secrets of the readme

Hanspagh commented 4 years ago

Thanks 👍