WIP / FEAT: Dual certificate support (eg ECDSA with RSA fallback)#1629
WIP / FEAT: Dual certificate support (eg ECDSA with RSA fallback)#1629polarathene wants to merge 3 commits intodocker-mailserver:masterfrom
Conversation
Since Postfix 3.4, `smtpd_tls_cert_file` and `smtpd_tls_key_file` have been deprecated in favor of `smtpd_tls_chain_files` which supports a list of values where a single or sequence of file paths provide a private key followed by it's certificate chain.
`smtpd_tls_chain_files` allows for multiple key+cert bundles so that you can provide different key types, such as ECDSA and RSA. To maintain compatibility with the current CERT/KEY ENV vars only a 2nd certificate is supported. Since Dovecot 2.2.31 a related feature is also available, but it is limited to only providing one alternative certificate via separate cert and key settings. --- This feature enables support for multiple certificates, eg for serving modern ECDSA certs with RSA as fallback.
|
Looks very nice. Keep up this good work. A word about the current situation with the tests: They are being refactored and rewritten in #1613 as this is absolutely necessary. |
|
Will that delay the merging of this feature? If so no worries :) I'm not particularly great at naming things, I've used
|
Not if you ask me :)
I guess it's fine. But have a look at polarathene#1 :D |
georglauterbach
changed the title
The current situation is centered mostly around getting all tests with their setup into .bats files instead of the makefile. Changing existing .bats files is absolutely fine with this PR. Changing tests.bats might light to some conflicts but I think if it is only addition of new test cases, they should be easy to resolve. In summary: No need to compromise the quality by leaving out tests. :) |
|
Thanks for clarifying @martin-schulze-vireso. If this is the case, a test or two won't be of harm - they should be added:) |
|
I welcome any help with sorting out tests for this then, but I'll try take a shot at it if no one can spare the time to. For now my spare time is allocated to the security update PR, slowly piecing together my review. |
|
I'm currently working on another k8s deployment, I'm sorry. Take your time. |
|
Just occurred to me that there should probably be some logic to "reset"/disable the alternative config if the ENV var is removed. It'd only affect Dovecot AFAIK, but should be reproducible with:
|
|
@polarathene This is just FYI: I was going through your commits when I recognized that they are not signed. If you'd like to improve on this, you can have a look here https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/signing-commits (improving on authenticity). This is just a recommendation from my side:) |
|
Because lately we've been seeing a lot of PRs, I will mark this as a draft too. Just mark it ready for review when it's done. |
|
Closed due to migration to docker-mailserver/docker-mailserver. Please re-open over there :D |
3 participants
Since Postfix 3.4 (Feb 2019),
smtpd_tls_cert_fileandsmtpd_tls_key_filehave been deprecated in favor ofsmtpd_tls_chain_fileswhich supports a list of values where a single or sequence of file paths provide a private key followed by it's certificate chain.1st commit - DRY logic, subtle changes/improvements, small bugfix
The first commit refactors the relevant code to support this settings change in Postfix, while also moving common logic for each TLS setup variation into a function.
sedlines were also revised as they often contained unnecessary options(-e,-r) and regex tokens/syntax(g), or specific paths that were better extracted out as variables or unnecessary for a match(use.*instead).The
self-signedcase had an additional-combined.pemfile used instead of the-cert.pemfor Dovecot'sssl_certvalue. This didn't make much sense (unless there's a known reason for doing this), the-combined.pemis a bundle file containing the private key followed by the certificate, I see no reason to support that so I've replaced it with the same-cert.pemthat Postfix uses for it's certificate. This makes it more consistent with other cases.self-signedDovecot paths to applysedto were also invalid due to whitespace within the file paths AFAIK.The
customcase also differed, in that it provides a similar key+cert bundle as the self-signed-combined.pem, this one is-full.pemwhich I've not inspected but assume contains a full certificate chain (leaf + issuer chain), which is useful for including CA certs in a chain that don't belong to a systems trust store.An additional bonus of this refactor is that previously a
docker-compose upwhen exited, persisted the container's internal filesystem state which would be used by a follow updocker-compose up, which caused unexpected behaviour / breakage in the event that you changed fromself-signedtomanualfor example or ifHOSTNAMEwere to change.docker-compose downwas required to properly clear this state to a "fresh" idempotent state. That issue may be present elsewhere in the codebase btw.Postfix
main.cfalso had a slight consistency fix to maintain spaces between=assignment like the rest of the config file and related configs such as Dovecot use.I've not tested each case personally, only the
manualcase. I'm not deeply familiar with the codebase or thebatstests, this PR has been validated to work formanual, it might benefit from an extrabatstest covering the dual cert support?2nd commit - Dual cert feature
smtpd_tls_chain_filesallows for multiple key+cert bundles so that you can provide different key types, such as ECDSA and RSA.To maintain compatibility with the current
SSL_CERT_PATHandSSL_KEY_PATHENV variables only a 2nd certificate is supported.Since Dovecot 2.2.31 (June 2017) a related feature is also available, but it is limited to only providing one alternative certificate via separate cert and key settings. That limitation is a non-issue as this feature is only supporting a 2nd alternative certificate via
SSL_ALT_CERT_PATHandSSL_ALT_KEY_PATHENV vars.TL;DR
This feature enables support for multiple certificates, eg for serving modern ECDSA cert with an RSA cert as fallback.
I needed this for testing my active PR. (which I'm already using locally, but figured was worth polishing up for a separate PR)
I presently do not have time to learn about
batsand how to implement any tests required for this PR, any assistance would be appreciated regarding that, even if it's just some guidance on what needs to be tested and roughly how to go about achieving that.