Skip to content
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

Corporate MITM Proxies and Rebar3 #2569

Closed
davebryson opened this issue May 26, 2021 · 4 comments
Closed

Corporate MITM Proxies and Rebar3 #2569

davebryson opened this issue May 26, 2021 · 4 comments

Comments

@davebryson
Copy link
Contributor

Environment

Task: get-deps
Entered as:
  get-deps
-----------------
Operating System: x86_64-apple-darwin20.4.0
ERTS: Erlang/OTP 24 [erts-12.0.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit] [dtrace]
...
-----------------
Loaded Applications:
bbmustache: 1.10.0
certifi: 2.6.1
cf: 0.3.1
common_test: 1.20.4
compiler: 8.0
crypto: 5.0.1
cth_readable: 1.5.1
dialyzer: 4.4
edoc: 1.0
erlware_commons: 1.5.0
eunit: 2.6.1
eunit_formatters: 0.5.0
getopt: 1.0.1
inets: 7.4
kernel: 8.0
providers: 1.8.1
public_key: 1.11
relx: 4.4.0
sasl: 4.1
snmp: 5.9.1
ssl_verify_fun: 1.1.6
stdlib: 3.15
syntax_tools: 2.6
tools: 3.5

Using the latest version of Rebar3: 3.16.1

I created a basic app from rebar3 new app.. to test this. I added a single dep: {gpb, "4.17.6"}

Current behaviour

Calling get-deps fails with:
TLS client: In state certify at ssl_handshake.erl:1899 generated CLIENT ALERT: Fatal - Unknown CA

I'm pretty sure this is a result of me sitting behind a corporate proxy the does the ole' mitm on certs:
Here's the complete DEBUG:

===> Load global config file ...
===> 24.0.1 satisfies the requirement for minimum OTP version 21
===> Compile (apps)
===> Expanded command sequence to be run: [app_discovery,install_deps,lock,'get-deps']
===> Running provider: app_discovery
===> Found top-level apps: [sample]
	using config: [{src_dirs,["src"]},{lib_dirs,["apps/*","lib/*","."]}]
===> Running provider: install_deps
===> Verifying dependencies...
===> Getting definition for package gpb from repo hexpm (#{api_url => <<"https://hex.pm/api">>,name => <<"hexpm">>,
         repo_name => <<"hexpm">>,repo_organization => undefined,
         repo_url => <<"https://repo.hex.pm">>,repo_verify => true,
         repo_verify_origin => true})
=NOTICE REPORT==== 26-May-2021::17:11:14.853149 ===
TLS client: In state certify at ssl_handshake.erl:1899 generated CLIENT ALERT: Fatal - Unknown CA

===> Hex get_package request failed: {error,
                                             {failed_connect,
                                              [{to_address,
                                                {"repo.hex.pm",443}},
                                               {inet,
                                                [inet],
                                                {tls_alert,
                                                 {unknown_ca,
                                                  "TLS client: In state certify at ssl_handshake.erl:1899 generated CLIENT ALERT: Fatal - Unknown CA\n"}}}]}}
===> Failed to update package gpb from repo hexpm
===> Package not found in any repo: gpb 4.17.6

Expected behaviour

Ideally users behind these type of proxies should be able to use Rebar without having to circumvent the proxy. Hex added a fix for this a while ago via a global variable setting: cacerts_path that allows a user to configure the pem needed to work with the proxy. I'm willing to take a stab at a patch to provide this capability if it's of interest to the group.

Thanks for all the great work!

@ferd
Copy link
Collaborator

ferd commented May 27, 2021

The SSL/TLS options are handled at https://github.com/erlang/rebar3/blob/master/src/rebar_utils.erl#L1028-L1068 and calls there currently do not take the rebar configuration for state handling to define different CA certs bundle, and instead assume all the CA Certs come from an in-memory bundle.

The options can be changed, but these options are currently usable by various pieces of internal code and by plugins.

If they rely state to be passed around (i.e. access to non-global config files) that could be app-dependent, it won't be acceptable.

However, you can see the code in get_ssl_config() that uses the global config file, which I believe would absolutely make sense for a corporate proxy: https://github.com/erlang/rebar3/blob/master/src/rebar_utils.erl#L1113-L1121

Currently, you could effectively just disable TLS verification by setting {ssl_verify, ssl_verify_disabled}, which is going to be about as safe as your corporate proxy would be for anything but the local network, but I'm assuming this isn't ideal either.

The handler at https://github.com/erlang/rebar3/blob/master/src/rebar_utils.erl#L1038-L1043 could take a third return value.

Now that third value is tricky to choose because validation needs to be done with:

            #{host := Hostname} = rebar_uri:parse(rebar_utils:to_list(Url)),
            VerifyFun = {fun ssl_verify_hostname:verify_fun/3,
                         [{check_hostname, Hostname}]},
            CACerts = certifi:cacerts(),
            [{verify, verify_peer}, {depth, 2}, {cacerts, CACerts},
             {partial_chain, fun partial_chain/1}, {verify_fun, VerifyFun}];

My assumption is that people will want to override cacerts and maybe depth but not necessarily the rest (unless they're pinning certs) and we're kind of stuck assuming they'll always rely on the URL regardless in the way we expect it.

As such maybe we could pass an option like {ssl_verify, {ssl_verify_cacerts, [{cacerts, Bundle} | {cacertfile, Path}]}}
We could rewrite ssl_opts/2 to call something like:

ssl_opts(Url) ->
    case get_ssl_config() of
        ssl_verify_enabled ->
            ssl_opts(ssl_verify_enabled, Url);
        ssl_verify_disabled ->
            [{verify, verify_none}];
       {ssl_verify_cacerts, Opts} ->
           ssl_opts(ssl_verify_enabled, Url, Opts)
    end.

ssl_opts(ssl_verify_enabled, Url) ->
    ssl_opts(ssl_verify_enabled, Url, [{cacerts, certifi:cacerts()}]).

ssl_opts(ssl_verify_enabled, Url, CAOpts) ->
    case check_ssl_version() of
        true ->
            #{host := Hostname} = rebar_uri:parse(rebar_utils:to_list(Url)),
            VerifyFun = {fun ssl_verify_hostname:verify_fun/3,
                         [{check_hostname, Hostname}]},
            [{verify, verify_peer}, {depth, 2},
             {partial_chain, fun partial_chain/1}, {verify_fun, VerifyFun} | CAOpts];
        false ->
            ?WARN("Insecure HTTPS request (peer verification disabled), "
                  "please update to OTP 17.4 or later", []),
            [{verify, verify_none}]
    end.

Something to that extent should keep all plugins and internal uses stable while also allowing a per-host configuration that can be overriden in a context-free manner.

@davebryson
Copy link
Contributor Author

Thanks for the response.

Here's what I have so far that appears to be working fine. It's all in rebar_utils.erl:
Added the following function:

get_cacerts() ->
    GlobalConfigFile = rebar_dir:global_config(),
    Config = rebar_config:consult_file(GlobalConfigFile),
    case proplists:get_value(cacert_path, Config) of
        undefined ->
            certifi:cacerts();
        Path ->
            %% The user has added a path to a PEM file to use in global config.
            {ok, Bin} = file:read_file(Path),
            Pems = public_key:pem_decode(Bin),
            [Der || {'Certificate', Der, _} <- Pems]
    end.

Then added the call to rebar_utils:ssl_opts/2:

ssl_opts(ssl_verify_enabled, Url) ->
    case check_ssl_version() of
        true ->
            #{host := Hostname} = rebar_uri:parse(rebar_utils:to_list(Url)),
            VerifyFun = {fun ssl_verify_hostname:verify_fun/3, [{check_hostname, Hostname}]},  
            %% Was: CACerts = certifi:cacerts(),
            CACerts = get_cacerts(), 
            [
                {verify, verify_peer},
                {depth, 2},
                {cacerts, CACerts},
                {partial_chain, fun partial_chain/1},
                {verify_fun, VerifyFun}
            ];
        false ->
            ?WARN(
                "Insecure HTTPS request (peer verification disabled), "
                "please update to OTP 17.4 or later",
                []
            ),
            [{verify, verify_none}]
    end.

To enable your own cacerts. You need to add the following to your global rebar.config:

{cacert_path, "FULLPATH TO YOUR PEM"}.

I can look at the additional options you mention, but I wonder if they're necessary?

One small side issue with my local erlang formatter (in vscode). It reformatted more code in rebar_utils than I touched, making the diff much larger than needed. Do you have a specific format to follow for a pull request?

Thanks!

@ferd
Copy link
Collaborator

ferd commented May 27, 2021

Yep that's smaller and self-contained. I'm giving myself a bit of time to think, the part I find a bit more annoying here is that you will essentially have to read and re-parse the CACerts on every single invocation of any URL we call (my proposal read the global config once per URL; not effective by any measure either). It's possibly not that bad since we're fairly efficient with network calls though.

I'm letting CI run on the PR.

@davebryson
Copy link
Contributor Author

Yes. I agree on the "read and re-parse". It's not the most efficient approach. But not sure how to resolve that right now

@ferd ferd closed this as completed May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants