security fixes

Author: kinjalh

lib/console/chat/teams/auth.ex

@@ -5,88 +5,93 @@ defmodule Console.Chat.Teams.Auth do
   Follows the Bot Connector authentication spec:
   https://learn.microsoft.com/en-us/azure/bot-service/rest-api/bot-framework-rest-connector-authentication
 
-  It fetches (and caches) the connector OpenID metadata + JWKS, then validates the issuer, audience (the bot's
-  Microsoft App ID), lifetime and RS256 signature.  When a `:service_url` opt is supplied it is also checked
-  against the `serviceurl` claim, since a valid token pins the service url the reply may be sent to.
+  Teams authenticates channel -> bot webhooks with a signed OIDC JWT in the `Authorization` header - there is no
+  hmac/shared-secret option like the scm/observability webhooks use - so we validate the token against the
+  connector's published JWKS.  We reuse `oidcc` (as the OIDC login flow does in `Console.Deployments.Settings`)
+  to discover the metadata, load + cache the JWKS, and validate the signature/issuer/audience/lifetime.  The
+  Teams-specific `serviceurl` claim is checked separately.
   """
-  alias Console.Cache
+  use Nebulex.Caching
   require Logger
 
-  @issuer "https://api.botframework.com"
-  @metadata_url "https://login.botframework.com/v1/.well-known/openidconfiguration"
-  @clock_skew 300
-  @ttl :timer.hours(24)
+  # The connector publishes its metadata here.  Two quirks are needed to consume it via oidcc:
+  #   * the document's declared issuer is `https://api.botframework.com` (not this url) -> allow_issuer_mismatch
+  #   * the document omits several oidc-required fields (scopes/response_types/subject_types), so we backfill
+  #     them via document_overrides purely to satisfy the parser - they do not affect token validation.
+  @config_issuer "https://login.botframework.com/v1"
+  @cache_adapter Console.conf(:cache_adapter)
+  @ttl :timer.hours(1)
+
+  @quirks %{
+    quirks: %{
+      allow_issuer_mismatch: true,
+      document_overrides: %{
+        "scopes_supported" => ["openid"],
+        "response_types_supported" => ["id_token"],
+        "subject_types_supported" => ["public"]
+      }
+    }
+  }
 
   @type claims :: map()
 
+  @doc """
+  Verifies a Bot Framework JWT.  `audience` must be the bot's Microsoft App ID (the connection's client id).
+  Pass `service_url:` to additionally pin the token's `serviceurl` claim to the inbound activity's serviceUrl.
+  """
   @spec verify(binary, binary, keyword) :: {:ok, claims} | {:error, binary}
-  def verify(token, audience, opts \\ []) when is_binary(token) and is_binary(audience) do
-    with {:ok, header}  <- Joken.peek_header(token),
-         {:ok, jwk}     <- signing_key(header),
-         {:ok, claims}  <- verify_signature(jwk, header, token),
-         :ok            <- validate_claims(claims, audience, opts) do
+  def verify(token, audience, opts \\ [])
+  def verify(token, audience, opts) when is_binary(token) and is_binary(audience) do
+    with {:ok, _}             <- peek(token),
+         {:ok, {conf, jwks}}  <- provider_configuration(),
+         ctx                  = Oidcc.ClientContext.from_manual(conf, jwks, audience, "dummy_secret", %{client_jwks: JOSE.JWK.generate_key(16)}),
+         validate_opts        = %{signing_algs: ctx.provider_configuration.id_token_signing_alg_values_supported},
+         {:ok, claims}        <- validate_jwt(token, ctx, validate_opts),
+         :ok                  <- validate_service_url(claims, opts[:service_url]) do
       {:ok, claims}
     end
   end
   def verify(_, _, _), do: {:error, "missing teams bot token or audience"}
 
-  defp verify_signature(jwk, %{"alg" => alg}, token) when is_binary(alg) do
-    case JOSE.JWT.verify_strict(JOSE.JWK.from_map(jwk), [alg], token) do
-      {true, %JOSE.JWT{fields: claims}, _} -> {:ok, claims}
-      _ -> {:error, "invalid teams jwt signature"}
+  @doc """
+  Loads and caches the Bot Framework provider configuration + JWKS.  Cached so we don't round-trip Microsoft on
+  every webhook; the ttl also bounds how stale the signing keys can get across a key rotation.
+  """
+  @decorate cacheable(cache: @cache_adapter, key: :teams_bf_oidc_config, opts: [ttl: @ttl], match: &ok?/1)
+  def provider_configuration() do
+    with {:ok, {conf, _}} <- Oidcc.ProviderConfiguration.load_configuration(@config_issuer, @quirks),
+         {:ok, {jwks, _}} <- Oidcc.ProviderConfiguration.load_jwks(conf.jwks_uri) do
+      {:ok, {conf, jwks}}
+    else
+      err ->
+        Logger.warning("failed to load teams bot framework oidc configuration: #{inspect(err)}")
+        {:error, "could not load teams bot framework configuration"}
     end
   end
-  defp verify_signature(_, _, _), do: {:error, "teams jwt is missing a signing algorithm"}
 
-  defp validate_claims(claims, audience, opts) do
-    now = System.system_time(:second)
-    with :ok <- check(claims["iss"] == @issuer, "invalid teams jwt issuer"),
-         :ok <- check(claims["aud"] == audience, "invalid teams jwt audience"),
-         :ok <- check(is_integer(claims["exp"]) and claims["exp"] + @clock_skew > now, "teams jwt expired"),
-         :ok <- check(!is_integer(claims["nbf"]) or claims["nbf"] - @clock_skew <= now, "teams jwt not yet valid"),
-         :ok <- validate_service_url(claims, opts[:service_url]) do
-      :ok
+  # cheap, network-free rejection of obviously malformed tokens before we touch the provider config
+  defp peek(token) do
+    case Joken.peek_header(token) do
+      {:ok, _} = ok -> ok
+      _ -> {:error, "malformed teams jwt"}
     end
   end
 
-  defp validate_service_url(_claims, url) when not is_binary(url), do: :ok
-  defp validate_service_url(%{"serviceurl" => claim}, url) when is_binary(claim),
-    do: check(String.trim_trailing(claim, "/") == String.trim_trailing(url, "/"), "teams jwt serviceUrl mismatch")
-  defp validate_service_url(_claims, _url), do: :ok
-
-  defp signing_key(%{"kid" => kid}) do
-    case Enum.find(signing_keys(), & &1["kid"] == kid) do
-      %{} = key -> {:ok, key}
-      _ -> {:error, "no matching signing key for teams jwt"}
+  defp validate_jwt(token, ctx, opts) do
+    case Oidcc.Token.validate_jwt(token, ctx, opts) do
+      {:ok, claims} -> {:ok, claims}
+      {:error, err} -> {:error, "invalid teams jwt: #{inspect(err)}"}
     end
   end
-  defp signing_key(_), do: {:error, "teams jwt is missing a key id"}
 
-  defp signing_keys() do
-    case Cache.get(:teams_bf_jwks) do
-      [_ | _] = keys -> keys
-      _ -> refresh_keys()
-    end
-  end
-
-  defp refresh_keys() do
-    with {:ok, %{"jwks_uri" => uri}} <- fetch(@metadata_url),
-         {:ok, %{"keys" => keys}}    <- fetch(uri) do
-      Cache.put(:teams_bf_jwks, keys, ttl: @ttl)
-      keys
-    else
-      err ->
-        Logger.warning("failed to fetch teams bot framework jwks: #{inspect(err)}")
-        []
-    end
-  end
+  # when no service_url is supplied there's nothing to pin against; otherwise the claim must be present and match.
+  defp validate_service_url(_claims, url) when not is_binary(url), do: :ok
+  defp validate_service_url(%{"serviceurl" => claim}, url) when is_binary(claim),
+    do: check(String.trim_trailing(claim, "/") == String.trim_trailing(url, "/"), "teams jwt serviceUrl mismatch")
+  defp validate_service_url(_claims, _url), do: {:error, "teams jwt is missing the serviceUrl claim"}
 
-  defp fetch(url) do
-    case Req.get(url) do
-      {:ok, %Req.Response{status: 200, body: body}} when is_map(body) -> {:ok, body}
-      err -> {:error, err}
-    end
-  end
+  defp ok?({:ok, _}), do: true
+  defp ok?(_), do: false
 
   defp check(true, _), do: :ok
   defp check(_, msg), do: {:error, msg}

test/console/chat/impl/teams_test.exs

@@ -61,6 +61,7 @@ defmodule Console.Chat.Impl.TeamsTest do
       no_mention = activity(%{"entities" => [%{"type" => "mention", "mentioned" => %{"id" => "28:someone-else"}}]})
 
       assert :ok = Teams.handle_activity(conn, no_mention)
+
       assert [] = Repo.all(ChatbotMessage)
     end