[Questions] Trust store not overwriting existing ssl_options

[gomoripeti]

Community Support Policy

• I have read RabbitMQ's Community Support Policy
• I run RabbitMQ 4.x, the only series currently covered by community support
• I promise to provide all relevant information (versions, logs from all nodes, rabbitmq-diagnostics output, detailed reproduction steps)

RabbitMQ version used

4.1.2

Erlang version used

27.3.x

Operating system (distribution) used

linux

How is RabbitMQ deployed?

Debian package

rabbitmq-diagnostics status output

Logs from node 1 (with sensitive values edited out)

Logs from node 2 (if applicable, with sensitive values edited out)

No response

Logs from node 3 (if applicable, with sensitive values edited out)

No response

rabbitmq.conf

Steps to deploy RabbitMQ cluster

Steps to reproduce the behavior in question

advanced.config

[
{rabbit, {ssl_options,
[{fail_if_no_peer_cert, false},
{verify, verify_none}
] } }
].

Application code

No response

Kubernetes deployment file

No response

What problem are you trying to solve?

When the trust_store plugin is enabled it adds [{verify, verify_peer}, {fail_if_no_peer_cert, true}] options to ssl_options. However if verify or fail_if_no_peer_cert is already defined, the original value is not overwritten. This is because at least since OTP 26.0 the last value wins in ssl_options erlang/otp@cd00692#diff-c6e4e01fbddfaa6057fc43144940a337eb3db17891c62cce9ad2123e0107f776R1588
and trust store's required options happen to be prepended to the beginning of the list.

ie after enabling trust store

(rabbit-1@localhost)34> application:get_env(rabbit, ssl_options).
{ok,[{verify,verify_peer},
     {fail_if_no_peer_cert,true},
     {verify_fun,{fun rabbit_trust_store:whitelisted/3,continue}},
     {partial_chain,#Fun<rabbit_trust_store_app.0.60531072>},
     {verify,verify_none},
     {fail_if_no_peer_cert,false}]}
(rabbit-1@localhost)35> application:get_env(rabbit, initial_SSL_options).
{ok,[{verify,verify_none},{fail_if_no_peer_cert,false}]}

My first question is if this behaviour is intentional, that the user can overwrite trust store's defaults? (although they shouldn't)

I think it is not intentional as since 10 years the trust store uses lists:keymerge/3 (99182b0#diff-0a398c7f98662ed80bcc80a09e527da2f7ca67f36ecc5be9f6ad46e05b7ea92bR59) which requires both input lists to be key-sorted before evaluating this function. And [{verify, verify_peer}, {fail_if_no_peer_cert, true}] is definitely not key sorted (and probably initial ssl_options isn't either).

If this is a bug can I submit a PR to sort the input lists, or this legacy behaviour should not be changed after so many years.

My other question is that at least since OTP 26 if verify_peer is enabled then there must also be a cacerts or cacertfile option configured, otherwise the ssl app complains and doesn't start the listener. But CA certs are not required for peer cert verification to work via the trust store. Should the trust store add a {cacerts, []} option in this case to please OTP/ssl? Or maybe this should be handled by the ssl config verification (that if there is a verify_fun, then no cacerts should be equivalent with empty cacerts) ?

[lukebakken]

👍 👍 to both of your suggestions - ensure that the default options are key-sorted, and add {cacerts, [...]}:

[{cacerts, [...]}, {fail_if_no_peer_cert, true}, {verify, verify_peer}]

Though, if cacertfile is present in the incoming options, it should be read, parsed, and added to cacerts as well. In the unlikely (but possible) event cacerts is configured in advance, those should be merged in. Now that I think of it they should also be added as trusted certs since this plugin won't take them into consideration otherwise, I think.

There must not be many users of rabbitmq_trust_store with OTP 26 or newer, or they all just happen to configure cacertfile as well so that ssl starts correctly.

[michaelklishin]

This behavior is the most logical approach that was available in 2016 when the trust store plugin was developed.

We can change the overwriting part. As for adding an empty list of cacerts, I have no objections.

The trust store plugin is an an "all or nothing" approach to peer verification for TLS, so playing well with alternative setups was not the goal. But we can add or preserve some ssl_options options.

[gomoripeti]

while writing tests for #15116 I realised how weird edge cases can happen if the user configured some conflicting options. The "all or nothing" approach makes total sense, in practice users shouldn't configure conflicting options. This only came up for us because a cluster had the implicit default values ({verify, verify_none} and {fail_if_no_peer_cert, false}) configured explicitly, and it was surprising that enabling trust store plugin worked differently depending on if the defaults were spelled out or not.

[michaelklishin]

Addressed in #15116 by @gomoripeti.