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
wip: postfix: refactored tls certificate configuration #89178
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these changes!
f9a73e1
to
a3a784e
Compare
smtpd_tls_key_file = cfg.sslKey; | ||
|
||
smtpd_use_tls = true; | ||
// optionalAttrs (cfg.clientTlsCAFile != "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer tlsCAFile
. It's short and corresponds to the upstream name.
@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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
@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.
Basically this PR improves the situation by enabling opertunistic tls for client connections (when postfix connects to other mails servers for delivery). |
a3a784e
to
4f1f891
Compare
Is there anything I can do to get a little bit more traction to that PR? @peti Do you agree with my explanation? |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
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. |
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`.
4f1f891
to
a277892
Compare
@peti I guess the base question should be if NixOS should enable a configuration by default which 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 |
@peti Thanks for your answer!
I however fully agree with:
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.
@asbachb Yes! I think in this PR, we should switch to |
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. |
Extracted only usage of recommended config keys to enable tls and usage of system trust store: #90115 |
Additional information why that configuration is set to "off" by default:
|
It would be really good to see this merged. I'm currently specifying the recommended 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. :) |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
description = "File containing trusted certification authorities (CA) to verify certificates of mailservers contacted for mail delivery. | ||
Defaults to NixOS trusted certification authorities."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, defaultText
is nicer.
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."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deviant What don't you like about the suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
default = ""; | ||
description = "SSL key to use."; | ||
tlsChainFiles = mkOption { | ||
type = types.nullOr (types.listOf types.str); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type = types.nullOr (types.listOf types.str); | |
type = types.nullOr (types.listOf types.path); |
smtpd_tls_cert_file = cfg.sslCert; | ||
smtpd_tls_key_file = cfg.sslKey; | ||
|
||
smtpd_use_tls = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
smtpd_tls_key_file = cfg.sslKey; | ||
|
||
smtpd_use_tls = true; | ||
// optionalAttrs (cfg.clientTlsCAFile != "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer tlsCAFile
. It's short and corresponds to the upstream name.
I marked this as stale due to inactivity. → More info |
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 ofsmtpd_tls_cert
andsmtpd_tls_key
since this is the postfix recommended configuration since3.4
.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./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
nix path-info -S
before and after)