Better validate OIDC and SAML configs (#1026)

* Bump postgres to release; Note on Caddy cert

* default auto_create_users

* Validate SAML and OIDC configs better

* Fix failing test

.gitignore

@@ -1,6 +1,8 @@
 # macOS cruft
 .DS_Store
 
+.devcontainer/pki/authorities/local/
+
 # The directory Mix will write compiled artifacts to.
 /_build/
 

CONTRIBUTING.md

@@ -9,6 +9,7 @@ started.
 * [Developer Environment Setup](#developer-environment-setup)
   * [Docker Setup](#docker-setup)
     * [Docker Caveat](#docker-caveat)
+  * [Local HTTPS](#local-https)
   * [asdf-vm](#asdf-vm)
   * [Pre-commit](#pre-commit)
   * [The .env File](#the-env-file)
@@ -81,6 +82,18 @@ reach their destination through the tunnel just fine. Because of this, it's
 recommended to use `172.28.0.0/16` for your `AllowedIPs` parameter when using
 host-based WireGuard clients with Firezone running under Docker Desktop.
 
+Routing packets from _another_ host on the local network, through your development
+machine, and out to the external Internet should work as well.
+
+### Local HTTPS
+
+We use Caddy as a development proxy. The `docker-compose.yml` is set up to link
+Caddy's local root cert into your `.devcontainer/pki/authorities/local/` directory.
+
+Simply add the `root.crt` file to your browser and/or OS certificate store in
+order to have working local HTTPS. This file is generated when Caddy launches for
+the first time and will be different for each developer.
+
 ### asdf-vm Setup
 
 While not strictly required, we use [asdf-vm](https://asdf-vm.com) to manage

apps/fz_http/lib/fz_http/conf/oidc_config.ex

@@ -5,6 +5,7 @@ defmodule FzHttp.Conf.OIDCConfig do
   use Ecto.Schema
 
   import Ecto.Changeset
+  import FzHttp.Validators.OpenIDConnect
 
   @primary_key false
   embedded_schema do
@@ -15,7 +16,7 @@ defmodule FzHttp.Conf.OIDCConfig do
     field :client_id, :string
     field :client_secret, :string
     field :discovery_document_uri, :string
-    field :auto_create_users, :boolean
+    field :auto_create_users, :boolean, default: true
   end
 
   def changeset(data) do
@@ -43,5 +44,6 @@ defmodule FzHttp.Conf.OIDCConfig do
       :discovery_document_uri,
       :auto_create_users
     ])
+    |> validate_discovery_document_uri()
   end
 end

apps/fz_http/lib/fz_http/conf/saml_config.ex

@@ -5,18 +5,20 @@ defmodule FzHttp.Conf.SAMLConfig do
   use Ecto.Schema
 
   import Ecto.Changeset
+  import FzHttp.Validators.SAML
 
   @primary_key false
   embedded_schema do
     field :id, :string
     field :label, :string
     field :metadata, :string
-    field :auto_create_users, :boolean
+    field :auto_create_users, :boolean, default: true
   end
 
   def changeset(data) do
     %__MODULE__{}
     |> cast(data, [:id, :label, :metadata, :auto_create_users])
     |> validate_required([:id, :label, :metadata, :auto_create_users])
+    |> validate_metadata()
   end
 end

apps/fz_http/lib/fz_http/devices/device.ex

@@ -7,7 +7,7 @@ defmodule FzHttp.Devices.Device do
   import Ecto.Changeset
   require Logger
 
-  import FzHttp.SharedValidators,
+  import FzHttp.Validators.Common,
     only: [
       trim: 2,
       validate_fqdn_or_ip: 2,

apps/fz_http/lib/fz_http/mfa/method.ex

@@ -5,7 +5,7 @@ defmodule FzHttp.MFA.Method do
 
   use Ecto.Schema
   import Ecto.Changeset
-  import FzHttp.SharedValidators, only: [trim: 2]
+  import FzHttp.Validators.Common, only: [trim: 2]
 
   @primary_key {:id, :binary_id, autogenerate: true}
   @whitespace_trimmed_fields :name

apps/fz_http/lib/fz_http/oidc/start_proxy.ex

@@ -23,17 +23,7 @@ defmodule FzHttp.OIDC.StartProxy do
 
     if parsed = auth_oidc_env && parse(auth_oidc_env) do
       Conf.Cache.put!(:parsed_openid_connect_providers, parsed)
-      # XXX: This is needed because this call can error out, bringing down
-      # the whole application if the OIDC config was entered incorrectly.
-      # Instead, swallow the Error and print to console.
-      #
-      # This should be fixed when refactoring OIDC.
-      try do
-        OpenIDConnect.Worker.start_link(parsed)
-      rescue
-        e in RuntimeError ->
-          Logger.error("ERROR starting OIDC worker: #{e}")
-      end
+      OpenIDConnect.Worker.start_link(parsed)
     else
       :ignore
     end

apps/fz_http/lib/fz_http/sites/site.ex

@@ -6,7 +6,7 @@ defmodule FzHttp.Sites.Site do
   use Ecto.Schema
   import Ecto.Changeset
 
-  import FzHttp.SharedValidators,
+  import FzHttp.Validators.Common,
     only: [
       trim: 2,
       validate_fqdn_or_ip: 2,

apps/fz_http/lib/fz_http/users/user.ex

@@ -9,7 +9,7 @@ defmodule FzHttp.Users.User do
   use Ecto.Schema
   import Ecto.Changeset
   import FzHttp.Users.PasswordHelpers
-  import FzHttp.SharedValidators, only: [trim: 2]
+  import FzHttp.Validators.Common, only: [trim: 2]
 
   alias FzHttp.{Devices.Device, OIDC.Connection}
 

apps/fz_http/lib/fz_http/validators/common.ex

@@ -1,4 +1,4 @@
-defmodule FzHttp.SharedValidators do
+defmodule FzHttp.Validators.Common do
   @moduledoc """
   Shared validators to use between schemas.
   """

apps/fz_http/lib/fz_http/validators/openid_connect.ex

@@ -0,0 +1,20 @@
+defmodule FzHttp.Validators.OpenIDConnect do
+  @moduledoc """
+  Validators various fields related to OpenID Connect
+  before they're saved and passed to the underlying
+  openid_connect library where they could become an issue.
+  """
+  import Ecto.Changeset
+
+  def validate_discovery_document_uri(changeset) do
+    changeset
+    |> validate_change(:discovery_document_uri, fn :discovery_document_uri, value ->
+      case OpenIDConnect.update_documents(discovery_document_uri: value) do
+        {:ok, _update_result} ->
+          []
+        {:error, :update_documents, reason} ->
+          [discovery_document_uri: "is invalid. Reason: #{inspect(reason)}"]
+      end
+    end)
+  end
+end

apps/fz_http/lib/fz_http/validators/saml.ex

@@ -0,0 +1,21 @@
+defmodule FzHttp.Validators.SAML do
+  @moduledoc """
+  Validators for SAML configs.
+  """
+
+  alias Samly.IdpData
+  import Ecto.Changeset
+
+  def validate_metadata(changeset) do
+    changeset
+    |> validate_change(:metadata, fn :metadata, value ->
+      try do
+        IdpData.from_xml(value, %IdpData{})
+        []
+      catch
+        :exit, e ->
+          [metadata: "is invalid. Details: #{inspect(e)}."]
+      end
+    end)
+  end
+end

apps/fz_http/mix.exs

@@ -64,8 +64,8 @@ defmodule FzHttp.MixProject do
       {:mox, "~> 1.0.1", only: :test},
       {:guardian, "~> 2.0"},
       {:guardian_db, "~> 2.0"},
-      {:openid_connect, "~> 0.2.2"},
-      {:samly, github: "dropbox/samly"},
+      {:openid_connect, github: "firezone/openid_connect"},
+      {:samly, github: "firezone/samly"},
       {:ueberauth, "~> 0.7"},
       {:ueberauth_identity, "~> 0.4"},
       {:httpoison, "~> 1.8"},

apps/fz_http/test/fz_http_web/live/setting_live/security_test.exs

@@ -3,6 +3,7 @@ defmodule FzHttpWeb.SettingLive.SecurityTest do
 
   alias FzHttp.Configurations, as: Conf
   alias FzHttpWeb.SettingLive.Security
+  import FzHttp.SAMLConfigFixtures
 
   describe "authenticated mount" do
     test "loads the active sessions table", %{admin_conn: conn} do
@@ -148,7 +149,7 @@ defmodule FzHttpWeb.SettingLive.SecurityTest do
     setup %{admin_conn: conn} do
       Conf.update_configuration(%{
         openid_connect_providers: %{},
-        saml_identity_providers: %{"test" => %{"metadata" => "<test></test>"}}
+        saml_identity_providers: %{"test" => saml_attrs()}
       })
 
       path = Routes.setting_security_path(conn, :show)
@@ -172,7 +173,7 @@ defmodule FzHttpWeb.SettingLive.SecurityTest do
         |> render_click()
 
       assert html =~ ~s|<p class="modal-card-title">SAML Config</p>|
-      assert html =~ ~s|&amp;amp;amp;lt;test&amp;amp;amp;gt;&amp;amp;amp;lt;/test&amp;amp;amp;gt;|
+      assert html =~ ~s|entityID=&quot;http://localhost:8080/realms/firezone|
     end
 
     test "validate", %{view: view} do
@@ -189,7 +190,7 @@ defmodule FzHttpWeb.SettingLive.SecurityTest do
       assert html =~ ~s|<p class="modal-card-title">SAML Config</p>|
 
       # not updated
-      assert Conf.get!(:saml_identity_providers) == %{"test" => %{"metadata" => "<test></test>"}}
+      assert Conf.get!(:saml_identity_providers) == %{"test" => saml_attrs()}
     end
 
     test "delete", %{view: view} do

apps/fz_http/test/support/fixtures/saml_config_fixtures.ex

@@ -0,0 +1,17 @@
+defmodule FzHttp.SAMLConfigFixtures do
+  @moduledoc """
+  Fixtures for SAML configs.
+  """
+
+  @xml """
+  <md:EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="http://localhost:8080/realms/firezone"><md:IDPSSODescriptor WantAuthnRequestsSigned="true" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol"><md:KeyDescriptor use="signing"><ds:KeyInfo><ds:KeyName>pdSMtx2s3RVVhxg_qJOjHhlZhwZk6JiBMiSm5PEgjkA</ds:KeyName><ds:X509Data><ds:X509Certificate>MIICnzCCAYcCBgGD18ZU8TANBgkqhkiG9w0BAQsFADATMREwDwYDVQQDDAhmaXJlem9uZTAeFw0yMjEwMTQxODMyMjJaFw0zMjEwMTQxODM0MDJaMBMxETAPBgNVBAMMCGZpcmV6b25lMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAur5Cb0jrDJbMwr96WWE+z9CjDg0A/uRkaB4loRqkmu3A2fQGsS6CP7F7lQWMJmpzvBgkNtB69toO2sgx1u1fhpIJBZ0uSHF5gnzQAivgVxInvkMKRTRSkpMbhObiDHZnEGI2+Ly+8iV8IvprdrbDgm52u4conam0H1PewUKkHulrVQ+ImFuEWAjKCRSqpUG2F1eRkA0YpqB09x0CZAOOoucwTsBYj/ZAz3dUXhYIENAF7v0ykvzGOCAyOZIn1uYQc7jvWpwoI8qQdL45phj2FLoFlght3tlZV8IG5hsXrE6rg7Ufqvv8xyGltrOMKj/jEFEunagZOUjkypDp36b8cwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBEZKLLr66GB3NxqXGTMl0PvTDNB9GdyShQHaJYjeeUQnEXixjlAVrOq/txEBKjhGUcqyFELoNuwcxxV1iHA5oXhCoqYmnp9T/ftmXPDT3c49PBABHgLJaFOKYTpVx1YjP7mA44X1ijLZmgboIeeFNerVNHIzR9BsxcloQlB0r9QfC14rsuXo6QD3QnaVI8wDgWXQHqpcwLFqvehXdNvMFniRvX2qBNU8E0FPoMaZ1C3n2nssLcVZ+C4ghq6YoAG+wLGY7XE8+v5rnYGDpGpfgr2wdefn6tryFq3PyGqA8ThjARESRRQG9kI/RlNX7qCnP/8/7JQ4wLdfz5C25uhakP</ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:ArtifactResolutionService Binding="urn:oasis:names:tc:SAML:2.0:bindings:SOAP" Location="http://localhost:8080/realms/firezone/protocol/saml/resolve" index="0"/><md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:persistent</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:transient</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:SOAP" Location="http://localhost:8080/realms/firezone/protocol/saml"/><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" Location="http://localhost:8080/realms/firezone/protocol/saml"/></md:IDPSSODescriptor></md:EntityDescriptor>
+  """
+  def saml_attrs do
+    %{
+      "metadata" => @xml,
+      "label" => "test",
+      "id" => "test",
+      "auto_create_users" => true
+    }
+  end
+end

docker-compose.prod.yml

@@ -51,7 +51,7 @@ services:
       <<: *default-deploy
 
   postgres:
-    image: postgres:15rc2
+    image: postgres:15
     volumes:
       - /data/firezone/postgres:/var/lib/postgresql/data
     environment:

docker-compose.yml

@@ -16,6 +16,7 @@ services:
     image: caddy:2
     volumes:
       - ./.devcontainer/Caddyfile:/etc/caddy/Caddyfile
+      - ./.devcontainer/pki:/data/caddy/pki
     ports:
       - 80:80
       - 443:443
@@ -62,7 +63,7 @@ services:
       - isolation
 
   postgres:
-    image: postgres:15rc2
+    image: postgres:15
     volumes:
       - postgres-data:/var/lib/postgresql/data
     environment:

docs/docs/administer/troubleshoot.mdx

@@ -7,6 +7,23 @@ For any problems that arise, a good first bet is to check the Firezone logs.
 Firezone logs are stored in `/var/log/firezone` and can be viewed with
 `sudo firezone-ctl tail`.
 
+## Application Crash Loop Preventing Config Changes
+
+In cases where the application is crash looping because of corrupt, inaccessible, or
+invalid data in the DB, you can try clearing the affected fields.
+
+For example, to clear OIDC configs:
+
+```text
+psql -d firezone -h 127.0.0.1 -U postgres -c "UPDATE configurations SET openid_connect_providers = '{}'"
+```
+
+Similarly, to clear SAML configs:
+
+```text
+psql -d firezone -h 127.0.0.1 -U postgres -c "UPDATE configurations SET saml_providers = '{}'"
+```
+
 ## Debugging Portal Websocket Connectivity Issues
 
 The portal UI requires a secure websocket connection to function. To facilitate

mix.lock

@@ -56,7 +56,7 @@
   "nimble_parsec": {:hex, :nimble_parsec, "1.2.3", "244836e6e3f1200c7f30cb56733fd808744eca61fd182f731eac4af635cc6d0b", [:mix], [], "hexpm", "c8d789e39b9131acf7b99291e93dae60ab48ef14a7ee9d58c6964f59efb570b0"},
   "nimble_totp": {:hex, :nimble_totp, "0.2.0", "010ad5a6627f62e070f753752680550ba9e5744d96fc4101683cd037f1f5ee18", [:mix], [], "hexpm", "7fecd15ff14637ccd2fb3bda68476a6a7f107af731c51b1714436b687e5b50b3"},
   "number": {:hex, :number, "1.0.3", "932c8a2d478a181c624138958ca88a78070332191b8061717270d939778c9857", [:mix], [{:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm", "dd397bbc096b2ca965a6a430126cc9cf7b9ef7421130def69bcf572232ca0f18"},
-  "openid_connect": {:hex, :openid_connect, "0.2.2", "c05055363330deab39ffd89e609db6b37752f255a93802006d83b45596189c0b", [:mix], [{:httpoison, "~> 1.2", [hex: :httpoison, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: false]}], "hexpm", "735769b6d592124b58edd0582554ce638524c0214cd783d8903d33357d74cc13"},
+  "openid_connect": {:git, "https://github.com/firezone/openid_connect.git", "e8dfd0033bb3420c1af43653406ccd870900032e", []},
   "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
   "phoenix": {:hex, :phoenix, "1.6.14", "57678366dc1d5bad49832a0fc7f12c2830c10d3eacfad681bfe9602cd4445f04", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 1.0", [hex: :phoenix_view, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.2", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d48c0da00b3d4cd1aad6055387917491af9f6e1f1e96cedf6c6b7998df9dba26"},
   "phoenix_ecto": {:hex, :phoenix_ecto, "4.4.0", "0672ed4e4808b3fbed494dded89958e22fb882de47a97634c0b13e7b0b5f7720", [:mix], [{:ecto, "~> 3.3", [hex: :ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.14.2 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "09864e558ed31ee00bd48fcc1d4fc58ae9678c9e81649075431e69dbabb43cc1"},
@@ -76,7 +76,7 @@
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
   "remote_ip": {:hex, :remote_ip, "1.0.0", "3d7fb45204a5704443f480cee9515e464997f52c35e0a60b6ece1f81484067ae", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "9e9fcad4e50c43b5234bb6a9629ed6ab223f3ed07147bd35470e4ee5c8caf907"},
   "rustler_precompiled": {:hex, :rustler_precompiled, "0.5.2", "7619fff0309a012eac7441993da4f6e257022bd456449a366756696a9a18fb19", [:mix], [{:castore, "~> 0.1", [hex: :castore, repo: "hexpm", optional: false]}, {:rustler, "~> 0.23", [hex: :rustler, repo: "hexpm", optional: true]}], "hexpm", "4e3716fd7cf6fbb806a9ed2b1449c987cfe578b24e3deb3ca4b8645638cc644c"},
-  "samly": {:git, "https://github.com/dropbox/samly.git", "4603438ed4a95ed74d6c0232676c24d097e2feec", []},
+  "samly": {:git, "https://github.com/firezone/samly.git", "4603438ed4a95ed74d6c0232676c24d097e2feec", []},
   "sleeplocks": {:hex, :sleeplocks, "1.1.1", "3d462a0639a6ef36cc75d6038b7393ae537ab394641beb59830a1b8271faeed3", [:rebar3], [], "hexpm", "84ee37aeff4d0d92b290fff986d6a95ac5eedf9b383fadfd1d88e9b84a1c02e1"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
   "sweet_xml": {:hex, :sweet_xml, "0.7.3", "debb256781c75ff6a8c5cbf7981146312b66f044a2898f453709a53e5031b45b", [:mix], [], "hexpm", "e110c867a1b3fe74bfc7dd9893aa851f0eed5518d0d7cad76d7baafd30e4f5ba"},