wip: postfix: refactored tls certificate configuration #89178

asbachb · 2020-05-29T15:34:49Z

Motivation for this change

The change fixes #88817 and can be separated into following tasks:

Encourage usage of encrypted connections for outgoing traffic

This was done by configuring a trust store and enabling opportunistic usage of tls for outgoing traffic.

Depreaction of old configuration

Removed client certificate configuration on server certificate configuration

Previously the ssl certificate and key were used for smtpd (incoming mail) and for smtp (ougoing) mail.
Using a certifcate for outgoing mail traffic is quite uncommon and most likely not wanted when setting sslCert. The postfix documentation request to configure certificates for outgoing traffic only when you "must present client TLS certificates".

smtpd_tls_chain_files was used in favour of smtpd_tls_cert and smtpd_tls_key since this is the postfix recommended configuration since 3.4.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"

 ✘ asbachb@nixos  ~/dev/src/nix/nixpkgs   88817-postfix-tls-certs  nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
these paths will be fetched (0.03 MiB download, 0.13 MiB unpacked):
  /nix/store/gwhk0z7brxyl0sn8gl44rlsyf0n1xhx9-nixpkgs-review-2.3.1
copying path '/nix/store/gwhk0z7brxyl0sn8gl44rlsyf0n1xhx9-nixpkgs-review-2.3.1' from 'https://cache.nixos.org'...
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
remote: Enumerating objects: 2001, done.
remote: Counting objects: 100% (2001/2001), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 4252 (delta 1997), reused 1998 (delta 1997), pack-reused 2251
Receiving objects: 100% (4252/4252), 1.54 MiB | 2.94 MiB/s, done.
Resolving deltas: 100% (3039/3039), completed with 762 local objects.
From https://github.com/NixOS/nixpkgs
   4c1bab50578..3bc5b8593be  master     -> refs/nixpkgs-review/0
$ git worktree add /home/asbachb/.cache/nixpkgs-review/rev-f9a73e18aea3270837649f0853d103b90eee0454-dirty/nixpkgs 3bc5b8593be638dd9fc7d0f427eccdb2a6c8485b
Preparing worktree (detached HEAD 3bc5b8593be)
Updating files: 100% (21804/21804), done.
HEAD is now at 3bc5b8593be gpsd: clarify license
$ nix-env -f /home/asbachb/.cache/nixpkgs-review/rev-f9a73e18aea3270837649f0853d103b90eee0454-dirty/nixpkgs -qaP --xml --out-path --show-trace
No diff detected, stopping review...
$ git worktree prune

Tested execution of all binary files (usually in ./result/bin/)
May 29 16:55:54 nixos postfix/smtp[123634]: Trusted TLS connection established to gmail-smtp-in.l.google.com[2a00:1450:400c:c00::1b]:25: TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-256) server-digest SHA256
Determined the impact on package closure size (by running nix path-info -S before and after)
Ensured that relevant documentation is up to date
Fits CONTRIBUTING.md.

nlewo

Thanks for these changes!

nixos/modules/services/mail/postfix.nix

pkgs/servers/mail/postfix/default.nix

asbachb · 2020-05-30T10:35:23Z

nixos/modules/services/mail/postfix.nix

-        smtpd_tls_key_file = cfg.sslKey;
-
-        smtpd_use_tls = true;
+      // optionalAttrs (cfg.clientTlsCAFile != "") {


I wonder if clientTlsCAFile is a good config name since currently there's no client/server differentiation in config values. In contrast tlsChainFiles does not specify if the value is used for server or client.

You could argument that in default case the option referes to server. But I'm a little bit unsure about that.

I'd love to hear your opinion about that.

I prefer tlsCAFile. It's short and corresponds to the upstream name.

Avaq · 2020-05-30T10:42:02Z

@Ma27 just a heads-up that you might've assigned me as a reviewer by mistake. Maybe you meant to assign a different person? I believe I have had no prior involvement with these modules, files, or postfix in general. As such, I feel rather unequipped to review these changes.

peti

I am confused by this PR. Is my understanding correct that you change the default configuration such that in situation where Postfix could authenticate itself with a proper TLS certificate it won't? How is that beneficial for anyone?

asbachb · 2020-05-30T12:42:20Z

@peti the usage of client certificates is a quite rare usecase like in the browser. Normally the server present a certificates and the client checks if he's trusting it.

Do not configure client certificates unless you must present client TLS certificates to one or more servers. Client certificates are not usually needed, and can cause problems in configurations that work well without them.
http://www.postfix.org/postconf.5.html#smtp_tls_cert_file

Basically this PR improves the situation by enabling opertunistic tls for client connections (when postfix connects to other mails servers for delivery).

asbachb · 2020-06-04T17:58:13Z

Is there anything I can do to get a little bit more traction to that PR?

@peti Do you agree with my explanation?

nixos-discourse · 2020-06-06T18:58:01Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/178

peti · 2020-06-07T07:33:38Z

@peti Do you agree with my explanation?

I don't know, honestly. I understand that presenting a TLS certificate for the client might brake attempts to deliver outgoing messages to other servers that are broken, i.e. can't deal with the fact that the client is trying to authenticate itself. I'm sure such servers exist and if you want to maximize your own servers interoperability with the rest of the Internet, then having no client-side certificate configured is probably a good idea.

On the other hand, the notion that we could authenticate ourselves with strong cryptography but won't because some 0.001% of potential SMTP peers might not accept our incoming connection feels conceptually wrong. There's good reason to push strong cryptography as much as possible. We shouldn't disable that feature because some servers are broken, we should rather push to fix those servers.

Anyhow, I don't feel strongly about this issue. I'll just enable client TLS certificates on my server explicity if it should come to pass.

pkgs/servers/mail/postfix/default.nix

The change fixes NixOS#88817 and can be separated into following tasks: ## Encourage usage of encrypted connections for outgoing traffic This was done by configuring a trust store and enabling opportunistic usage of tls for outgoing traffic. ## Depreaction of old configuration ## Removed client certificate configuration on server certificate configuration Previously the ssl certificate and key were used for smtpd (incoming mail) and for smtp (ougoing) mail. Using a certifcate for outgoing mail traffic is quite uncommon and most likely not wanted when setting `sslCert`. The postfix documentation request to configure certificates for outgoing traffic only when you "must present client TLS certificates". `smtpd_tls_chain_files` was used in favour of `smtpd_tls_cert` and `smtpd_tls_key` since this is the postfix recommended configuration since `3.4`.

asbachb · 2020-06-07T11:25:27Z

@peti I guess the base question should be if NixOS should enable a configuration by default which
a) Might have less use cases than causing trouble
b) Is against the suggestions of postfix itself

I agree that we should push server to use encryption. But I don't think providing client certificates would improve the situation since I don't see someone enabling client certificates, but not enabling server certificates.

Maybe another option would be to split up that PR into another PR which only change clientTlsCAFile and smtp{,d}_tls_security_level and wait for additional feedback on the question if we should configure client certificates by default.

nlewo · 2020-06-07T15:31:38Z

@peti Thanks for your answer!
Even if I don't have a strong opinion on this issue, I was initially in favor of this PR because it follows the behavior recommended by the Postfix team (http://www.postfix.org/postconf.5.html#smtp_tls_cert_file):

Do not configure client certificates unless you must present client TLS certificates to one or more servers. Client certificates are not usually needed, and can cause problems in configurations that work well without them.

I however fully agree with:

the notion that we could authenticate ourselves with strong cryptography but won't because some 0.001% of potential SMTP peers might not accept our incoming connection feels conceptually wrong.

I'm really surprised with the Postfix team recommendation and I'm wondering if we are not missing something...

If the only motivation of removing the client TLS authentication is to support some SMTP servers that doesn't support this authentication, I agree with @peti that we should not make this behavior the default one.

Moreover, I'm running a Postfix NixOS server since more than 3 years and I never observed any issue related to this client TLS authentication configuration.

Maybe another option would be to split up that PR into another PR which only change clientTlsCAFile and smtp{,d}_tls_security_level and wait for additional feedback on the question if we should configure client certificates by default.

@asbachb Yes! I think in this PR, we should switch to smtp{,d}_tls_chain_files and smtp{,d}_tls_security_level configuration variables by preserving the current behavior (ie. TLS client authentication is enabled by default). We could then discuss the TLS client authentication somewhere else (issue or PR).

asbachb · 2020-06-07T19:04:07Z

I'm really surprised with the Postfix team recommendation and I'm wondering if we are not missing something...

I guess because it's another use case. I don't think the main reason is encryption, I guess the main use case of client certificates is the same as in webserver world - To limit or ensure that a client is the client which is allowed to use a server. Like authorized keys for ssh.

I'd bet if you check your logs you don't find any situation your server offered a client certificate and it was accepted.

asbachb · 2020-06-11T01:52:14Z

Extracted only usage of recommended config keys to enable tls and usage of system trust store: #90115

asbachb · 2020-06-16T19:07:58Z

Additional information why that configuration is set to "off" by default:

To receive a remote SMTP client certificate, the Postfix SMTP server must explicitly ask for one (any contents of $smtpd_tls_CAfile are also sent to the client as a hint for choosing a certificate from a suitable CA). Unfortunately, Netscape clients will either complain if no matching client certificate is available or will offer the user client a list of certificates to choose from. Additionally some MTAs (notably some versions of qmail) are unable to complete TLS negotiation when client certificates are requested, and abort the SMTP session. So this option is "off" by default.

see http://www.postfix.org/TLS_README.html

deviant · 2021-05-21T10:44:03Z

It would be really good to see this merged. I'm currently specifying the recommended smtpd_tls_chain_files manually instead of using this module's TLS options, but would like to be able to just rely on it Doing The Right Thing.

I agree about disabling client certificates: using TLS at all for mail is opportunistic, and we have DKIM/DMARC for authenticating domains. Currently the only use I'm aware of for client certificates is authentication by MUAs. While I'd love to exist in a time where mutual TLS is mandatory between mail servers, we aren't nearly there yet, and Postfix recommends sane defaults for good deliverability. Regardless of whether servers should accept them, the least tested code path is the one with the most bugs. :)

asbachb · 2021-05-22T23:39:55Z

Currently there's no plan to merge that PR due the lack of support for these changes. I guess as long as the package maintainer @globin and @dotlambda don't position to this issue we won't get any further.

dotlambda

Regarding the client certificate: Removing it doesn't affect whether traffic is encrypted or not. Since virtually no mailserver uses client certificates to authenticate the sending server, I don't see any use in them either. DKIM is used for that and is built on the same root of trust as TLS certificates: DNS.

Some people from nixos-mailserver should have a look at this.
cc @nlewo @andir @symphorien @petabyteboy

PS: Some line wrapping would be nice.

dotlambda · 2021-05-23T11:30:07Z

nixos/modules/services/mail/postfix.nix

+        description = "File containing trusted certification authorities (CA) to verify certificates of mailservers contacted for mail delivery.
+        Defaults to NixOS trusted certification authorities.";


Suggested change

description = "File containing trusted certification authorities (CA) to verify certificates of mailservers contacted for mail delivery.

Defaults to NixOS trusted certification authorities.";

description = ''

File containing trusted certification authorities (CA) to verify certificates of mailservers contacted for mail delivery.

'';

The default should be obvious enough.

How does the default render? If it's (build of cacert-3.36)/etc/ssl/certs/ca-bundle.crt or /nix/store/732wsyyfia0l69nj5n1046gg35azjqnq-nss-cacert-3.63/etc/ssl/certs/ca-bundle.crt, then defaultText might be in order.

Sure, defaultText is nicer.

dotlambda · 2021-05-23T11:31:15Z

nixos/modules/services/mail/postfix.nix

+        description = "List of one or more PEM files, each holding one or more private keys directly followed by a corresponding certificate chain.
+        Each key must appear immediately before the corresponding certificate, optionally followed by additional issuer certificates that complete the certificate chain for that key.";


Suggested change

description = "List of one or more PEM files, each holding one or more private keys directly followed by a corresponding certificate chain.

Each key must appear immediately before the corresponding certificate, optionally followed by additional issuer certificates that complete the certificate chain for that key.";

description = ''

List of one or more PEM files, each holding one or more private keys directly followed by a corresponding certificate chain.

Each key must appear immediately before the corresponding certificate, optionally followed by additional issuer certificates that complete the certificate chain for that key.

'';

example = [ "/var/lib/acme/example.com/key.pem" "/var/lib/acme/example.com/cert.pem" ];

@deviant What don't you like about the suggestion?

Apologies, @dotlambda. I was in the middle of writing this out and got waylaid:

The Postfix docs explicitly recommend against splitting up the key and certificate chain for reasons of update atomicity; how about the following?

[ "/var/lib/acme/example.com/full.pem" "/var/lib/acme/example.org/full.pem" ]

Additionally, the description feels like it's repeating itself a bit, but I'm not sure how to better put it right now.

dotlambda · 2021-05-23T11:31:19Z

nixos/modules/services/mail/postfix.nix

-        default = "";
-        description = "SSL key to use.";
+      tlsChainFiles = mkOption {
+        type = types.nullOr (types.listOf types.str);


Suggested change

type = types.nullOr (types.listOf types.str);

type = types.nullOr (types.listOf types.path);

dotlambda · 2021-05-23T11:46:17Z

nixos/modules/services/mail/postfix.nix

-        smtpd_tls_cert_file = cfg.sslCert;
-        smtpd_tls_key_file = cfg.sslKey;
-
-        smtpd_use_tls = true;


Maybe we should add an option to set smtpd_tls_security_level. Seems like a very important option that users of the module should be made aware of.

Probably, yes, but may is a sane default at the very least. Perhaps that should go in a separate PR, since this wasn't configurable from the start?

"may" is a sane default, but not the upstream one. So I would be hesitant to make it the default in NixOS.
If @asbachb wants to add a commit adding such an option, I don't see a need for a separate PR. Otherwise, I'll make a PR with the change.

@dotlambda Only because TLS is disabled by default. When using it, "may" is the level they tell you to use: http://www.postfix.org/TLS_README.html#server_enable

Explicitly switch it on with "smtpd_tls_security_level = may".

The only other options are "none", and "encrypt". For "encrypt", it has the following to say:

According to RFC 2487 this MUST NOT be applied in case of a publicly-referenced SMTP server. Instead, this option should be used only on dedicated servers.

Per RFC 2487, a "publicly-referenced SMTP server" is:

A publicly-referenced SMTP server MUST NOT require use of the
STARTTLS extension in order to deliver mail locally. This rule
prevents the STARTTLS extension from damaging the interoperability of
the Internet's SMTP infrastructure. A publicly-referenced SMTP server
is an SMTP server which runs on port 25 of an Internet host listed in
the MX record (or A record if an MX record is not present) for the
domain name on the right hand side of an Internet mail address.

In other words, unless you're running Postfix for an intranet, you almost certainly want to use "may" here.

dotlambda · 2021-05-23T11:46:20Z

nixos/modules/services/mail/postfix.nix

-        smtpd_tls_key_file = cfg.sslKey;
-
-        smtpd_use_tls = true;
+      // optionalAttrs (cfg.clientTlsCAFile != "") {


I prefer tlsCAFile. It's short and corresponds to the upstream name.

stale · 2022-01-09T07:07:16Z

I marked this as stale due to inactivity. → More info

asbachb requested a review from peti as a code owner May 29, 2020 15:34

ofborg bot added 6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) labels May 29, 2020

ofborg bot requested review from rickynils and globin May 29, 2020 15:42

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 29, 2020

nlewo requested changes May 29, 2020

View reviewed changes

nixos/modules/services/mail/postfix.nix Outdated Show resolved Hide resolved

nixos/modules/services/mail/postfix.nix Outdated Show resolved Hide resolved

pkgs/servers/mail/postfix/default.nix Outdated Show resolved Hide resolved

Ma27 requested review from Avaq, cawilliamson and WilliButz May 29, 2020 20:48

asbachb force-pushed the 88817-postfix-tls-certs branch from f9a73e1 to a3a784e Compare May 30, 2020 10:27

asbachb commented May 30, 2020

View reviewed changes

peti reviewed May 30, 2020

View reviewed changes

asbachb force-pushed the 88817-postfix-tls-certs branch from a3a784e to 4f1f891 Compare June 3, 2020 16:33

asbachb requested review from nlewo and peti June 3, 2020 16:35

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1 and removed 10.rebuild-darwin: 1-10 labels Jun 3, 2020

peti reviewed Jun 7, 2020

View reviewed changes

pkgs/servers/mail/postfix/default.nix Outdated Show resolved Hide resolved

asbachb force-pushed the 88817-postfix-tls-certs branch from 4f1f891 to a277892 Compare June 7, 2020 08:43

asbachb changed the title ~~postfix: refactored tls certificate configuration~~ wip: postfix: refactored tls certificate configuration Jun 11, 2020

asbachb marked this pull request as draft June 16, 2020 19:41

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

Avaq removed their request for review December 30, 2020 14:14

dotlambda requested changes May 23, 2021

View reviewed changes

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wip: postfix: refactored tls certificate configuration #89178

wip: postfix: refactored tls certificate configuration #89178

asbachb commented May 29, 2020 •

edited

nlewo left a comment •

edited

asbachb May 30, 2020

dotlambda May 23, 2021

Avaq commented May 30, 2020

peti left a comment

asbachb commented May 30, 2020 •

edited

asbachb commented Jun 4, 2020

nixos-discourse commented Jun 6, 2020

peti commented Jun 7, 2020

asbachb commented Jun 7, 2020

nlewo commented Jun 7, 2020

asbachb commented Jun 7, 2020 •

edited

asbachb commented Jun 11, 2020

asbachb commented Jun 16, 2020

deviant commented May 21, 2021

asbachb commented May 22, 2021

dotlambda left a comment

dotlambda May 23, 2021

deviant May 25, 2021

dotlambda May 25, 2021

dotlambda May 23, 2021

dotlambda May 25, 2021

deviant May 28, 2021

dotlambda May 23, 2021

dotlambda May 23, 2021

deviant May 25, 2021

dotlambda May 25, 2021

deviant May 28, 2021

dotlambda May 23, 2021

stale bot commented Jan 9, 2022

		description = "File containing trusted certification authorities (CA) to verify certificates of mailservers contacted for mail delivery.
		Defaults to NixOS trusted certification authorities.";

		description = "List of one or more PEM files, each holding one or more private keys directly followed by a corresponding certificate chain.
		Each key must appear immediately before the corresponding certificate, optionally followed by additional issuer certificates that complete the certificate chain for that key.";

	type = types.nullOr (types.listOf types.str);
	type = types.nullOr (types.listOf types.path);

wip: postfix: refactored tls certificate configuration #89178

Are you sure you want to change the base?

wip: postfix: refactored tls certificate configuration #89178

Conversation

asbachb commented May 29, 2020 • edited

Motivation for this change

Encourage usage of encrypted connections for outgoing traffic

Depreaction of old configuration

Removed client certificate configuration on server certificate configuration

Things done

nlewo left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Avaq commented May 30, 2020

peti left a comment

Choose a reason for hiding this comment

asbachb commented May 30, 2020 • edited

asbachb commented Jun 4, 2020

nixos-discourse commented Jun 6, 2020

peti commented Jun 7, 2020

asbachb commented Jun 7, 2020

nlewo commented Jun 7, 2020

asbachb commented Jun 7, 2020 • edited

asbachb commented Jun 11, 2020

asbachb commented Jun 16, 2020

deviant commented May 21, 2021

asbachb commented May 22, 2021

dotlambda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jan 9, 2022

asbachb commented May 29, 2020 •

edited

nlewo left a comment •

edited

asbachb commented May 30, 2020 •

edited

asbachb commented Jun 7, 2020 •

edited