Don't expose SMTP/IMAP if announced "not provided" via SRV

[SailReal]

Fixes #5944

[milkmaker]

Thanks for contributing!

I noticed that you didn't select staging as your base branch. Please change the base branch to staging.
See the attached picture on how to change the base branch to staging:

[SailReal]

Is there anything else I can help with or do to get the PR merged and published?

[DerLinkman]

Still did not had time to test... but we'll keep it on track for 2024-08 Update

[dragoangel]

Why this applies only to non SSL versions? Why this needed at all? What if dns resolving just hangs on mailcow side?

Really not sure if this have to be merged! Srvs for client ports never used at all, why mailcow have to decide what to show and what to not based on resolving of srv and doing it "selectively"? This is NOT okay.

I have assumption @SailReal doesn't know that mailcow DO NOT allow plaintext connection on starttls ports for everything except 25 port. So every client connect is SECURED. If user can't establish tls over 587 for example - mailcow would just discard further connection, before accepting credentials and other things.

@DerLinkman

[dragoangel]

Also why contibute.md is part of this PR? How it's relevant to autodiscovery?

[SailReal]

Why this applies only to non SSL versions? Why this needed at all? What if dns resolving just hangs on mailcow side?

Really not sure if this have to be merged! Srvs for client ports never used at all, why mailcow have to decide what to show and what to not based on resolving of srv and doing it "selectively"? This is NOT okay.

I have assumption @SailReal doesn't know that mailcow DO NOT allow plaintext connection on starttls ports for everything except 25 port. So every client connect is SECURED.

@dragoangel did you read #5944 (comment) ? It means we do this already e.g. in https://github.com/mailcow/mailcow-dockerized/blob/master/data/web/autodiscover.php#L164-L185 so why is this a problem for autoconfig but not for autodiscover? This PR only removes the diff between those configs.

Also why contibute.md is part of this PR? How it's relevant to autodiscovery?

This is because the default branch of this repo is master and I started my changes using this branch but #5945 (comment) wants to have staging as the base branch and staging is not merged into master, see master...staging or

...but yes, I could have rebased my changes after changing the base of this PR.

[dragoangel]

Yeah @SailReal , I was not aware that there already such logic exist about checking dns srv records, it isn't documented by the way. My fail

The another part that you modify only specific smtp and imap, while for pop3 there both starttls and ssl ports are checked.

About hanging dns, well I wrongly read expression, | 🤪, in php is funky... So it should not be issue here, okay.

[dragoangel]

So, I think it can be done with documenting this approach in mailcow docs, and applies logic fully to every proto.

[SailReal]

Yeah @SailReal , I was not aware that there already such logic exist about checking dns srv records, it isn't documented by the way.

and

So, I think it can be done with documenting this approach in mailcow docs, and applies logic fully to every proto.

Sure, it is documented and I also mention it in my issue, please read the issue.

In the docs in https://docs.mailcow.email/getstarted/prerequisite-dns/#the-advanced-dns-configuration is stated

SRV records specify the server(s) for a specific protocol on your domain. If you want to explicitly announce a service as not provided, give "." as the target address (instead of "mail.example.org."). Please refer to RFC 2782.

[dragoangel]

So, I think it can be done with documenting this approach in mailcow docs, and applies logic fully to every proto.

@SailReal you okay with that?

[DerLinkman]

Hello, i'm sorry for the mix up...

@dragoangel said right that you missed the SRV Checks for some Socket types (SMTP is missing for SSL, IMAP is missing for SSL).

As you correctly said it was setup before for Pop3 for example, but there for both Socket Types.. could you reopen a new pr with the said features (as i think i cannot reopen this pr here?)

[dragoangel]

Plus this example is wrong:

_smtps._tcp           SRV   0 1 465 mail.example.com
_submissions._tcp     SRV   0 1 465 mail.example.com
_submission._tcp      SRV  0 0 0   .
_smtp._tcp            SRV  0 0 0   .

Because it doesn't work that way, actually it's about:

_smtp._tcp              IN SRV  10 0 25  smtp.my.server.
_smtps._tcp             IN SRV   0 0 465 smtp.my.server.
_submission._tcp        IN SRV   0 0 587 smtp.my.server.

smtp is not client port at all.

smtps is 465 and it's client port with ssl

submission is 587 client port with starttls and there never was such proto as submissions.

Also you can't point client to 465 smtps on submission...

And for smtp - you should not null it, this not client port at all. It should not be used in any way in autodiscovery process!

[SailReal]

could you reopen a new pr with the said features (as i think i cannot reopen this pr here?)

@DerLinkman First let's see in which direction we are going...

submission is 587 client port with starttls and there never was such proto as submissions.

@dragoangel This means I misunderstand https://datatracker.ietf.org/doc/html/rfc8314#section-3.3 ?

[dragoangel]

@SailReal I will double read the rfc and come back

[dragoangel]

Okay, was not aware about "submissions" used for that, every day something new 😁 (rfc 4 years old to note). Still smtp proto is about MTA-MTA traffic, and should not be touched here.

To note: if you try google "submissions SRV", you will not find it, but older https://datatracker.ietf.org/doc/html/rfc6186

And you would not find any docs from other mail servers that utilize it or describe it's usage.

Why it so: main reason is security and real world practice.

Mailcow using unbound to directly resolve DNS names from authorities NS, DNS not secure out of the box and can be intercepted, without DNSSEC (which is far not always enabled) and validation of signatures it will allow to manipulate behavior of the server.

Due to same reasons SRV records aren't used for service discovery at all email clients.

Till we not parse target this is DoS vulnurability (potentially allow to autodiscovery with 0 services in list), if we would touch target (render it from SRV) - then this would be even worse - account takeover vulnerability, as client could be pointed to evil smtp or imap server and will expose his credentials to it.

What proper, secure and clean solution I see here to allow user to control what to show and what to not: add to mailcow admin UI autodiscovery settings for domain which will store it's data in MySQL database. This settings can cover:

1. checkboxes over each protocol to show it in autodiscovery or not.
2. each enabled protocol would allow to set it's port and target (hostname)

By default all servers have enabled all protocols and target to {MAILCOW_HOSTNAME} and default ports.
Proto-port:

1. IMAPS: 993
2. IMAP: 143
3. POP3S: 995
4. POP3: 110
5. Submission: 587
6. SubmissionS: 465

We can allow admins to use 2 of predefined variable in way we doing this for webhooks in Pushover:

1. {DOMAIN} - allow user to compose autodiscovery in way imap.{DOMAIN}, and so on
2. {MAILCOW_HOSTNAME}

And there will be ACL to allow domain admins to control this settings or prohibit rights to them.

This will be better from interception perspective due to DNS not involved here as all settings would be written by authorized users to database and would allow admins to point clients to non default targets.

For SRV records, as said - on practice they aren't impact anything from client perspective, I propose drop them at all from docs and usage, this honestly a rudiment.

We had previously propose users to set TLSA on each SSL port, and it was not give any profit everywhere except SMTP where it actually used, here is same story - we asking users to set SRV records which actually doesn't doing anything.

[dragoangel]

To add about proper solution: we can prohibit user from turning off fully smtp or both pop3 and imap, or opposte: allow turning autodiscovery fully and then use it as base to configure acme to skip autodiscovery records for this domain ;)

[dragoangel]

Based on targets in autodiscover and extra checkbox for LE (for each target which enabled by default) we can compose list of SAN names to request from acme, I think it would be good option as well.

Add extra LE section in general setting to enable it or not, extra sans not covered by targets (like webmail subdomain), email to use for LE, and here we moved every LE setting to DB.

[SailReal]

I would like to be able to configure this in the backend, sure, but currently as mentioned in the issue to this PR, if SRV is set, we already handle this differently in autodiscover and autoconfig! This PR is just trying to make those endpoints a bit more streamlined. I agree that the fact that it can be configured via DNS could be exploited, but this is not introduced in this PR, it already exists as in production for a long time, even worse, if you can force a client to use one endpoint instead of another, it now uses different configurations (autodiscover VS autoconfig). Which in turn, if you also agree to https://www.usenix.org/system/files/sec21-poddebniak.pdf, introduces a downgrade attack itself next to DNS.

Moving everything into the backend is for sure the right choise but it is a completely different issue or feature request than I tried to address in this PR and issue (at least imo). If your answer is that we should solve my problem by moving it to the backend, I would say that is the right way to go, but what do we do in the meantime, because that sounds like a bigger task to me?

With this PR, I tried to get as close as possible to https://github.com/mailcow/mailcow-dockerized/blob/master/data/web/autodiscover.php#L164-L185... they always expose only SMPTs and IMAPs, which I also tried to archive with this PR. Of course, this is still not true if SMPTs and IMAPs are not set to SRV. So in the short term, if you agree that we should not leave it as it is in the short term, what would you accept?

[dragoangel]

I really don't think allow removing all protos over SRV is safe. I saw many times how people in mailcow telegram chat suffered from DNS MITM by gov, or who ever it was (hosting company, etc).

I think we can go for now with just imaps, pop3s, and submissions and do remove this SRV resolving at all. Client who using autoconfig will have one of the options to choose (pop3 or imap, every client by default prefer imap) and if it works for him, its works, if disabled for user it will not - simple and solid.

Plus create FR based on my proposed solution to work on if wanted.

@SailReal is this okay for you?

[SailReal]

I think we can go for now with just imaps, pop3s, and submissions and do remove this SRV resolving at all. Client who using autoconfig will have one of the options to choose (pop3 or imap, every client by default prefer imap) and if it works for him, its works, if disabled for user it will not - simple and solid.

That sounds good, personally I'd rather go with what autodiscover currently reveals (no POP3s) because in my area it regularly causes confusion and I don't know anyone who intentionally uses POP3 but could live with it as an option, this is up to you but dropping all the if/else stuff is awesome imho.

[dragoangel]

Totally with you about pop3, it's far way worse then imap, specially because it doesn't cares about everything except inbox, like mail in junk is not visible for pop3 users, but this is another story, so lets people do their own decisions. Admins can block access for user via settings if needed.

[SailReal]

@dragoangel So it should be somehting like this SailReal@0e91470, right?

I'm asking because I'm a bit confused by this if condition SailReal@0e91470#diff-b137114f3d0fba21611babf284b9a7fb3abe18c94220fead8086768e3d002455R155 ... "if imap then add (also) smtp" 😅

[dragoangel]

Will check tomorrow. But I don't have Outlook to test 😞

[dragoangel]

I'm asking because I'm a bit confused by this if condition SailReal@0e91470#diff-b137114f3d0fba21611babf284b9a7fb3abe18c94220fead8086768e3d002455R155 ... "if imap then add (also) smtp" 😅

About what you see: it because MS Outlook can connect to IMAP\POP3 and SMPT, CalDAV and CalDAV or just to EAS (ActiveSync) which will provide everything at once, but it designed for mobile use.

About PR: now it looks okay. Should work just fine. If anyone has Outlook to test this, it would be nice 🙂

[SailReal]

Created #6014 with the discussed changes.