search

huan / docker-simple-mail-forwarder

Simplest and Smallest Email Forward Service based on Docker.
https://hub.docker.com/r/zixia/simple-mail-forwarder/
Apache License 2.0
544 stars 86 forks source link

Fix for issue where a signed cert and self signed cert can both exist #112

Open nickwest opened 2 years ago

nickwest commented 2 years ago

init-openssl.sh creates both RSA and EC self-signed certificates. When using a properly signed certificate of only one type (RSA or EC) the init-openssl.sh script will still generate the self-signed version of the other certificate type. This results in some requests to Postfix returning the self-signed cert rather than the signed cert.

This pull requests modifies init-openssl.sh to prevent the generation of self-signed certificates if either type of certificate already exists. If a certificate does exist it also checks for missing certificates and then uses sed to comment out the appropriate smtpd_tls_cert_file/key lines in main.cf.

When both or neither certificates are present it functions as it currently does.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

huan commented 2 years ago

Hello @nickwest , thank you very much for your improvement of the better compatible with CA settings!

I'd like to merge this PR after it gets at least one approval from the community so that we can make sure it works as expected.

P.S. add an unit test will be highly appreciated.

Have a great day!

nickwest commented 2 years ago

Hi @huan, I added a couple unit tests as requested. They verify that when both EC and RSA certs are present they share the same CN, domains, and are either both self signed or both CA signed. This ensures that regardless of which certificate is used the result will be equal.

The already present TLS tests will continue to verify that connections will work regardless of certificate matching and validity. Those tests also verify that having a single cert (the case prior to PR #51) is working properly.

If @Czocher is available they'd probably be a good reviewer/approver since this touches their work in PR #51 where EC certs were introduced.

I also see that PR #36 could introduce custom paths for certs. The unit tests in my PR do not account for that currently and would need to be changed if that PR is merged.

czocher commented 2 years ago

Hi @nickwest I'm available but I don't have access to any proper production environment at the moment to try to deploy and test it. I can have a look at the code though.