haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.09k stars 661 forks source link

auth_proxy config #1892

Closed urlund closed 7 years ago

urlund commented 7 years ago

Expected behavior

auth_proxy should use the tls plugin config for key and cert path.

Observed behavior

auth_proxy has hardcoded names (paths) for key and cert

Current auth_proxy (line 118 and 119)

var key = self.config.get('tls_key.pem', 'binary');
var cert = self.config.get('tls_cert.pem', 'binary');
amirkarimi commented 7 years ago

I just took a look at tls plugin but couldn't find key and cert paths as config. Can you please tel me what do you mean by tls plugin config?

urlund commented 7 years ago

Currently auth_proxy cert/key are hardcoded here: https://github.com/haraka/Haraka/blob/master/plugins/auth/auth_proxy.js#L120-#L121

Since auth_proxy plugin requires the TLS plugin it should get it's cert/key from the tls.cert and tls.cert value: https://github.com/haraka/Haraka/blob/master/tls_socket.js#L312-L317

or at least make it a configurable value as auth_proxy.cert and auth_proxy.key (like tls) instead of being hardcoded.

amirkarimi commented 7 years ago

Thanks, I'll start working on this

amirkarimi commented 7 years ago

@urlund I just ran the tests before doing any change. One is failing should I ignore that?

✖ implicit_mx - mxs4am-aaaa.josef-froehle.de

AssertionError: 0 == 1
    at Object.equal (/home/amir/projects/open-source/Haraka/node_modules/nodeunit/lib/types.js:83:39)
    at /home/amir/projects/open-source/Haraka/tests/plugins/mail_from.is_resolvable.js:123:18
    at mail_from.is_resolvable:163:20
urlund commented 7 years ago

@amirkarimi I don't know about your setup, or why that error is thrown. I just reported a bug/missing feature 6 months ago. But I guess you can disable the mail_form.is_resolvable plugin?

If you refer to my "contributor" status, it's because I fixed a bug myself around the same time I reported this issue ;)

msimerson commented 7 years ago

Hey guys. It's a very good idea to use the config (tls.ini) settings from the TLS plugin as the defaults. In other places (smtp_forward, outbound, maybe more) we have "per-plugin" overrides for TLS. You could have an [auth_proxy] section defined in tls.ini. So a good idea might be to load tls.ini, checking for an [auth_proxy] section with a key/cert defined, and if not, use the settings from the [main] section, and finally, use the static defaults already defined in this plugin.

const tls_ini = config.get('tls.ini');
const key = self.config.get(tls_ini.main.key || 'tls_key.pem', 'binary');
amirkarimi commented 7 years ago

@msimerson I've seen net_utils.tls_ini_section_with_defaults('outbound') used in outbound/tls.js and net_utils.load_tls_ini() used in outbound/hmail.js for this purpose (if i'm not mistaken). Which one do you recommend?

msimerson commented 7 years ago

You are correct, but we're moving away from the "bits of TLS spread all over the place" situation. The net_utils functions should be considered deprecated and will be removed eventually. On the roadmap is also deprecating the TLS plugin as well and subsuming all of the TLS functionality into tls_socket.js.

amirkarimi commented 7 years ago

Just sent a PR. I would love to write some tests for this but have no idea what to emit as line event data on socket to reach here and see if the key and cert are loaded correctly.