Skip to content

Add SSL validation support for certs_keys #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions lib/plug/ssl.ex
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ defmodule Plug.SSL do
|> check_for_missing_keys()
|> validate_ciphers()
|> normalize_ssl_files()
|> normalize_certs_keys_ssl_files()
|> convert_to_charlist()
|> set_secure_defaults()
|> configure_managed_tls()
Expand All @@ -179,24 +180,45 @@ defmodule Plug.SSL do
end

defp check_for_missing_keys(options) do
has_certs_keys? = List.keymember?(options, :certs_keys, 0)
has_sni? = List.keymember?(options, :sni_hosts, 0) or List.keymember?(options, :sni_fun, 0)
has_key? = List.keymember?(options, :key, 0) or List.keymember?(options, :keyfile, 0)
has_cert? = List.keymember?(options, :cert, 0) or List.keymember?(options, :certfile, 0)

cond do
has_sni? -> options
not has_key? -> fail("missing option :key/:keyfile")
not has_cert? -> fail("missing option :cert/:certfile")
not (has_key? or has_certs_keys?) -> fail("missing option :key/:keyfile/:certs_keys")
not (has_cert? or has_certs_keys?) -> fail("missing option :cert/:certfile/:certs_keys")
true -> options
end
end

defp normalize_ssl_files(options) do
ssl_files = [:keyfile, :certfile, :cacertfile, :dhfile]
Enum.reduce(ssl_files, options, &normalize_ssl_file(&1, &2))
Enum.reduce(ssl_files, options, &normalize_ssl_file(&1, &2, options[:otp_app]))
end

defp normalize_ssl_file(key, options) do
defp normalize_certs_keys_ssl_files(options) do
if certs_keys = options[:certs_keys] do
ssl_files = [:keyfile, :certfile]

updated_certs_keys =
Enum.map(certs_keys, fn cert_key ->
Enum.reduce(
ssl_files,
Map.to_list(cert_key),
&normalize_ssl_file(&1, &2, options[:otp_app])
)
|> Map.new()
end)

List.keystore(options, :certs_keys, 0, {:certs_keys, updated_certs_keys})
else
options
end
end

defp normalize_ssl_file(key, options, otp_app) do
value = options[key]

cond do
Expand All @@ -207,7 +229,7 @@ defmodule Plug.SSL do
put_ssl_file(options, key, value)

true ->
put_ssl_file(options, key, Path.expand(value, otp_app(options)))
put_ssl_file(options, key, Path.expand(value, resolve_otp_app(otp_app)))
end
end

Expand All @@ -225,9 +247,9 @@ defmodule Plug.SSL do
List.keystore(options, key, 0, {key, value})
end

defp otp_app(options) do
if app = options[:otp_app] do
Application.app_dir(app)
defp resolve_otp_app(otp_app) do
if otp_app do
Application.app_dir(otp_app)
else
fail("the :otp_app option is required when setting relative SSL certfiles")
end
Expand Down
72 changes: 72 additions & 0 deletions test/plug/ssl_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ defmodule Plug.SSLTest do
import Plug.Conn

describe "configure" do
# make sure some dummy files used for the keyfile and certfile
# tests are removed after each test.
setup do
on_exit(fn ->
File.rm("_build/test/lib/plug/abcdef")
File.rm("_build/test/lib/plug/ghijkl")
end)

[]
end

import Plug.SSL, only: [configure: 1]

test "sets secure_renegotiate and reuse_sessions to true depending on the version" do
Expand Down Expand Up @@ -97,6 +108,67 @@ defmodule Plug.SSLTest do
assert {:cert, "ghijkl"} in opts
end

test "fails to configure if keyfile and certfile aren't absolute paths and otp_app is missing" do
assert {:error, message} = configure([:inet6, keyfile: "abcdef", certfile: "ghijkl"])
assert message == "the :otp_app option is required when setting relative SSL certfiles"
end

test "fails to configure if the keyfile doesn't exist" do
assert {:error, message} =
configure([:inet6, keyfile: "abcdef", certfile: "ghijkl", otp_app: :plug])

assert message =~
":keyfile either does not exist, or the application does not have permission to access it"
end

test "fails to configure if the certfile doesn't exist" do
File.touch("_build/test/lib/plug/abcdef")

assert {:error, message} =
configure([:inet6, keyfile: "abcdef", certfile: "ghijkl", otp_app: :plug])

assert message =~
":certfile either does not exist, or the application does not have permission to access it"
end

test "expands the paths to the keyfile and certfile using the otp_app" do
File.touch("_build/test/lib/plug/abcdef")
File.touch("_build/test/lib/plug/ghijkl")

assert {:ok, opts} =
configure([:inet6, keyfile: "abcdef", certfile: "ghijkl", otp_app: :plug])

assert to_string(opts[:keyfile]) =~ "_build/test/lib/plug/abcdef"
assert to_string(opts[:certfile]) =~ "_build/test/lib/plug/ghijkl"
end

test "supports the certs_keys ssl config option" do
assert {:ok, opts} =
configure([:inet6, certs_keys: [%{key: "abcdef", cert: "ghijkl"}]])

assert :inet6 in opts
assert opts[:certs_keys] == [%{key: "abcdef", cert: "ghijkl"}]
end

test "expands the paths for keyfile and certfile in the certs_keys ssl config option" do
File.touch("_build/test/lib/plug/abcdef")
File.touch("_build/test/lib/plug/ghijkl")

assert {:ok, opts} =
configure([
:inet6,
certs_keys: [%{keyfile: "abcdef", certfile: "ghijkl"}],
otp_app: :plug
])

assert :inet6 in opts

[%{keyfile: keyfile, certfile: certfile}] = opts[:certs_keys]

assert to_string(keyfile) =~ "_build/test/lib/plug/abcdef"
assert to_string(certfile) =~ "_build/test/lib/plug/ghijkl"
end

test "errors when an invalid cipher is given" do
assert configure(key: "abcdef", cert: "ghijkl", cipher_suite: :unknown) ==
{:error, "unknown :cipher_suite named :unknown"}
Expand Down
Loading