Add support for chained certificates #3134
Conversation
nelsonkopliku
requested review from
dottorblaster,
stefanotorresi,
CDimonaco,
balanza and
gagandeepb
| describe "Http executor certificate handling" do | ||
| test "should support chained certificates" do | ||
| scenarios = [ | ||
| %{cert: "one_entry_chain.pem", expected_entries: 1}, | ||
| %{cert: "two_entries_chain.pem", expected_entries: 2}, | ||
| %{cert: "three_entries_chain.pem", expected_entries: 3} | ||
| ] | ||
|
|
||
| for %{cert: cert, expected_entries: expected_entries} <- scenarios do | ||
| assert cert | ||
| |> load_certificate_content() | ||
| |> HttpExecutor.get_cert_der() | ||
| |> length() == expected_entries | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
I'm only left longing for more comprehensive testing, e.g. checking that the CA chain can actually verify a server certificate previously signed with it, but I realize this is a lot of work, and at the moment time is of the essence.
Maybe @gagandeepb could pick this up at a later stage, since he mentioned he wanted to do some testing in the context of our TLS usage in general.
There was a problem hiding this comment.
Definitely something to look into. Thanks for the pointer.
There was a problem hiding this comment.
I agree with you @stefanotorresi but at the same time this is also something on which we need a lot of feedback by the users 👍
lib/trento/infrastructure/software_updates/adapter/suma_http_executor.ex
Outdated
Show resolved
Hide resolved
| defp split_type_and_entry(asn1_entry) do | ||
| asn1_type = elem(asn1_entry, 0) | ||
| {asn1_type, asn1_entry} |
There was a problem hiding this comment.
nitpick: this function could be just declarative:
| defp split_type_and_entry(asn1_entry) do | |
| asn1_type = elem(asn1_entry, 0) | |
| {asn1_type, asn1_entry} | |
| defp split_type_and_entry({asn1_type, _} = asn1_entry) do | |
| {asn1_type, asn1_entry} |
There was a problem hiding this comment.
At the end I removed the function altogether and moved it in the mapping pipe 9569dde
Just for reference, the pem decoded entry seems to be a 4 element tuple rather than a 2 element tuple https://www.erlang.org/doc/apps/public_key/public_key.html#t:pem_entry/0.
Hope it remains like this 😄
test/trento/infrastructure/software_updates/adapter/suma_http_executor_test.exs
Outdated
Show resolved
Hide resolved
Description
Adds support for chained certificates in SUMA integration
See #2596 and https://elixirforum.com/t/using-client-certificates-from-a-string-with-httposion/8631/7
Supersedes #3131 which was targeting last release tag for testing purposes.