Skip to content

Injected cacerts default silently overrides a user-supplied cacertfile (private-CA pinning broken since #123) #133

Description

@Taure

Since #123 landed on main, a user-supplied cacertfile in ssl_options is silently discarded, because pgo also injects a cacerts default and OTP treats the two as mutually exclusive with cacerts winning. Any pool that pins a private CA via cacertfile fails to connect with {tls_alert, {unknown_ca, ...}}.

Mechanism

default_ssl_options/2 injects {cacerts, public_key:cacerts_get()} (the public trust store) among its defaults:

https://github.com/erleans/pgo/blob/36efee8/src/pgo_handler.erl#L177-L188

%% src/pgo_handler.erl:183-188
Defaults = [{verify, verify_peer},
            {cacerts, public_key:cacerts_get()},
            {server_name_indication, SNI},
            {customize_hostname_check,
             [{match_fun, public_key:pkix_verify_hostname_match_fun(https)}]}],
merge_ssl_options(Defaults, UserSSLOptions).

merge_ssl_options/2 drops an injected default only when the same key name appears in the user's options:

https://github.com/erleans/pgo/blob/36efee8/src/pgo_handler.erl#L190-L192

%% src/pgo_handler.erl:190-192
merge_ssl_options(Defaults, UserOptions) ->
    UserKeys = proplists:get_keys(UserOptions),
    [Opt || {Key, _} = Opt <- Defaults, not lists:member(Key, UserKeys)] ++ UserOptions.

cacertfile and cacerts are different key names, so supplying cacertfile does not suppress the injected cacerts, and both reach ssl:connect/2.

OTP then resolves the conflict in favour of cacerts and throws the file away. In ssl_config:opt_cacerts/3 (ssl 11.7.2, OTP 29.0.2), when cacerts is set, the cacertfile is replaced by <<>> and a notice is logged:

%% lib/ssl/src/ssl_config.erl:940-946
CaCertFile = case get_opt_file(cacertfile, <<>>, UserOpts, Opts) of
                 {Where1, _FileName} when CaCerts =/= undefined ->
                     warn_override(Where1, UserOpts, cacerts, [cacertfile], LogLevel),
                     <<>>;
                 ...

The private CA is therefore never in the chain, verification is attempted against the public bundle only, and the handshake is rejected.

Reproduction

Postgres serving a certificate signed by a private CA (this was a CloudNativePG cluster, but any private CA reproduces it), OTP 29.0.2 / ssl 11.7.2:

pgo:start_pool(default, #{host => "pg-rw.example.internal",
                          database => "app",
                          user => "app",
                          password => "...",
                          ssl => true,
                          ssl_options => [{cacertfile, "/etc/ssl/private-ca.crt"}]}).

ssl_options above is the documented way to pin a CA (README: "List of SSL options to use if ssl is true").

Observed on every connection attempt, first the OTP notice:

description: Options [cacertfile] are ignored
reason: Option cacerts is set

then the failure:

{error,{tls_alert,{unknown_ca,"TLS client: ... CLIENT ALERT: Fatal - Unknown CA"}}}

Bisect: the code as of v0.20.0 connects fine; with #123 applied every connection fails; reverting #123 restores it. Passing the CA as {cacerts, [DerBytes]} instead of {cacertfile, Path} also works, since that key name does suppress the injected default - that is the workaround we are using in the meantime.

Impact

This breaks any pgo user who pins a private CA via cacertfile and has SSL enabled, which is the standard configuration for CloudNativePG, RDS with a custom CA, Aiven, and most managed Postgres with verify_peer. It is a regression introduced by the fix for OTP-26+ SSL defaults. main only so far, since #123 is not in a released tag yet, but anyone tracking main as a git dependency is affected.

Suggested fix

merge_ssl_options/2 should treat mutually exclusive alternatives as a single logical slot, so that supplying either key suppresses the injected default for the whole slot. cacertfile/cacerts is the pair that bites today; the same shape covers any future mutually exclusive pair the defaults inject.

merge_ssl_options(Defaults, UserOptions) ->
    UserKeys = proplists:get_keys(UserOptions),
    Suppressed = UserKeys ++ [Alt || Key <- UserKeys, Alt <- alternative_keys(Key)],
    [Opt || {Key, _} = Opt <- Defaults, not lists:member(Key, Suppressed)] ++ UserOptions.

alternative_keys(cacerts) -> [cacertfile];
alternative_keys(cacertfile) -> [cacerts];
alternative_keys(_) -> [].

With that, {cacertfile, Path} from the user suppresses the injected {cacerts, public_key:cacerts_get()}, while the other secure defaults (verify_peer, SNI, hostname check) still apply, which keeps the intent of #123 intact.

Happy to send a PR with the change plus a test that asserts an injected default is dropped when its alternative key is supplied, if that would be useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions